Skip to content

Fix/incubator dialog fader visibility and more #2379

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

Merged
merged 4 commits into from
Dec 27, 2022

Conversation

M-i-k-e-l
Copy link
Collaborator

Description

You can look at each commit separately
Fixes for the fade view (most important is moving the withTiming to the useAnimatedStyle), the others are making the code a little better.
Do not set pointerEvents by default (allow via props) - this fixes a bug in Android where clicking on the header closes the dialog (it goes through to the fade view)

WOAUILIB-3268

Changelog

Fix Incubator.Dialog's fader view not being shown sometimes

@M-i-k-e-l M-i-k-e-l added the Important for Next Release PR that must be included in the release version label Dec 15, 2022
@M-i-k-e-l M-i-k-e-l requested a review from Inbal-Tish December 15, 2022 11:15
@@ -163,7 +164,7 @@ const Dialog = (props: DialogProps, ref: ForwardedRef<DialogImperativeMethods>)
const renderDialog = () => {
return (
<PanGestureHandler onGestureEvent={isEmpty(directions) ? undefined : panGestureEvent}>
<View pointerEvents={'box-none'} reanimated style={style} onLayout={onLayout} ref={setRef} testID={testID}>
<View {...containerProps} reanimated style={style} onLayout={onLayout} ref={setRef} testID={testID}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't removing pointerEvents={'box-none'} the fix? So not sure why the 'containerProps' are needed... Assuming that's for passing 'pointerEvents', in what scenario would the user need to pass pointerEvents={'box-none'} in the container props?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In private we have a case were we render our own header which only part of it has pointerEvents: 'box-none' - that's the use case.

From the description:

Do not set pointerEvents by default (allow via props) - this fixes a bug in Android where clicking on the header closes the dialog (it goes through to the fade view)

You can look at master in private on ActionSheet (Android only) - click on the header where there's no text

@M-i-k-e-l M-i-k-e-l requested a review from Inbal-Tish December 19, 2022 11:07
@Inbal-Tish Inbal-Tish merged commit 9425c85 into master Dec 27, 2022
@M-i-k-e-l M-i-k-e-l deleted the fix/incubator-dialog-fader-visibility-and-more branch December 27, 2022 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Important for Next Release PR that must be included in the release version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants