-
Notifications
You must be signed in to change notification settings - Fork 355
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
feat(Select): Typeahead example #10207
Merged
tlabaj
merged 12 commits into
patternfly:main
from
adamviktora:select-typeahead-example-2
May 8, 2024
+541
−349
Merged
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
d0afb7d
refactor(Select): rename shouldFocusFirstMenuItemOnOpen
adamviktora 674ee20
feat(SelectTypeahead example): better arrow up/down keys handling
adamviktora 82bb47b
feat(SelectTypeahead example): don't close menu on clicking clear but…
adamviktora 85c6c35
refactor(SelectTypeahead example)
adamviktora 9ba9f22
refactor(SelectTypeahead example)
adamviktora 87fbdec
fix(SelectTypeaheadCreatable example): changes based on SelectTypeahead
adamviktora 6e8ee52
fix(SelectMultiTypeahead example): changes based on SelectTypeahead
adamviktora da0179a
fix(SelectTypeaheadCreatable example): don't show create option if th…
adamviktora f34d61c
fix(SelectMultiTypeaheadCreatable): changes based on SelectTypeahead
adamviktora 2848912
fix(SelectMultiTypeaheadCheckbox): changes based on SelectTypeahead
adamviktora 87121d7
fix(SelectTypeaheadCreatable): close menu after creating option
adamviktora fa9a1af
fix(SelectTypeahead template): rename prop back to shouldFocusFirstIt…
adamviktora File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
fix(SelectTypeaheadCreatable example): changes based on SelectTypeahead
commit 87fbdece7e11ed8329f6d085c131783e24362665
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
When selecting a creatable option for a single select, the new option is created + selected, but the menu stays open. I think I'd expect the menu to close after selection, similar to selecting an already existing option.
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.
Yeah, I know, it was intentional. I thought it might be better for user to see the option was actually created, so I kept the menu open. But I was hesitating on this one. I'll change that so it is consistent with selecting other options.
It is not that much important here, but for the templates, we might want to have this behavior customizable with props.
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.
Ah gotcha! With that context if we did want to keep the menu open, it'd be worth adding some verbiage in the example description just to state that. Something like "Unlike other typeahead examples, this example keeps the select menu open after selecting a creatable option in order to show that the item was created." Bascially just so consumers don't think we're recommending that this be the behavior they implement or something (though if they need that behavior they of course can keep it).
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.
OK, good idea. Anyway, I won't change that back anymore, the closing after creation seems more convenient.