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

Cannot close row: keyExtractor doesn't appear to be honored in rowMap when 'key' attribute present in row item #547

Closed
rpattcorner opened this issue Apr 2, 2021 · 10 comments

Comments

@rpattcorner
Copy link

rpattcorner commented Apr 2, 2021

I've got a corner case perhaps ... my application is a music application, so its data.item object has an attribute called, lterally, 'key'.

  • As an aside, this doc seems germane, but results in unswipable rows when inserted in my code
  • FWIW, the screen is a functional component, not class based per most examples

Per #449 and general practice, I use a keyExtractor in the <SwipeListView>, just like I do in the <FlatList> that I'm trying to replace. FlatList works fine.

In the FlatList my keyExtractor looks like keyExtractor={item => item.tune_id}, but I've changed it to keyExtractor={(item, index) => item.tune_id} per #449 just to follow available advice. Makes no difference to the problem, which is an inability to close a row.

So, for completeness, we have:

<SwipeListView
                    ListEmptyComponent={ListEmptyView}
                    ItemSeparatorComponent={RenderSeparator}
                    data={theTuneList}
                    keyExtractor={(item, index) => item.tune_id}
                    renderItem={renderItem}
                    renderHiddenItem={renderHiddenItem}
                    extraData={state.initialized}
                    leftOpenValue={75}
                    rightOpenValue={-150}
                    previewRowKey={'0'}
                    previewOpenValue={-40}
                    previewOpenDelay={3000}
                    onRowDidOpen={onRowDidOpen}
                    disableRightSwipe={true}
                />

and you'd think all would be well. As you can guess from the title of this issue, my item looks like:

{
   tune_id: SomeUniqueId
   key: 'G'
   meter: 'reel'
   ...
}

and in my vanilla closeRow function:

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

The rowMap appears to be ignoring the keyExtractor. Have a look at this debug view from Chrome, in whose underlying data there are 3 tune items, two in the key of G and one in the key of D, with the 'D' tune slid to the left, and a click on the close button that invokes closeRow. Note that the second 'G' tune does not show up, and more importantly, the rowMap seems to be keying off the literal 'key' attribute of the item instead of the keyExtractor mapping:

image

As you can see there are two rows in the rowMap, not the expected 3. Possibly because two of the tunes have the same (literal) key attribute. This could be a hint, or an effect, not sure.

image

And there's no way that rowMap[rowKey], which should literally be rowMap['@tune_id.482'] is ever going to find a row to close.

image

@jemise111 am I (hopefully) doing something simple and obviously wrong here?

@jemise111
Copy link
Owner

Hey @rpattcorner, I never thought that if objects had a key they wouldn't be unique! I guess if you had a music app or a locksmith app there's definitely an edge case I never thought of 😂 !

Currently a key takes precedence over a keyExtractor so that certainly could be the issue. Let's do this.. can you manually make a change to the code and if it works I'll push a release with the fix.

Can you please go to node_modules/react-native-swipe-list-view/components/SwipeListView.js => line 378 and change the following code

let { key } = item;
if (!key && this.props.keyExtractor) {
    key = this.props.keyExtractor(item, index);
}

to

let key = this.props.keyExtractor(item, index);

And let me know if that fixes your issues, thanks!

@rpattcorner
Copy link
Author

@jemise111 , thanks for the quick response! Yes, the proposed one line substitution fixes the problem for my app! Any reason to hold out for a push before sending my next build to testers?

@rpattcorner
Copy link
Author

(hold on a minute, seeing a problem, likely unrelated)

@jemise111
Copy link
Owner

@rpattcorner I just released version v3.2.7 which will now favor keyExtractor over key, you should be able to use that version without issue

I will leave this issue open per your latest comment, but please close this if it is unrelated, thanks!

@rpattcorner
Copy link
Author

Thanks! It looks to me like the issue I experienced (a freeze on navigation) is unrelated, and seems to correlate with running both an older Android emulator and a real Android device at the same time, but I'll download the new version and bang on it awhile, then close if all still seems well.

Again, much appreciated. Those edge cases are like 'who's on first'.

@rpattcorner
Copy link
Author

I did see this, which is new and possibly relates, just FWIW:

[11:27:07] VirtualizedList: You have a large list that is slow to update - make sure your renderItem function renders components that follow React performance best practices like PureComponent, shouldComponentUpdate, etc. Object {
[11:27:07]   "contentLength": 2770.28564453125,
[11:27:07]   "dt": 759,
[11:27:07]   "prevDt": 1032,
[11:27:07] }

@jemise111
Copy link
Owner

@rpattcorner IMO that shouldn't be something related to this lib but I'll keep an eye out for other reports

@rpattcorner
Copy link
Author

Looks good, many thanks! Closing.

@code-by
Copy link

code-by commented May 28, 2023

I found that rowMap is empty object if I use not directly in renderItem function, but enclose it with another intermediate component. Why?

@deyanskiq
Copy link

@code-by hitting the same issue. Have you managed to figure it out somehow?

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