🐛 Refactor handle outside click for listbox - firefox issue #145
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
far-fetched
changed the title
Contributor
Author
|
@dmcnamara-eng I would like to hear your voice here, as you are main author of this component, thanks ! |
far-fetched
force-pushed
the
close-listbox-on-button-click-firefox
branch
from
77d504e to
003ea8d
Compare
dmcnamara-eng
approved these changes
Jun 1, 2022
Collaborator
There was a problem hiding this comment.
Turns out Event.composedPath is the cross browser equivalent of Event.path.
It's not quite functionally equivalent to closest() but in this case achieves the same result.
Looks good to merge 👍
Collaborator
|
LGTM! thanks for submitting this! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changed and Why ?
I noticed strange behavior using firefox for
<Listbox />component:Clicking on button when list is open – it does not close listbox:
simplescreenrecorder-2022-05-27_08.43.34.mp4
With transition, clicking on button when list is open – it closes the listbox, but to open it again you have to click twice
simplescreenrecorder-2022-05-27_08.43.54.mp4
You can check out our examples: link
Remember to use firefox
Code fix:
Problem seems to be with using non-standard property
pathon JSevent. More here. I changed a bit of logic to useclosestmethod, which supposed to be extensively compatible with browsers:After fix:
simplescreenrecorder-2022-05-27_08.48.38.mp4
simplescreenrecorder-2022-05-27_08.48.52.mp4
When executing test (using
firefox) which asserts close - it fails.PS: We already have 37 errors when executing test suite with firefox ;( I will try to fix them in following PR's !