react-native-hold-menu icon indicating copy to clipboard operation
react-native-hold-menu copied to clipboard

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

Open NoodleOfDeath opened this issue 2 years ago • 3 comments

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}`,
 }] }>

https://github.com/enesozturk/react-native-hold-menu/assets/14790443/c0643c27-89a2-4be0-b0de-06588bf5738a

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 avatar Jul 03 '23 22:07 NoodleOfDeath

~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 avatar Jul 04 '23 13:07 NoodleOfDeath

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 avatar Jul 04 '23 13:07 NoodleOfDeath

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!

NoodleOfDeath avatar Jul 04 '23 13:07 NoodleOfDeath