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

Fix nested listbox selection behavior #851

Merged
merged 2 commits into from Jan 25, 2024
Merged

Conversation

henryB99
Copy link
Collaborator

This PR fixes a problem where a popover would close when selecting a listbox item from a nested listbox.

When selecting an item from a nested listbox, the click event's target property differs depending on the OS the browser is running on. While the browser on MacOS sets the target to the actual element I clicked on, on Windows it sets the target to another element (in my case this was the portal root element).
This behavior seems to be browser-independent, but I only tested this for Edge and Safari on MacOS and Edge and Chrome on Windows.
This PR avoids this problem altogether by changing the event the listbox item listens to from mousedown to click. This way, the event listener registered by the closeOnDismiss() method does not run, because the event gets correctly cancelled by the listbox item. Before, the listbox item only cancelled the mousedown event triggered by the click, but not the click event.

The tests are passing on both MacOS and Windows.

This PR also adds an event listener to elements using OpenClose functionality, so that non-HTMLButtonElements behave identical to actual HTMLButtonElements.

@Lysander
Copy link
Collaborator

Have you tested this also with touchpads? It might be that the mousedown has been intentionally used to support the latter.

@Lysander Lysander added this to the 1.0-RC16 milestone Jan 25, 2024
@Lysander Lysander added the headless All about headless components and foundations label Jan 25, 2024
@Lysander Lysander assigned Lysander and henryB99 and unassigned Lysander Jan 25, 2024
@henryB99
Copy link
Collaborator Author

henryB99 commented Jan 25, 2024

MDN says that click is triggerd by touch devices. Also, the spec says that using click is encouraged over other mouse events such as mousedown for implementing custom ui elements so I think we are safe here.

@henryB99 henryB99 merged commit 6aa2c1e into master Jan 25, 2024
2 checks passed
@Lysander Lysander deleted the henry/fix-nested-listbox branch January 25, 2024 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
headless All about headless components and foundations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants