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.

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

fixing modal not hiding because of the animation #107

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

nicolaslazzos
Copy link

On the hide() method, when the menuState is being setted as HIDDEN, this was triggering the _onMenuLayout() method, so the menuState was immediately being setted to ANIMATING, which was causing the modalVisible variable to be true. So, though this was an unwanted behavior, in mobile it wasn't showing, but in web, because of the modal wasn't closing, it stayed and forced you to click on the screen again so that it closed after the menu disappeared.

I simply added that when the menuState is HIDDEN, the _onMenuLayout() method ignores that trigger.

src/Menu.js Outdated Show resolved Hide resolved
@nicolaslazzos nicolaslazzos requested a review from mxck March 30, 2021 20:13
@gpsolarco
Copy link

Is there any workaround for this issue for v1.2.0 + Expo SDK42? I am facing issues getting v2.0.0 to start with Expo SDK 42 to will need to stick to v1.2.0 till the other issue gets resolved.

@mxck
Copy link
Owner

mxck commented Aug 26, 2021

@gpsolarco you got this issue on mobile?

@gpsolarco
Copy link

Both Mobile and Web.
My current RN code base is still class based but that should not be an issue I think.

@gpsolarco
Copy link

Any update on this issue? thanks!

@@ -124,6 +124,12 @@ class Menu extends React.Component {

// @@ TODO: Rework this
_hide = () => {
if (!!this.props.blockedHide) {

Choose a reason for hiding this comment

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

!! are not necessary in if statements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants