feat(Select): Typeahead example #10207

adamviktora · 2024-03-27T13:31:05Z

What: Closes #10180

introduces new prop shouldFocusFirstItemOnOpen in Select, because typeahead would auto focus on the first item (instead of keeping the focus on the input)
adds tabindex={-1} to the MenuToggle arrow button for typeahead variant to prevent being able to use typeahead as a classic select and focus items (I wonder if this might break some consumer's tests and we should warn them?)

Additional issues:

hover on MenuItem and keep the cursor there, then move in the menu via arrows up/down -> the originally hovered item will still have the hovered styling (which is not ideal when the hover and focus styling looks the same)

patternfly-build · 2024-03-27T13:36:14Z

Preview: https://patternfly-react-pr-10207.surge.sh

A11y report: https://patternfly-react-pr-10207-a11y.surge.sh

thatblindgeye

hover on MenuItem and keep the cursor there, then move in the menu via arrows up/down -> the originally hovered item will still have the hovered styling (which is not ideal when the hover and focus styling looks the same)

This may be more on the user rather than it being something for us to resolve.

thatblindgeye · 2024-04-15T14:15:07Z

packages/react-core/src/components/MenuToggle/MenuToggle.tsx

@@ -97,6 +97,7 @@ class MenuToggleBase extends React.Component<MenuToggleProps> {
            aria-expanded={isExpanded}
            onClick={onClick}
            aria-label={ariaLabel || 'Menu toggle'}
+            tabIndex={-1}


Was the main issue with being able to focus and click the toggle via keyboard the focus/styling issue?

Just wondering if it'd be confusing that this toggle can be interacted with via mouse and keyboard when using a screen reader, but not via keyboard only.

The main issue was that it focused the first menu item after focusing and clicking the toggle with keyboard, so then user is not able to write text to the input. We always want to have the input focused I think.

I am checking the w3 combobox example for reference: https://www.w3.org/WAI/ARIA/apg/patterns/combobox/examples/combobox-autocomplete-list/ and the toggle button also has a tabindex="-1"

We also might change the toggle icon to arrow-up when the menu is expanded.

Sounds good, we can keep this in and see if anyone brings it up and revisit it then if so.

Changing the toggle icon would probably be worth a discussion since it'd have to include updating anywhere else we're using icons in a similar fashion, as well as making sure we have documentation about consumers needing to do the same if they're doing something more custom.

packages/react-core/src/components/Select/Select.tsx

thatblindgeye

Had more of a nit below that would be great to get others' feedback on, but otherwise things are looking nice. If we'd need to also update other Typeahead examples I think it'd be worth doing so in this PR rather than needing to do a followup for something we're already starting to fix

thatblindgeye · 2024-04-22T12:55:30Z

packages/react-core/src/components/Select/examples/SelectTypeahead.tsx

        ];
+        resetActiveAndFocusedItem();


This isn't a blocker and more of a nit, but it is odd what you had mentioned about the focus styling for the "no results" option.

We could replace this reset... call and instead setActiveAndFocusedItem to keep focus on the first item regardless. Or we could maybe try to combine the 2 useEffects into a single one to better ensure focus never gets placed on a "no results" option.

Another option is to just not auto place the "focus" on the first item, which is something that a W3C combobox example is doing. Which that actually makes sense when testing with VoiceOver. As I type into the input, I may not get any indication that the first item is "focused", so if I press down arrow to traverse the list it sounds like I'm "starting" on the second item. Sometimes I can hear VO saying "[first item] not selected...", but sometimes it may not do that or it may get cutoff as it announces what I've typed into the input. Preventing the "autofocus" would mean pressing an arrow key brings me to the expected first (or last, if Up arrow is pressed) item. Curious what others think, though.

I really like the other option you are mentioning about not auto focusing the first option. As you say, it makes sense with the voice over, and it would also solve the unwanted focusing of the "no results" option.

If we would go with this option, we should then prevent selecting the first option on pressing "enter" - it should only open/close the menu.

About updating other examples - I agree with that, I'll update them once we agree on the final version of the Typeahead behaviour.

Eric and I were talking about this earlier, and I agree with both of you that it would probably make sense to not auto focus the first option and just shift "focus" to the first option when the user presses the arrow keys (which I believe is what the W3C combobox example is doing). This would probably help improve the screen reader experience.

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

thatblindgeye

This is looking good 💪🏼 We can see during retro today if we want to try and get this in ASAP or not, but if not we should update other typeahead examples as part of this PR.

thatblindgeye · 2024-04-29T17:47:50Z

packages/react-core/src/components/Select/examples/SelectTypeaheadCreatable.tsx


+  const onSelect = (_event: React.MouseEvent<Element, MouseEvent> | undefined, value: string | number | undefined) => {


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.

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.

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).

OK, good idea. Anyway, I won't change that back anymore, the closing after creation seems more convenient.

* 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)

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

