-
Notifications
You must be signed in to change notification settings - Fork 458
Migrate HTMLInputElement #2338
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Migrate HTMLInputElement #2338
Conversation
|
Thanks for the PR! This section of the codebase is owned by @saschanaz - if they write a comment saying "LGTM" then it will be merged. |
inputfiles/addedTypes.jsonc
Outdated
| "overrideType": "AutoFillBase | `${OptionalPrefixToken<AutoFillSection>}${OptionalPrefixToken<AutoFillAddressKind>}${AutoFillField}${OptionalPostfixToken<AutoFillCredentialField>}`" | ||
| }, | ||
| { | ||
| "name": "Directions", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have a plan to remove that again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now, let's keep it in the JSON until we migrate because we still don't have support for type defs in KDL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Maybe something like SelectionDirection to prevent future conflict.
- Maybe do the KDL work first to prevent back and forth?
|
Can you say maybe HTMLInputElement or something to be more clear for the title? |
|
Done |
src/build/patches.ts
Outdated
| name, | ||
| ...optionalMember("type", "string", node.properties?.type), | ||
| ...optionalMember("overrideType", "string", node.properties?.overrideType), | ||
| ...optionalMember("nullable", "boolean", node.properties?.nullable), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is part of Typed and that should be handled by type node.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Added comments to clarify nullable types in setSelectionRange method.
| } | ||
| } | ||
| property autocomplete type=AutoFill | ||
| property valueAsDate type=Date |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this should be allowed, Date is not an IDL type. But maybe not worth disallowing it... Let's see.
|
LGTM |
|
Merging because @saschanaz is a code-owner of all the changes - thanks! |
No description provided.