Skip to content
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 template #10235

Merged
merged 15 commits into from
May 2, 2024

Conversation

adamviktora
Copy link
Contributor

@adamviktora adamviktora commented Apr 2, 2024

What: Closes #10113

Based on changes from #10207. For this PR, only the last 2 commits: bb7e7f8 dacd40d should be reviewed

TODO: add tests (possibly as a followup, once the changes from this PR are confirmed)

@patternfly-build
Copy link
Contributor

patternfly-build commented Apr 2, 2024

Copy link
Contributor

@wise-king-sullyman wise-king-sullyman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking great 💪

I have a few comments, and also a request for unit tests, though I wouldn't block over them if you'd like to just make a followup issue for that.

EDIT: I see now that you already have adding tests on your todo list 😅 🤦

- new changes were done also based on SelectTypeahead example updates (patternfly#10207)
Copy link
Contributor

@wise-king-sullyman wise-king-sullyman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

Copy link
Contributor

@kmcfaul kmcfaul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. We may want to add a prop to let the user customize the filter function, but the current filtering is likely sufficient for a basic typeahead so not a blocker.

@tlabaj tlabaj merged commit c7d2e9f into patternfly:main May 2, 2024
13 checks passed
kmcfaul pushed a commit to kmcfaul/patternfly-react that referenced this pull request Jun 27, 2024
* fix(SelectTypeahead example): make "no results" option aria-disabled

* fix(SelectTypeahead example): don't close the menu on input click when there is text

* fix(SelectTypeahead example): remove visual focus on item after closing the menu

Prevents situation where we open the menu via focusing on the toggle arrow and clicking enter -- then two items can have focus styling, which is not ideal.

* fix(SelectTypeahead example): remove check icon from the selected option when input text changes

* fix(SelectTypeahead example): rename example

* feat(Select): add prop to opt out of focusing first menu item on open

Flag prop shouldFocusFirstMenuItemOnOpen has been added, because of typeahead select, which should keep focus on the input.

* refactor(SelectTypeahead example): adaption on first menu item focused

* feat(MenuToggle): make typeahead toggle button not focusable

* fix(SelectTypeahead example): focus input after toggle button click

* feat(SelectTypeahead example): change the focused item on hover

* fix(SelectTypeahead example): don't focus on first item after tabbing

* feat(Select): add typeahead select template

* fix(SelectTypeahead): address PR review

- new changes were done also based on SelectTypeahead example updates (patternfly#10207)

* fix(SelectTypeahead template): call onToggle every time menu opens/closes

* refactor(SelectTypeahead template)
dlabaj pushed a commit that referenced this pull request Jul 2, 2024
* feat(Select): Typeahead template (#10235)

* fix(SelectTypeahead example): make "no results" option aria-disabled

* fix(SelectTypeahead example): don't close the menu on input click when there is text

* fix(SelectTypeahead example): remove visual focus on item after closing the menu

Prevents situation where we open the menu via focusing on the toggle arrow and clicking enter -- then two items can have focus styling, which is not ideal.

* fix(SelectTypeahead example): remove check icon from the selected option when input text changes

* fix(SelectTypeahead example): rename example

* feat(Select): add prop to opt out of focusing first menu item on open

Flag prop shouldFocusFirstMenuItemOnOpen has been added, because of typeahead select, which should keep focus on the input.

* refactor(SelectTypeahead example): adaption on first menu item focused

* feat(MenuToggle): make typeahead toggle button not focusable

* fix(SelectTypeahead example): focus input after toggle button click

* feat(SelectTypeahead example): change the focused item on hover

* fix(SelectTypeahead example): don't focus on first item after tabbing

* feat(Select): add typeahead select template

* fix(SelectTypeahead): address PR review

- new changes were done also based on SelectTypeahead example updates (#10207)

* fix(SelectTypeahead template): call onToggle every time menu opens/closes

* refactor(SelectTypeahead template)

* feat(Select): Typeahead example (#10207)

* refactor(Select): rename shouldFocusFirstMenuItemOnOpen

* feat(SelectTypeahead example): better arrow up/down keys handling

- does not apply visual focus on the first menu option
- handles disabled options
- opens menu on pressing up/down arrow keys

* feat(SelectTypeahead example): don't close menu on clicking clear button when open

* refactor(SelectTypeahead example)

* refactor(SelectTypeahead example)

* fix(SelectTypeaheadCreatable example): changes based on SelectTypeahead

* fix(SelectMultiTypeahead example): changes based on SelectTypeahead

* fix(SelectTypeaheadCreatable example): don't show create option if that exact option exists

* fix(SelectMultiTypeaheadCreatable): changes based on SelectTypeahead

* fix(SelectMultiTypeaheadCheckbox): changes based on SelectTypeahead

* fix(SelectTypeaheadCreatable): close menu after creating option

* fix(SelectTypeahead template): rename prop back to shouldFocusFirstItemOnOpen

* feat(Dropdown): Added simple template (#10308)

* feat(Dropdown): Added simple template

* Added tests

* Added imports to example file

* Additional fixes for docs fail

* Updated import name

* Added additional tests

* feat(templates): toggle props & improvements (#10473)

* feat(templates): toggle props & improvements

* remove toggleContent from typeahead template

* update template names

* update tests

* added SimpleSelect tests

* fix yarnlock

* fix(): update demo-app version and snap

---------

Co-authored-by: adamviktora <84135613+adamviktora@users.noreply.github.com>
Co-authored-by: Eric Olkowski <70952936+thatblindgeye@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create TypeaheadSelect template & example
5 participants