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

If menu items do not have unique names keys are not updating #105

Open
NoodleOfDeath opened this issue Jul 3, 2023 · 3 comments
Open

If menu items do not have unique names keys are not updating #105

NoodleOfDeath opened this issue Jul 3, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@NoodleOfDeath
Copy link

NoodleOfDeath commented Jul 3, 2023

Describe the bug
Because this uses nanoid instead of allowing the developer to specify their own key values, sometimes the menu onPress does not update when it should.

To Reproduce
Steps to reproduce the behavior:

  1. Add HoldMenuProvider at the top of stack.
  2. Make a list of HoldItem components with items that differ only by the onPress property but have the same titles and icons for each item.
  3. yarn start --resetCache
  4. Open app. Try long pressing a few items back and forth and occasionally the onPress function will get called for the wrong item rather than the one that was long pressed.

Expected behavior
The correct onPress being called for the hold item being long pressed.

Actual behavior
Occasionally the wrong onPress is called for the corresponding hold item. Adding an item with text that is unique across all hold items fixes the problem, but I don't want to make them unique other than the onPress functions.

import React from 'react';
import {
  SafeAreaView,
  Text,
  View,
} from 'react-native';

import { HoldItem } from 'react-native-hold-menu';

function Test() {
  const titles = ['one', 'two', 'three'];
  return (
    <View style={ { rowGap: 10 } }>
      {titles.map((title) => (
        <HoldItem
          key={ title }
          items={ [{
            onPress: () => alert(title),
            text: 'share',
          }] }>
          <Text style={ {
            backgroundColor: '#eee',
            padding: 10,
          } }>
            {title}
          </Text>
        </HoldItem>
      ))}
    </View>
  );
}

export function TestScreen() {
  return (
    <SafeAreaView style={ { flex: 1 } }>
      <View style={ { padding: 24 } }>
        <Test />
      </View>
    </SafeAreaView>
  );
}

Screenshots
If applicable, add screenshots to help explain your problem.

*** Not working as expected. ***
https://github.com/enesozturk/react-native-hold-menu/assets/14790443/88583b0d-a8eb-49bc-8bf3-2b22329452f9

*** Oddly it works when make text unique for each item***

 items={ [{
  onPress: () => alert(title),
  text: `share-${title}`,
 }] }>
RPReplay_Final1688477184.MP4

Package versions

  • React: 18.2.0
  • React Native: ^0.71.8
  • React Native Reanimated: ^3.3.0
  • React Native Hold Menu: ^0.1.6
  • Expo: ^48.0.0

Additional context
Add any other context about the problem here.

@NoodleOfDeath NoodleOfDeath added the bug Something isn't working label Jul 3, 2023
@NoodleOfDeath
Copy link
Author

NoodleOfDeath commented Jul 4, 2023

Can I recommend instead of using a nanoid a timebased uuid like uuid.v1 be used? this solved my issue when i replaced nanoidi with new Date().toString()

Correction -- even using a date string doesnt change it, not a memoized version of the items. only fix is to make the titles unique as shown in screenrecordings

@NoodleOfDeath
Copy link
Author

NoodleOfDeath commented Jul 4, 2023

Can we add a key or id property t MenuItemProps? I think the react hooks are not recomputing the items because they don't have anything that differs other than the onPress function

Also, line 91 of HoldItem.tsx has no reactive dependencies, so does that mean it is only computed exactly once on mount?

@NoodleOfDeath
Copy link
Author

NoodleOfDeath commented Jul 4, 2023

A HAH! I found the culprit. Line 14 of MenuItems.tsx! Never ever use an index as a key! Adding a key property to MenuItemProps and passing that as the key for MenuItem on line 14 fixes this problem!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant