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
[Bugfix][SwipeableFlatList] Fix SwipeableFlatList on close #16682
[Bugfix][SwipeableFlatList] Fix SwipeableFlatList on close #16682
Conversation
@facebook-github-bot label Needs more information @facebook-github-bot label Needs more information Generated by 🚫 dangerJS |
What if we don't want the other rows to close automatically? |
Read source. It's by design |
Reviewed By: sahrens Differential Revision: D5912488 fbshipit-source-id: 3d2872a7712c00badcbd8341a7d058df14a9091a
@kesha-antonov can you check, I think with your current solution all rows are rerendered on every swipe. If I'm wrong, cool then. |
@@ -35,7 +35,9 @@ type SwipableListProps = { | |||
type Props<ItemT> = SwipableListProps & FlatListProps<ItemT>; | |||
|
|||
type State = { | |||
openRowKey: ?string, | |||
extraData: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need extra object inside state? isn't it simpler keep it as before but put into flat list extraData={this.state}
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following the example in https://facebook.github.io/react-native/docs/flatlist.html
By passing extraData={this.state} to FlatList we make sure FlatList itself will re-render when the state.selected changes. Without setting this prop, FlatList would not know it needs to re-render any items because it is also a PureComponent and the prop comparison will not show any changes.
@kesha-antonov I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project. |
}; | ||
|
||
this._shouldBounceFirstRowOnMount = this.props.bounceFirstRowOnMount; | ||
|
||
this.onRefFlatList = this.onRefFlatList.bind(this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code works faster than fat arrow.
In render we should avoid create function every time
@tomasreimers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kesha-antonov totally get that -- I meant why .bind(this) in the initializer instead of using class fields (like _onScroll)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ping
Waiting for this fix to be merged? Any updates on it? |
ping |
Hi @kesha-antonov , I think this was fixed by #18001 |
Motivation
This commit d8cc6e3 introduced
SwipeableFlatList
It has bug - doesn't close on swipe other rows.
I fixed it.
Release Notes
[IOS/ANDROID] [BUGFIX] [SwipeableFlatList] - opened rows did not close on swipe other rows