…ton when open

…at exact option exists

…emOnOpen

adamviktora · 2024-05-06T08:17:47Z

I renamed the prop shouldFocusFirstMenuItemOnOpen back to shouldFocusFirstItemOnOpen so the name is not that long.

It was unfortunately merged with the longer name in this PR: #10235, but as it was not released yet, I think it is safe to rename it back without it being a breaking change.

* 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)

* 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(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>

adamviktora mentioned this pull request Mar 27, 2024

Review: Select Typeahead example #10208

Closed

adamviktora marked this pull request as ready for review March 27, 2024 14:07

adamviktora marked this pull request as draft March 27, 2024 15:39

adamviktora marked this pull request as ready for review April 2, 2024 10:01

adamviktora changed the title ~~docs(Select): Typeahead example~~ feat(Select): Typeahead example Apr 2, 2024

adamviktora requested review from thatblindgeye and kmcfaul April 2, 2024 10:02

adamviktora mentioned this pull request Apr 2, 2024

feat(Select): Typeahead template #10235

Merged

thatblindgeye reviewed Apr 15, 2024

View reviewed changes

adamviktora force-pushed the select-typeahead-example-2 branch from 3ab72d1 to 9800074 Compare April 16, 2024 08:14

thatblindgeye reviewed Apr 22, 2024

View reviewed changes

adamviktora added a commit to adamviktora/patternfly-react that referenced this pull request Apr 23, 2024

fix(SelectTypeahead): address PR review

dacd40d

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

thatblindgeye reviewed Apr 24, 2024

View reviewed changes

thatblindgeye reviewed Apr 29, 2024

View reviewed changes

thatblindgeye approved these changes Apr 30, 2024

View reviewed changes

adamviktora added 12 commits May 6, 2024 09:58

refactor(Select): rename shouldFocusFirstMenuItemOnOpen

d0afb7d

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

674ee20

- 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 but…

82bb47b

…ton when open

refactor(SelectTypeahead example)

85c6c35

refactor(SelectTypeahead example)

9ba9f22

fix(SelectTypeaheadCreatable example): changes based on SelectTypeahead

87fbdec

fix(SelectMultiTypeahead example): changes based on SelectTypeahead

6e8ee52

fix(SelectTypeaheadCreatable example): don't show create option if th…

da0179a

…at exact option exists

fix(SelectMultiTypeaheadCreatable): changes based on SelectTypeahead

f34d61c

fix(SelectMultiTypeaheadCheckbox): changes based on SelectTypeahead

2848912

fix(SelectTypeaheadCreatable): close menu after creating option

87121d7

fix(SelectTypeahead template): rename prop back to shouldFocusFirstIt…

fa9a1af

…emOnOpen

adamviktora force-pushed the select-typeahead-example-2 branch from f2a551e to fa9a1af Compare May 6, 2024 08:09

kmcfaul approved these changes May 8, 2024

View reviewed changes

tlabaj merged commit 2d19e26 into patternfly:main May 8, 2024
13 checks passed

LukasStordeur mentioned this pull request May 22, 2024

Update Patternfly React Core Dependency Q3 inmanta/web-console#5757

Open

tlabaj mentioned this pull request Jun 4, 2024

Bug - Select - The first time you click on a filter input field, it does not have focus #10333

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(Select): Typeahead example #10207

feat(Select): Typeahead example #10207

adamviktora commented Mar 27, 2024 •

edited

Loading

patternfly-build commented Mar 27, 2024 •

edited

Loading

thatblindgeye left a comment

thatblindgeye Apr 15, 2024

adamviktora Apr 16, 2024 •

edited

Loading

thatblindgeye Apr 16, 2024

thatblindgeye left a comment

thatblindgeye Apr 22, 2024

adamviktora Apr 22, 2024

jessiehuff Apr 22, 2024

thatblindgeye left a comment

thatblindgeye Apr 29, 2024

adamviktora Apr 30, 2024

thatblindgeye Apr 30, 2024

adamviktora Apr 30, 2024

adamviktora commented May 6, 2024


		const onSelect = (_event: React.MouseEvent<Element, MouseEvent> \| undefined, value: string \| number \| undefined) => {

feat(Select): Typeahead example #10207

feat(Select): Typeahead example #10207

Conversation

adamviktora commented Mar 27, 2024 • edited Loading

patternfly-build commented Mar 27, 2024 • edited Loading

thatblindgeye left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamviktora Apr 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thatblindgeye left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thatblindgeye left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamviktora commented May 6, 2024

adamviktora commented Mar 27, 2024 •

edited

Loading

patternfly-build commented Mar 27, 2024 •

edited

Loading

adamviktora Apr 16, 2024 •

edited

Loading