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

Select-only Combobox Example: Fix scroll event listener bug #2723

Merged

Conversation

ariellalgilmore
Copy link
Contributor

@ariellalgilmore ariellalgilmore commented Jun 9, 2023

Fixes #2719.
Select only scrollbar would close when user would try to click on it.
worked on with @andreancardona 🎉

Preview fixed Select-Only Combobox Example | APG | WAI | W3C

Review checklist

Reviewers: To learn what needs to be covered by each review, Follow the link for the type of review to which you are assigned.


WAI Preview Link (Last built on Tue, 25 Jul 2023 22:54:47 GMT).

@mcking65
Copy link
Contributor

@ccanash please assign someone to determine why the preview failed to build andacquire a functional preview link.

@mcking65 mcking65 added bug Code defects; not for inaccurate prose Example Page Related to a page containing an example implementation of a pattern labels Jun 10, 2023
@mcking65 mcking65 added this to the H1/2023 Content Updates milestone Jun 10, 2023
@mcking65 mcking65 changed the title fix(select-only): example scroll event listener Select-only Combobox Example: Fix scroll event listener bug Jun 10, 2023
@howard-e
Copy link
Contributor

howard-e commented Jun 13, 2023

@ariellalgilmore @mcking65 this was related to w3c/wai-aria-practices#220 not being merged when #2670 was deployed.

I've re-ran the action and the preview link is now available in the top comment.

@andreancardona andreancardona marked this pull request as ready for review June 13, 2023 19:02
@css-meeting-bot
Copy link
Member

The ARIA Authoring Practices (APG) Task Force just discussed PR 2723: Select-only Combobox Example: Fix scroll event listener bug by ariellalgilmore.

The full IRC log of that discussion <jugglinmike> subtopic: PR 2723: Select-only Combobox Example: Fix scroll event listener bug by ariellalgilmore
<jugglinmike> github: https://github.com//pull/2723
<jugglinmike> Matt_King: This is ready for review; did we assign a reviewer last week?
<jugglinmike> andrea_cardona: We asked Valerie to review, and she said that she could
<jugglinmike> Matt_King: It's awesome to see spectranaut back in APG stuff

Copy link
Contributor

@spectranaut spectranaut left a comment

Choose a reason for hiding this comment

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

thanks so much for talking this on!! unfortunately... it looks like you turn on "ignoreBlur" permanently for the element, so if you click the scroll bar to scroll, the combobox will no longer close when focus is lost.

To replicate:

  1. Open the combobox
  2. Click the scrollbar, use it to scroll
  3. Click outside the combobox, doesn't close

I'm not totally sure how I would fix this... skipping the blur entirely (except for background clicks, as originally proposed) will also keep the combobox from closing when keyboard focus is moved away.

@andreancardona andreancardona marked this pull request as draft June 27, 2023 17:11
@andreancardona
Copy link
Contributor

@spectranaut converting to draft while we still look into this!

@css-meeting-bot
Copy link
Member

The ARIA Authoring Practices (APG) Task Force just discussed PR 2723: Select-only Combobox Example: Fix scroll event listener bug by ariellalgilmore.

The full IRC log of that discussion <jugglinmike> subtopic: PR 2723: Select-only Combobox Example: Fix scroll event listener bug by ariellalgilmore
<jugglinmike> github: https://github.com//pull/2723
<jugglinmike> arigilmore: The fixes I've tried are causing either a keyboard issue or a mouse issue, so we've converted it back to draft for now
<jugglinmike> arigilmore: We'll take another try, but we might reach out to the team to see if anyone has other suggestions

@css-meeting-bot
Copy link
Member

The ARIA Authoring Practices (APG) Task Force just discussed PR 2723: Select-only Combobox Example: Fix scroll event listener bug by ariellalgilmore.

The full IRC log of that discussion <jugglinmike> Subtopic: PR 2723: Select-only Combobox Example: Fix scroll event listener bug by ariellalgilmore
<jugglinmike> github: https://github.com//pull/2723
<jugglinmike> arigilmore: I was working on this with Andrea, and when we tried the suggested solution, it didn't work for mouse, and our solution didn't work for keyboard
<jugglinmike> arigilmore: We were having trouble getting the "blur" effect
<jugglinmike> arigilmore: The original issue was: when someone would click on the scrollbar in the combobox, it would immediately close the combobox.
<jugglinmike> arigilmore: By fixing that, we were having trouble getting keyboard navigation to work AND mouse navigation to work when navigating in the combobox.
<jugglinmike> arigilmore: Now, you can click on the scroll bar, and it will scroll without closing. But after that, if you click outside the combobox, it doesn't close.
<jugglinmike> arigilmore: Though it does close when you tab away. It's just a problem for the case where you've clicked on the scrollbar
<jugglinmike> arigilmore: This doesn't appear to be a browser bug because the interaction works as expected on the other examples (even though those examples look quite different)
<jugglinmike> arigilmore: I don't currently understand why the other comboboxes work while this one particular combobox does not
<jugglinmike> Matt_King: Sarah wrote the original code here; they might be able to help
<jugglinmike> arigilmore: I'll reach out to them

@smhigley
Copy link
Contributor

smhigley commented Jul 25, 2023

Made a quick codepen demo-ing a fix: https://codepen.io/smhigley/pen/QWJVPWp/f733695118b87fa51dee32b52e5d6614

The approach is to not close the popup if focus moves to the listbox on blur (using event.relatedTarget), and to add a focusout listener to the listbox so that it will also close when the listbox is blurred.

This also means the ignoreBlur logic can be removed entirely, so it seems like a nice fix

@ariellalgilmore ariellalgilmore marked this pull request as ready for review July 25, 2023 21:07
@ariellalgilmore
Copy link
Contributor Author

@spectranaut updated!! Thank you @smhigley for the fix suggestions!

@mcking65 mcking65 requested a review from spectranaut August 1, 2023 18:21
Copy link
Contributor

@spectranaut spectranaut left a comment

Choose a reason for hiding this comment

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

Looks great! Sorry I missed this update :)

@mcking65 mcking65 merged commit 164188c into w3c:main Aug 8, 2023
@mcking65
Copy link
Contributor

mcking65 commented Aug 8, 2023

thank you @ariellalgilmore, @andreancardona, @smhigley, and @spectranaut!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Code defects; not for inaccurate prose Example Page Related to a page containing an example implementation of a pattern
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Select-Only Combobox Example - listbox closes when clicking on scrollbar
8 participants