-
Notifications
You must be signed in to change notification settings - Fork 106
fix(ui-selectable,ui-select): fix typing of Select and Selectable eve… #2289
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
fix(ui-selectable,ui-select): fix typing of Select and Selectable eve… #2289
Conversation
|
| return handleHighlightOption(event, { | ||
| id: options[options.length - 1].id | ||
| }) | ||
| if ('key' in event) { |
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 changed a few examples in README a bit to make them TypeScript error-free
b32cfdb to
d8ea8da
Compare
| const newOptions = filterOptions(value) | ||
| setInputValue(value) | ||
| setFilteredOptions(newOptions) | ||
| sethHighlightedOptionId(newOptions.length > 0 ? newOptions[0].id : null) |
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 was a typo
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.
We should really type check these somehow...
matyasf
left a comment
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.
looks good, just a small addition
| /** | ||
| * Callback fired when first option should be highlighted | ||
| */ | ||
| onRequestHighlightFirstOption?: (event: React.SyntheticEvent) => void | ||
| onRequestHighlightFirstOption?: (event: React.KeyboardEvent) => void | ||
|
|
||
| /** | ||
| * Callback fired when last option should be highlighted | ||
| */ | ||
| onRequestHighlightLastOption?: (event: React.SyntheticEvent) => void | ||
| onRequestHighlightLastOption?: (event: React.KeyboardEvent) => void |
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 did not even knew that we have such event listeners, as I see from the code these are activating when pressing the home and end buttons respectively. Can you add this to the documentation here?
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 updated these
| const newOptions = filterOptions(value) | ||
| setInputValue(value) | ||
| setFilteredOptions(newOptions) | ||
| sethHighlightedOptionId(newOptions.length > 0 ? newOptions[0].id : null) |
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.
We should really type check these somehow...
…nt types and TypeScript errors in the examples INSTUI-4866
d8ea8da to
bdd9fe6
Compare
balzss
left a comment
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.
good attention to detail, nice job!
…nt types and TypeScript errors in the examples
INSTUI-4866
ISSUE:
TEST PLAN: