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

[android] Fixed menu list remained tappable after dismiss on android #115

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kesha-antonov
Copy link
Contributor

Hi

Fixes #76

dmytro-ivonin pushed a commit to dmytro-ivonin/react-native-hold-menu-no-expo that referenced this pull request Jan 10, 2024
Comment on lines +146 to +158
<Animated.View style={[styles.menuContainer, messageStyles]}>
<AnimatedView
intensity={100}
animatedProps={animatedProps}
style={StyleSheet.absoluteFillObject}
>
<MenuItems items={itemList} />
</Animated.View>
</AnimatedView>
<Animated.View
style={[
StyleSheet.absoluteFillObject,
styles.menuInnerContainer,
animatedInnerContainerStyle,
]}
>

Choose a reason for hiding this comment

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

what does this change do?

Choose a reason for hiding this comment

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

Comment on lines -336 to -337
zIndex: 10,
position: 'absolute',

Choose a reason for hiding this comment

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

what does this do?

Choose a reason for hiding this comment

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

I think because we have these styles already in the styles.holdItem

Comment on lines +142 to +143
if (!isVisible)
return null

Choose a reason for hiding this comment

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

Did not test it, but I guess that this will break the animation when closing the menu.
We might need to wait for the animation somehow and then remove it

Copy link
Contributor Author

@kesha-antonov kesha-antonov Jan 24, 2024

Choose a reason for hiding this comment

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

It doesn't brake, you can check it
I set state in the animation callback when it's finished

@Nasseratic
Copy link

@kesha-antonov An idea could be to use animatedProps with pointerEvents :)
I believe it can make the fix simpler

@kesha-antonov
Copy link
Contributor Author

@kesha-antonov An idea could be to use animatedProps with pointerEvents :) I believe it can make the fix simpler

Why to keep rendered views underneath and waste device's resources?

@fukemy
Copy link

fukemy commented Feb 16, 2024

Hi, I saw the lib render {children} 2 times like this:

<GestureHandler>
        <Animated.View ref={containerRef} style={containerStyle}>
          {children} => * HERE
        </Animated.View>
      </GestureHandler>

      <Portal key={key} name={key}>
        <Animated.View
          key={key}
          style={portalContainerStyle}
          animatedProps={animatedPortalProps}
        >
          <PortalOverlay />
          {children} => * HERE
        </Animated.View>
      </Portal>

So I think that make performance down to hell from here

@kesha-antonov
Copy link
Contributor Author

Hi, I saw the lib render {children} 2 times like this:

<GestureHandler>

        <Animated.View ref={containerRef} style={containerStyle}>

          {children} => * HERE

        </Animated.View>

      </GestureHandler>



      <Portal key={key} name={key}>

        <Animated.View

          key={key}

          style={portalContainerStyle}

          animatedProps={animatedPortalProps}

        >

          <PortalOverlay />

          {children} => * HERE

        </Animated.View>

      </Portal>

So I think that make performance down to hell from here

You need to address it to main code base, not my PR) I didn't add this logic

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.

Menu list items are pressable after close
4 participants