nandorojo / zeego

Menus for React (Native) done right.
https://zeego.dev
MIT License
1.45k stars 42 forks source link

[iOS] Trigger children component disappears when navigating back on react-navigation stack #11

Closed thespacemanatee closed 1 year ago

thespacemanatee commented 1 year ago

On iOS, when navigating from a screen that uses DropdownMenu to a new screen, and then going back to the previous screens, the components inside DropdownMenu.Trigger disappears forever. See below for example and repro:

Demo

https://user-images.githubusercontent.com/6837599/180454095-ef0fc707-8060-49dc-bac7-2d4790b45b4a.mov

Workaround

Set detachPreviousScreen: false on the next screen options which prevents the DropdownMenu from detaching in the first place, but may cause performance problems.

Repro

<DropdownMenu.Root>
  <DropdownMenu.Trigger>
    <ThemedChip
      key={availableOptions[0]}
      isSelected={!isDefault}
      label={capitalize(getChipLabel(chosenOption, type))}
      rightComponent={
        <ThemedIcon
          name="chevron-down"
          color={isDefault ? lightColors.primary : 'white'}
          size={16}
          style={tw('ml-1')}
        />
      }
    />
  </DropdownMenu.Trigger>
  <DropdownMenu.Content>
    {availableOptions.map(availableOption => (
      <DropdownMenu.Item
        key={`${availableOptions[0]}-${availableOption}`}
        onSelect={() => {
          onToggleOption?.(availableOption, type)
        }}
      >
        <DropdownMenu.ItemTitle>
          {capitalize(availableOption.split('_').pop())}
        </DropdownMenu.ItemTitle>
        {availableOption === chosenOption && (
          <DropdownMenu.ItemIcon iosIconName="checkmark" />
        )}
      </DropdownMenu.Item>
    ))}
  </DropdownMenu.Content>
</DropdownMenu.Root>

Dependencies

"dependencies": {
  "@react-native-masked-view/masked-view": "0.2.7",
  "@react-navigation/bottom-tabs": "^6.3.2",
  "@react-navigation/devtools": "^6.0.8",
  "@react-navigation/material-top-tabs": "^6.2.2",
  "@react-navigation/native": "^6.0.11",
  "@react-navigation/native-stack": "^6.7.0",
  "@react-navigation/stack": "^6.2.2",
  "expo": "^46.0.0-beta.6",
  "react": "18.2.0",
  "react-native": "0.69.2",
  "react-native-reanimated": "^2.9.1",
  "react-native-safe-area-context": "4.3.1",
  "react-native-screens": "~3.15.0",
  "react-native-shared-element": "^0.8.4",
  "react-navigation-shared-element": "^3.1.3",
  "zeego": "^0.4.0"
  ...
}
nandorojo commented 1 year ago

thanks for the issue! is the menu item itself triggering the navigation?

nandorojo commented 1 year ago

oh i just saw the video, never mind.

hmm, is this native stack? what if you use normal text inside the trigger, same issue?

nandorojo commented 1 year ago

https://user-images.githubusercontent.com/13172299/180456009-bf6e7928-a55d-4fc5-8ba3-cb867a32841c.MOV

works fine for me with native stack, i wonder if there’s some special thing in your trigger? do you have enableFreeze(true)?

thespacemanatee commented 1 year ago

@nandorojo thanks for the quick response! I am using the stack from react-navigation-shared-element but it did not work when I used the regular stack and native stack from rn-navigation as well, and am not using enableFreeze(true). I'll add the relevant deps to the OP, and lmk if you need more information :)

nandorojo commented 1 year ago

hmm i see. i’m not too sure about what could cause it with that stack, especially since it isn’t maintained anymore

nandorojo commented 1 year ago

if you’re using native stack, it should work…maybe try toggling enableFreeze?

thespacemanatee commented 1 year ago

Tried native stack as well as toggling enableFreeze to no avail :/ I understand if there's not much to go on here but maybe we could keep this issue up in case someone else encounters the same issue too. Not detaching the screen is an ok workaround for us atm.

nandorojo commented 1 year ago

did you try changing the trigger?

thespacemanatee commented 1 year ago

Happens with this Trigger as well:

<DropdownMenu.Trigger>
  <Text style={tw('text-white')}>Hello world</Text>
</DropdownMenu.Trigger>

https://user-images.githubusercontent.com/6837599/180464932-9da05fae-cf3c-444f-88b5-f42fed9e8138.mov

nandorojo commented 1 year ago

Ok so I'm getting a similar issue in my app of the trigger disappearing...it happens when i scroll far down a flatlist in my app. Will try to figure this out.

nandorojo commented 1 year ago

@thespacemanatee do you have fullScreenGestureEnabled by chance? I think disabling this fixes it for me.

nandorojo commented 1 year ago

Also, does it happen both if you swipe back and press the back button?

thespacemanatee commented 1 year ago

@nandorojo Unfortunately I do not have fullScreenGestureEnabled enabled and it happens both on swipe back and back button press

nandorojo commented 1 year ago

hmm got it, will keep investigating

nandorojo commented 1 year ago

Just confirming, are you on the latest version of react-native-ios-context-menu?

thespacemanatee commented 1 year ago

Just confirming, are you on the latest version of react-native-ios-context-menu?

Yup, I was and still am on the latest version.

nandorojo commented 1 year ago

maybe we could cross-post on that repo?

i also assume memoizing doesn’t help on the zeego components?

fobos531 commented 1 year ago

maybe we could cross-post on that repo?

i also assume memoizing doesn’t help on the zeego components?

This is most likely about the same issue: https://github.com/dominicstop/react-native-ios-context-menu/issues/34

I will test out the patch provided in https://github.com/dominicstop/react-native-ios-context-menu/issues/34#issuecomment-1135781594 and submit a PR if that proves to fix the issue.

nandorojo commented 1 year ago

that definitely looks right. curious to see if it fixed it.

thespacemanatee commented 1 year ago

@nandorojo looks like its fixed with the latest version of react-native-ios-context-menu!

fobos531 commented 1 year ago

@nandorojo

Confirming the comment above, looks like it works for some other use cases too! 🚀

I think this issue can be closed until further notice.