Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix items disappear when zooming VirtualizedList #33765

Closed
wants to merge 3 commits into from

Conversation

islandryu
Copy link
Contributor

@islandryu islandryu commented May 5, 2022

Summary

fix #33705
Fixed the disappearance of items when scrolling after zooming VirtualizedList.
example https://github.com/islandryu/zoomVirtualizedList

Before modification

2022-05-05.9.23.40.mov

After modification

2022-05-05.14.42.05.mov

Changelog

[General] [Fixed] - Fixed the disappearance of items when scrolling after zooming VirtualizedList.

Test Plan

Make the VirtualizedList zoomable with a prop such as maximumZoomScale.
Apply the patch and make sure that items in the VirtualizedList do not disappear when you scroll after zooming the VirtualizedList.

Or apply the patch from this repository and check it.
https://github.com/islandryu/zoomVirtualizedList

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 5, 2022
@islandryu islandryu marked this pull request as ready for review May 5, 2022 05:55
@facebook-github-bot facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label May 5, 2022
Copy link
Contributor

@yungsters yungsters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks for this improvement.

@@ -47,6 +47,7 @@ type Context = $ReadOnly<{
timestamp: number,
velocity: number,
visibleLength: number,
...
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not add zoomScale: number here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@facebook-github-bot
Copy link
Contributor

@yungsters has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Copy link
Contributor

@yungsters yungsters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The VirtualizedList-test.js Jest test seems to be failing after this change. Specifically, the "keeps sticky headers above viewport visualized" test is failing. Can you take a look?

@@ -25,12 +25,13 @@ export function elementsThatOverlapOffsets(
offset: number,
...
},
zoomScale: number,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This requires calls in VirtualizeUtils-test.js to be updated. Otherwise, the Jest test fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed so that the test passes.

@islandryu islandryu requested a review from yungsters May 6, 2022 08:12
@facebook-github-bot
Copy link
Contributor

@yungsters has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @islandryu in 13a72e0.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label May 6, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FlatList items are disappeared after pinching zoom
4 participants