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

[DismissableLayer] Don't exit fullscreen when esc-ing out of a layer #1423

Merged
merged 1 commit into from
May 27, 2022
Merged

[DismissableLayer] Don't exit fullscreen when esc-ing out of a layer #1423

merged 1 commit into from
May 27, 2022

Conversation

cpmsmith
Copy link
Contributor

Description

Basically just call preventDefault! I’m not sure about the semantics of onDismiss being undefined, but I think if it’s undefined, we shouldn’t be preventing the default—I’m open to input on that.

Fixes #1420

@cpmsmith cpmsmith mentioned this pull request May 25, 2022
Closed
benoitgrelard
benoitgrelard approved these changes May 26, 2022
View reviewed changes
Copy link
Contributor

@benoitgrelard benoitgrelard left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!
It's all good, just needs the version tweak and then I'll merge 👍

.yarn/versions/aa7247ea.yml Outdated

declined:
- primitives
- "@radix-ui/react-hover-card"
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like a slip-up here, but HoverCard should be part of the version change too as it depends on DismissableLayer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't bump hover-card because it's not possible to open a hover card with the keyboard, but I suppose it is possible to close them. I'll update that now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! Yeah it has the same close on escape behaviour.

packages/react/dismissable-layer/src/DismissableLayer.tsx
@@ -104,7 +104,10 @@ const DismissableLayer = React.forwardRef<DismissableLayerElement, DismissableLa
const isHighestLayer = index === context.layers.size - 1;
if (!isHighestLayer) return;
onEscapeKeyDown?.(event);
if (!event.defaultPrevented) onDismiss?.();
if (!event.defaultPrevented && onDismiss) {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@cpmsmith cpmsmith force-pushed the dismissable-layer-esc-preventdefault branch from 33a57c7 to 48adf84 Compare May 27, 2022 03:52
@cpmsmith cpmsmith requested a review from benoitgrelard May 27, 2022 03:54
@benoitgrelard benoitgrelard merged commit 38070ed into radix-ui:main May 27, 2022
benoitgrelard added a commit to radix-ui/website that referenced this pull request May 27, 2022
@benoitgrelard
Document latest DismissableLayer change
77bf1db 
radix-ui/primitives#1423
@benoitgrelard benoitgrelard mentioned this pull request May 27, 2022
Merged
andy-hook pushed a commit to radix-ui/website that referenced this pull request May 30, 2022
@benoitgrelard
Document latest DismissableLayer change (#401)
aa5eb49 
radix-ui/primitives#1423
andy-hook pushed a commit to radix-ui/website that referenced this pull request May 30, 2022
@benoitgrelard @andy-hook
Document latest DismissableLayer change (#401)
78255aa 
radix-ui/primitives#1423
benoitgrelard added a commit to radix-ui/website that referenced this pull request Jun 7, 2022
@benoitgrelard
Document latest DismissableLayer change
462d789 
radix-ui/primitives#1423
andy-hook pushed a commit to radix-ui/website that referenced this pull request Jun 8, 2022
@benoitgrelard @andy-hook
Document latest DismissableLayer change
047c71c 
radix-ui/primitives#1423
benoitgrelard added a commit to radix-ui/website that referenced this pull request Jun 13, 2022
@benoitgrelard
Document latest DismissableLayer change
e7af66c 
radix-ui/primitives#1423
benoitgrelard added a commit to radix-ui/website that referenced this pull request Jun 15, 2022
@benoitgrelard
Document latest DismissableLayer change
9113dd3 
radix-ui/primitives#1423
benoitgrelard added a commit to radix-ui/website that referenced this pull request Jul 8, 2022
@benoitgrelard
Document latest DismissableLayer change
67c2948 
radix-ui/primitives#1423
andy-hook pushed a commit to radix-ui/website that referenced this pull request Jul 18, 2022
@benoitgrelard @andy-hook
Document latest DismissableLayer change
9c6cf84 
radix-ui/primitives#1423
benoitgrelard added a commit to radix-ui/website that referenced this pull request Jul 20, 2022
@benoitgrelard @andy-hook @bukinoshita
Release: Agile Halo (#368)
80c2962 
* Setup base PR

* Initial documentation updates (#369)

Catching up with merged features/fixes

* Update release note for React 18 (#371)

* Update release note for React 18

* Update data/primitives/overview/releases.mdx

Co-authored-by: Benoît Grélard <benoit.grelard@gmail.com>

Co-authored-by: Benoît Grélard <benoit.grelard@gmail.com>

* Primitives 1344 documentation (#376)

* Add new version pages

* Remove references to `forwards` in new versions

* Document NavigationMenu close on click 1347 (#380)

* Document Select fix (#381)

* Update `NavigationMenu` docs (#385)

* Document Slot fix (#386)

* Document Slot fix

* PR feedback

* Document latest menu/select updates (#387)

* Document latest menu/select updates

* Update 0.1.2.mdx

* Update 0.1.2.mdx

* Update 0.1.2.mdx

* Add context menu note to release (#388)

* Fix release note (#389)

* Document ScrollArea animatable Thumb change (#390)

* Remove enter key as per #393

* Document DismissableLayer fixes (#394)

* Catchup on demo updates (#396)

* Catchup on demo updates

* Update data/primitives/components/select/0.1.2.mdx

Co-authored-by: Andy Hook <hello@andyhook.dev>

* PR feedback

Co-authored-by: Andy Hook <hello@andyhook.dev>

* Document tabs animation (#397)

* Update file version/s

* Update doc

* Document submenu api change, `allowPinchZoom` prop move (#395)

* Document submenu changes

* Document `allowPinchZoom` prop change

* Feedback

* Document latest select fix (#398)

* Document latest select fix

* Update data/primitives/overview/releases.mdx

Co-authored-by: Andy Hook <hello@andyhook.dev>

Co-authored-by: Andy Hook <hello@andyhook.dev>

* Document latest RadioGroup change (#399)

* Document latest DismissableLayer change

radix-ui/primitives#1423

* Sync files with recent changes (#412)

* Document new Portal parts (#413)

* Document new Portal parts

* Document changes

* Update demos

* Update DeveloperExperienceSection.tsx

* Document latest ContextMenu changes (#414)

* Document latest Select changes (#415)

* Document latest Select changes

radix-ui/primitives#1459

* Add missing prop

* Document latest Portal changes (#416)

radix-ui/primitives#1463

* Parity in release branch from #417 (#419)

#417

* Document `data-*` attributes (#407)

* Setup base PR

* Initial documentation updates (#369)

Catching up with merged features/fixes

* Document `data-*` attributes

* Document `data-*`

* Fix spaces and render

* Fix data

* Document `data-*` attributes

* Document `data-*` attributes

* Add missing data attributes

* Fix minor tweaks

* Fix minor tweaks

* Fix minor tweaks

Co-authored-by: Benoît Grélard <benoit.grelard@gmail.com>

* Parity with #421 (#422)

* Document toast animation improvements (#423)

* Update release note

* Label required, update package version

* Document CSS variables (#426)

* Document CSS variables

* Change `Tooltip` CSS variable description

* remove from slider

* Fix `Accordion` CSS variable

* Remove `ScrollArea` CSS variables

* Update `transform-origin` description

* Add `Toast` CSS variables better description

* Fix `PropsTable` width

* Fix minor tweaks

* Fix minor tweaks

* Apply copy suggestions

* Fix minor tweaks

* Fix minor tweaks

* Fix minor tweaks

* Update data/primitives/components/tooltip/0.1.8.mdx

Co-authored-by: Andy Hook <hello@andyhook.dev>

Co-authored-by: Benoît Grélard <benoit.grelard@gmail.com>
Co-authored-by: Andy Hook <hello@andyhook.dev>

* Document tooltip hover changes (#431)

* Parity with #430 (#432)

* Document `allowPinchZoom` changes (#433)

* Document `allowPinchZoom` changes

* Feedback

* Release docs adjustments

* Updating versions in log to v1 (for all hands purpose)

* Copy change

* Fix typo in `onSelect` desc and release note

* Document popper changes (#437)

* Update changelog

* Document prop changes

* Update demos with props

* Update live demos

* Add missing portals

* Update deps

* Revert "Update live demos"

This reverts commit 981d20a.

* Update deps

* Update codesandbox css

* Remove portal in marketing hero demos

* Document new boundary prop

* Missing changelog + add portal back on subs

* PR feedback

* PR feedback

* Update stats (#438)

* Update release to use 1.0.0 filenames (#439)

* Shift breaking to first in change log

* Move files to 1.0.0

* Upgrade DS version (#440)

* Release refinement (#441)

* Revert announce filename and deprecate

* Update data-highlighted description

* Release date consistency

* Revert "Update data-highlighted description"

This reverts commit 30b3ae1.

* Tweak realease note order

* Update release tags to major

* Upgrade DS and primitives (#442)

Co-authored-by: Andy Hook <hello@andyhook.dev>
Co-authored-by: Bu Kinoshita <bukinoshita@gmail.com>
luisorbaiceta pushed a commit to luisorbaiceta/primitives that referenced this pull request Jul 21, 2022
@cpmsmith @luisorbaiceta
Don't exit fullscreen when esc-ing out of a DismissableLayer (radix-u…
3b7fc0f 
…i#1423)

Fixes radix-ui#1420
@cpmsmith cpmsmith mentioned this pull request Nov 2, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andy-hook andy-hook Awaiting requested review from andy-hook

@benoitgrelard benoitgrelard Awaiting requested review from benoitgrelard

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[DropdownMenu] Escape key exits fullscreen on Firefox & Safari
2 participants
@cpmsmith @benoitgrelard