-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Move Dialogs to Radix #5648
Move Dialogs to Radix #5648
Conversation
* Use Redix FocusTrap * force resolutions on radix libs * add focus guards * use @radix-ui/dismissable-layer for escape handling * fix banner menu keypress by using `Pressable` * add menu in dialog example to storybook --------- Co-authored-by: Samuel Newman <mozzius@protonmail.com>
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.
Beautiful
@@ -90,14 +93,11 @@ export function UserBanner({ | |||
|
|||
// setUserBanner is only passed as prop on the EditProfile component | |||
return onSelectNewBanner ? ( | |||
<EventStopper onKeyDown={false}> | |||
<EventStopper onKeyDown={true}> |
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.
Not sure we even need this anymore
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.
oh cool! I'll follow up with a PR to remove those, wanna test thoroughly first so won't do it here
@@ -10,7 +10,9 @@ import { | |||
import Animated, {FadeIn, FadeInDown} from 'react-native-reanimated' | |||
import {msg} from '@lingui/macro' | |||
import {useLingui} from '@lingui/react' | |||
import {FocusScope} from '@tamagui/focus-scope' |
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.
Can we remove this library now?
Adds Radix's
focus-scope
andfocus-guard
utilities to the dialogs and composer, so that they can compose properly with the dropdown menus. This means that dialogs and menus can now be nested, and theEscape
key will work properly.I also did the same to the composer, so now focus loops around when you get to the end
I also added nested sheet support to the menu component on native
Test plan
Dialog with menu in it
button in the Storybook (all platforms). Before these changes, this would've caused a recursion bug on firefox, so make sure that doesn't happen