Skip to content

Comments

[EuiComboBox] Blur upon selection when singleSelection is defined#7279

Closed
nickofthyme wants to merge 1 commit intoelastic:mainfrom
nickofthyme:blur-single-select-cb
Closed

[EuiComboBox] Blur upon selection when singleSelection is defined#7279
nickofthyme wants to merge 1 commit intoelastic:mainfrom
nickofthyme:blur-single-select-cb

Conversation

@nickofthyme
Copy link
Contributor

@nickofthyme nickofthyme commented Oct 11, 2023 •
edited
Loading

Summary

This PR changes the blur behavior of the EuiComboBox for single selections.

Now when singleSelection is set, either asPlainText or not - Upon selection of a value the refocusing of the EuiComboBoxInput is prevented effectively blurring the EuiComboBox.

Fixes #7169

Before

Screen Recording 2023-10-11 at 12 13 17 PM

After

Screen Recording 2023-10-11 at 12 11 07 PM

Context

I did not implement a blur and focus method on the EuiComboBox itself as I suggested in the issue, because I think this type of change should be applied to all custom input elements and not just the EuiComboBox.

QA

General checklist

  • Browser QA
    • Checked in both light and dark modes
    • Checked in mobile
    • Checked in Chrome, Safari, Edge, and Firefox
    • Checked for accessibility including keyboard-only and screenreader modes
  • Docs site QA
  • Code quality checklist
  • Release checklist
    • A changelog entry exists and is marked appropriately.
    • If applicable, added the breaking change issue label (and filled out the breaking change checklist)
  • Designer checklist
    • Updated the Figma library counterpart

@nickofthyme nickofthyme requested a review from a team as a code owner October 11, 2023 19:14
@nickofthyme nickofthyme changed the title feat: blur comboBox upon single selection [EuiComboBox] Blur upon selection when singleSelection is defined Oct 11, 2023
@nickofthyme nickofthyme mentioned this pull request Oct 11, 2023
Closed
@kibanamachine
Copy link

kibanamachine commented Oct 11, 2023

Preview staging links for this PR:

@elasticmachine
Copy link
Collaborator

elasticmachine commented Oct 11, 2023

💚 Build Succeeded

cee-chen
cee-chen suggested changes Oct 12, 2023
View reviewed changes
Copy link
Contributor

@cee-chen cee-chen left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Unfortunately, this is a no-go from an accessibility point of view. Blurring the input strands focus/kicks the active element all the way back to on the <body>, which completely throws off screen reader and keyboard user UX.

It's not clear to me what the goal here is of blocking re-focus on the combobox after single selection. Why exactly is that a problem?

CC @1Copenut for accessibility expertise

@nickofthyme
Copy link
Contributor Author

nickofthyme commented Oct 12, 2023

Yeah this came up in elastic/kibana#166024 (comment)

Going back to your proposal, is it possible to automatically remove focus on an input after a user has made their selection (whenever the input only allows for single selection)

Which I agree, it's strange for a single select element to re-focus on itself (specifically the search input), the only reason to refocus would be to add more values.

Blurring the input strands focus/kicks the active element all the way back to on the <body>

Is it possible to fix this by focusing the containing element of the EuiComboBox itself and not the nested <input>?

In any case happy to close if not desirable

1Copenut
1Copenut reviewed Oct 12, 2023
View reviewed changes
Copy link
Contributor

@1Copenut 1Copenut left a comment

Choose a reason for hiding this comment

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

Cee asked me to review this PR, so I ran through it with four input forms. Assume screen readers were paired with preferred browsers.

  1. Keyboard only
  2. Mouse only
  3. Keyboard + screen reader (VoiceOver, NVDA, JAWS)
  4. Mouse + screen reader (VoiceOver, NVDA, JAWS)

Cee called out the change stranding focus on the <body> tag for mouse users. This is a different experience from keyboard users where the input retains focus after a selection is made. That break in UX is made worse for screen reader users who navigate with a mouse because the expectation would be the combo box still have focus.

If users try to use the arrow keys to make a quick change like they would a standard <select>, they'll scroll the page instead.

I agree with Nick that it'd be excellent to drop the cursor when a selection is made, more in keeping with standard select menus. I would also say if we can't have that and return focus to the combo box when a selection is made, we should leave the component working as it does on production.

@cee-chen
Copy link
Contributor

cee-chen commented Oct 16, 2023 •
edited
Loading

I agree with Nick that it'd be excellent to drop the cursor when a selection is made, more in keeping with standard select menus.

Standard select menus don't have a search, so I don't think this is 1:1 UX. I do see WAI-ARIA's example combobox w/ autocomplete does remove the caret on select however: https://www.w3.org/WAI/ARIA/apg/patterns/combobox/examples/combobox-autocomplete-list/

What's interesting is that the above example does not remove the caret on click for keyboard users (selection made via up/down arrow and enter keys) but does remove the caret/input focus for mouse users. I really don't love the idea of making event exceptions like that however, it feels like it'll make our JS code more convoluted and harder to test.

If the flashing caret is all that's "bothering" people in terms of UX, I wonder if we could find a visual workaround via CSS instead where we simply visually hide the caret but leave focus on the input. It looks like we could use caret-color: transparent (MDN link) to do so, and conditionally apply the style if a single selection has been made. What do folks think?

@nickofthyme
Copy link
Contributor Author

nickofthyme commented Oct 17, 2023

Standard select menus don't have a search

Yeah very true, this is really a combination of the select, multi-select (w/ search) and autocomplete all in one.

I think it would be helpful to improve the focus state of the comboBox to match that of similar input components like the EuiFieldText. With the EuiFieldText, the overall input UI is focused (blue underline) anytime the user's cursor is inside until tabbing or clicking away the focus.

Screen Recording 2023-10-16 at 07 29 42 PM

With the EuiComboBox the inner input is focused (cursor flashing) but the overall input container that the user sees is not focused (blue underline) unless the list is open. To me this is inconsistent behavior to not focus whenever the inner input is focused.

Screen Recording 2023-10-16 at 07 38 53 PM

The only reason I mention this inconsistency is to provide an alternate to blurring the entire component, as currently done in the PR, and instead blur the focus of the input to the parent wrapper component (. euiComboBox__inputWrap ). This seems similar to how W3 does it in the example your shared, maintaining the focus of the entire element until it is blurred.

In the end, this was simply an observation while reviewing a PR in kibana that I felt was a little strange, which was independently raised by others without even eliciting their input on the matter.

All that said - I see this has differing ideas and I'm fine closing this PR, I also don't see a point in hiding the cursor, as an alternative.

@nickofthyme nickofthyme deleted the blur-single-select-cb branch October 17, 2023 02:51
@cee-chen cee-chen mentioned this pull request Nov 16, 2023
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@1Copenut 1Copenut 1Copenut left review comments

@cee-chen cee-chen cee-chen requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[EuiComboBox] Improve blur from single selection

5 participants

@nickofthyme @kibanamachine @elasticmachine @cee-chen @1Copenut