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.

Sign up for GitHub

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

Jump to bottom

Editable Combobox With List Autocomplete: Remove "onBlur" events. #1699

Merged
merged 6 commits into from
Feb 21, 2021
Merged

Editable Combobox With List Autocomplete: Remove "onBlur" events. #1699

merged 6 commits into from
Feb 21, 2021

Conversation

spectranaut
Copy link
Contributor

@spectranaut spectranaut commented Dec 22, 2020 •
edited
Loading

Adding the change request in this comment: #1619 (comment)

In summary:

  • Removed the "on blur" event which causes the listbox to close every time the textbox loses focus
  • Close the listbox every time there is a click outside of the listbox or text box
  • There was a request to close the listbox on tab, this is already supported

Preview

@mcking65 mcking65 mentioned this pull request Dec 22, 2020
Closed
@spectranaut
Copy link
Contributor Author

fixed merge conflicts!

@useit-consulting
Copy link

I've tested the preview with VoiceOver on an iPhone SE with iOS 14.2 without issues 👍

jongund
jongund approved these changes Jan 27, 2021
View reviewed changes
Copy link
Contributor

@jongund jongund left a comment

Choose a reason for hiding this comment

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

@mcking65
I made a small change to also include testing the button in the new mousedown event. and pushed the change to the branch Otherwise the coding changes to not use the blur event look good to me.

carmacleod
carmacleod approved these changes Feb 3, 2021
View reviewed changes
Copy link
Contributor

@carmacleod carmacleod left a comment

Choose a reason for hiding this comment

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

+1

Looks good, @spectranaut - thanks!

@mcking65 mcking65 linked an issue Feb 21, 2021 that may be closed by this pull request
iOS VoiceOver issue in Editable Combobox With List Autocomplete Example #1619
Closed
@mcking65 mcking65 merged commit 2a80944 into master Feb 21, 2021
oliviertassinari
oliviertassinari reviewed Feb 21, 2021
View reviewed changes
examples/combobox/js/combobox-autocomplete.js
this.comboboxNode.addEventListener('blur', this.onComboboxBlur.bind(this));

document.body.addEventListener(
'mouseup',
Copy link

@oliviertassinari oliviertassinari Feb 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

One note, mouseup fire when dragging the body's scrollbar, blur doesn't.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there another issue to resolve here? Does that mean that scrolling the page unexpectedly closes the dropdown? As a screen reader user, I can't test this, so I'm not quite clear on what the problem is.

Copy link

@oliviertassinari oliviertassinari Feb 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I was raising a potential regression (not sure about the importance of it) on the UX for mouse users. I'm not aware of any impact on screen readers.

I can't reproduce the issue on https://raw.githack.com/w3c/aria-practices/combobox-VOMac-fix/examples/combobox/combobox-autocomplete-list.html.

@mcking65 mcking65 deleted the combobox-VOMac-fix branch February 21, 2021 18:54
@mcking65
Copy link
Contributor

@jongund @smhigley @carmacleod

Could one of you investigate to see if we have a regression here?

@useit-consulting useit-consulting mentioned this pull request Feb 22, 2021
Open
2 tasks
@jongund
Copy link
Contributor

jongund commented Feb 22, 2021

@mcking65
I can look at it, maybe we need to switch this example to use pointer events.

Jon

@jongund jongund self-assigned this Feb 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcking65 mcking65 mcking65 left review comments

@oliviertassinari oliviertassinari oliviertassinari left review comments

@carmacleod carmacleod carmacleod approved these changes

@jongund jongund jongund approved these changes

Assignees

@jongund jongund

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

iOS VoiceOver issue in Editable Combobox With List Autocomplete Example
6 participants
@spectranaut @useit-consulting @mcking65 @jongund @oliviertassinari @carmacleod