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 items don't have the key attribute, rowMap is useless #360

Closed
michaelkowalski opened this issue Oct 18, 2019 · 11 comments
Closed

If items don't have the key attribute, rowMap is useless #360

michaelkowalski opened this issue Oct 18, 2019 · 11 comments

Comments

@michaelkowalski
Copy link

michaelkowalski commented Oct 18, 2019

Hello, if the items in the list don't have the key attrubute, rowMap becomes one big merged row. I tried to use keyExtractor, but SwipeList seems to ignore it and still needs the key attr in the data prop.

The problem in my case is that I had to explicitly add the key attribute to the data prop to use .closeRow().

Is this a bug or am I doing something wrong? For me the expected behavior would be to use any data shape, a proper keyExtractor and still have the rowMap working correctly. Please correct me if I'm wrong.

@elioncho
Copy link

+1

@jemise111
Copy link
Owner

Hey @michaelkowalski @elioncho I'm trying to repro this but I'm just not seeing the same issues. I'm going to lay out everything I'm looking at and you tell me what I'm missing.

Here is my example code. I do not have keys in my data array. Instead I'm using a keyExtractor and using data.text as the key since it is unique.

Note that the log in onRowDidOpen is showing the correct key, and the rowMap which is an object where the keys are the keys from keyExtractor and the values are the SwipeRows. The row is closing on scroll and when I click "close".

Also note when I invoke closeRow I'm passing the data.item.text as the second arg since that is what I use as my unique keys.

class App extends Component {
    constructor(props) {
        super(props);
        this.state = {
            listViewData: Array(20)
                .fill('')
                .map((_, i) => ({ text: `item #${i}` })),
        };
    }

    closeRow(rowMap, rowKey) {
        if (rowMap[rowKey]) {
            rowMap[rowKey].closeRow();
        }
    }

    onRowDidOpen = (rowKey, rowMap) => {
        console.log('Row key: ', rowKey);
        console.log('Row map: ', rowMap);
    };

    render() {
        return (
            <View style={styles.container}>
                <SwipeListView
                    data={this.state.listViewData}
                    keyExtractor={item => item.text}
                    renderItem={data => (
                        <TouchableHighlight
                            onPress={() => console.log('You touched me')}
                            style={styles.rowFront}
                            underlayColor={'#AAA'}
                        >
                            <View>
                                <Text>
                                    I am {data.item.text} in a SwipeListView
                                </Text>
                            </View>
                        </TouchableHighlight>
                    )}
                    renderHiddenItem={(data, rowMap) => (
                        <View style={styles.rowBack}>
                            <Text>Left</Text>
                            <TouchableOpacity
                                style={[
                                    styles.backRightBtn,
                                    styles.backRightBtnLeft,
                                ]}
                                onPress={() =>
                                    this.closeRow(rowMap, data.item.text)
                                }
                            >
                                <Text style={styles.backTextWhite}>Close</Text>
                            </TouchableOpacity>
                            <TouchableOpacity
                                style={[
                                    styles.backRightBtn,
                                    styles.backRightBtnRight,
                                ]}
                            >

@elioncho
Copy link

elioncho commented Oct 19, 2019

I don't see anything wrong. This is what I'm personally doing.

I am trying to display an array of basic array of objects:

[{"id":20,"name":"Item 1"},{"id":2,"name":"item 2"},{"id":3,"name": "item 3"}]

    <SwipeListView
      keyExtractor={item => String(item.id)}
      data={lists}
      renderItem={data => (
        <ListItem
          title={data.item.name}
          onPress={() => {
            navigate('ListDetails', { 
              listId: data.item.id,
              listName: data.item.name
            })
          }}
        />
      )}

If I remove the keyExtractor prop and add the key property to every element of the array it works. I'm not sure what I'm doing wrong then.

@jemise111
Copy link
Owner

Hey @elioncho I'm not seeing any closeRow function here, what exactly is the issue you're trying to fix?

@elioncho
Copy link

elioncho commented Oct 21, 2019

I copied the whole component and the closeRow and deleteRow functions in the following example. The only thing I'm not copying are the styles.

  useEffect(() => {
    Client.lists.index()
      .then(result => {
        result.forEach(item => item['key'] = item.id); // adding key to make it work
        setIsLoading(false);
        setLists(result);
      })
  }, [])

  closeRow = (rowMap, rowKey) => {
    console.log('rowKey', rowKey);
    if (rowMap[rowKey]) {
        rowMap[rowKey].closeRow();
    }
  }

  deleteRow = (rowMap, rowKey) => {
    closeRow(rowMap, rowKey);
    const newList = [...lists];
    const prevIndex = lists.findIndex(item => item.key === rowKey);
    newList.splice(prevIndex, 1);
    setLists(newList)
    Client.lists.destroy(rowKey);
  }

<SwipeListView
      keyExtractor={item => String(item.id)}
      data={lists}
      renderItem={data => (
        <ListItem
          title={data.item.name}
          onPress={() => {
            navigate('ListDetails', { 
              listId: data.item.id,
              listName: data.item.name
            })
          }}
          bottomDivider
          chevron
        />
      )}
      renderHiddenItem={({ item }, rowMap) => (
        <View>
          <TouchableOpacity
            onPress={() => {
              closeRow(rowMap, item.key);
              navigate('EditList', { list: item })
            }}
          >
            <Text>
              Edit
            </Text>
          </TouchableOpacity>
          <TouchableOpacity
            onPress={() =>
              deleteRow(rowMap, item.key)
            }
          >
            <Text>
              Delete
            </Text>
          </TouchableOpacity>
        </View>
      )}
      rightOpenValue={-150}
      style={{ marginBottom: 60 }}
      keyExtractor={(item) => String(item.id) }
      ListEmptyComponent={ 
        <EmptyComponent message="You haven't created any lists yet." />
      }
    />

If I remove the line that sets the key attribute on the result array (see the useEffect method), I will get a:

rowKey undefined

on the console.log in the closeRow function.

@jemise111
Copy link
Owner

Hey @elioncho thanks for the detailed example and sorry for the delay in responding.

So in your example if you don't have a key set on every object in your data than indeed item.key will be undefined as you pass it to your closeRow function. In that case you'll have to pass the same "key" that your keyExtractor function is creating for each object. So in your example you should change this line:

<TouchableOpacity
  onPress={() => {
    closeRow(rowMap, item.key);

to this:

<TouchableOpacity
  onPress={() => {
    closeRow(rowMap, String(item.id));

And all should work as intended :) let me know if that's not the case, thanks!

@elioncho
Copy link

elioncho commented Nov 10, 2019

Works perfectly, thanks!

@jemise111
Copy link
Owner

Going to close this, please reopen if anyone else has issues, thanks

@StevenBlack
Copy link

Jessie @jemise111 I'm wondering, why does the key (see the item.key I quote here) need to be a String, presently?

My keys are all integers, everywhere. Coming at this straight and clean, no legacy to worry about, this machination is a source of cognitive loading, and code complexity. So I'm fascinated, why a string, strictly?

<TouchableOpacity
  onPress={() => {
    closeRow(rowMap, item.key);

@jemise111
Copy link
Owner

Hey @StevenBlack in this case above I was simply using the key provided by the OP's keyExtractor function above. i.e.

keyExtractor={item => String(item.id)}

You could, in theory, use integers for your keys. This component will function properly. However you will get a warning from react-native. See this discussion for more:

facebook/react-native#18291

@StevenBlack
Copy link

ah! So ignore the warning. Cool! That's valuable input. Thank you, Jessie @jemise111.

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

No branches or pull requests

4 participants