-
-
Notifications
You must be signed in to change notification settings - Fork 206
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
Feat: use stacks in useEscapeKeydown
& introduce escapeBehavior
prop
#1142
Merged
TGlide
merged 40 commits into
melt-ui:develop
from
anatolzak:fix/escape-keydown-nested-elements
Apr 21, 2024
Merged
Feat: use stacks in useEscapeKeydown
& introduce escapeBehavior
prop
#1142
TGlide
merged 40 commits into
melt-ui:develop
from
anatolzak:fix/escape-keydown-nested-elements
Apr 21, 2024
Conversation
This file contains 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
🦋 Changeset detectedLatest commit: 63d3110 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
anatolzak
changed the title
fix(escape-keydown) correctly handling escape keydown with nested elements
fix(escape-keydown) handle correctly escape keydown with nested elements
Mar 30, 2024
…able updating `escapeBehavior` without losing position in stack
anatolzak
changed the title
fix(escape-keydown) handle correctly escape keydown with nested elements
feat(escape-keydown): use stacks & introduce Apr 1, 2024
escapeBehavior
anatolzak
changed the title
feat(escape-keydown): use stacks & introduce
feat(escape-keydown): use stacks & introduce Apr 1, 2024
escapeBehavior
escapeBehavior
prop
…therwise-ignore`
…`defer-otherwise-ignore`
…e-close` and `defer-otherwise-ignore`
…`defer-otherwise-close` and `defer-otherwise-ignore`
…defer-otherwise-ignore`
anatolzak
changed the title
feat(escape-keydown): use stacks & introduce
Feat: use stacks in Apr 5, 2024
escapeBehavior
propuseEscapeKeydown
& introduce escapeBehavior
prop
built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
|
TGlide
approved these changes
Apr 21, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dude. this is amazing! Such a comprehensive PR. Thank you!
TGlide
approved these changes
Apr 21, 2024
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
closes #1141, #1143
This PR fixes how we handle escape presses when dealing with multiple nested floating elements by maintaining a stack of the open elements. This was part of #1080 but I realized that it changed too many things and would make it unnecessarily difficult to review.
Here is a video showing nested floating elements and outside escape presses only closing the top-most floating element.
Screen.Recording.2024-03-15.at.4.57.33.PM.mp4
Previously we added
closeOnEscape
to the dependencies in the effect of every builder that was interested in the escape behavior. This breaks as soon as we use stacks, since updatingcloseOnEscape
while an element is open removes the element from stack and then adds it to the top of the stack. To prevent that, this PR allows passing thecloseOnEscape
store touseEscapeKeydown
which therefore allows updatingcloseOnEscape
without reseting the position of the element in the stack. I added tests for this as well.Now that we use a stack to keep track of open elements that are listening to the escape presses, I had to remove a lot of extra keydown event listeners that we had in many builders that would interfere when using multiple nested elements.
This PR also replaces the
closeOnEscape
builder prop withescapeBehavior
which provides even further fine-grained control over the escape behavior in builders.Previously when dealing with multiple nested floating elements and passing
closeOnEscape: false
to a child element, when the escape key was pressed, both the parent and the child would close. I would expect neither the parent nor the child to close since I explicitly "asked" the child not to close on escape. So previously there was no way to force an element to stay open when escape is pressed without triggering the parent to close as well.To allow more fine-grained behaviors, we now introduce the
escapeBehavior
prop:close
: Closes the element immediately.ignore
: Prevents the element from closing and also blocks the parent element from closing in response to the Escape key.defer-otherwise-close
: Delegates the action to the parent element. If no parent is found, it closes the element.defer-otherwise-ignore
: Delegates the action to the parent element. If no parent is found, nothing is done.Here are videos showing the behavior of adding different
escapeBehavior
values to a popover that is nested inside a dialog. The dialog'sescapeBehavior
isclose
.Popover
escapeBehavior: 'close'
Screen.Recording.2024-03-31.at.5.51.58.PM.mov
Popover
escapeBehavior: 'ignore'
Screen.Recording.2024-03-31.at.5.52.21.PM.mov
Popover
escapeBehavior: 'defer-otherwise-ignore'
Untitled.mov
Popover
escapeBehavior: 'defer-otherwise-close'
Screen.Recording.2024-04-05.at.5.21.40.PM.mov
Popover
escapeBehavior: 'defer-otherwise-close'
mounted as a single elementScreen.Recording.2024-04-05.at.5.22.04.PM.mov