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

[FIX] Add a check for scrollEnabled to VirtualizedList error #34637

Closed

Conversation

annepham25
Copy link

Summary

Already merged PR here, but there was confusion that the changes should also be in VirtualizedList_EXPERIMENTAL.js and VirtualizedList.js, not one or the other.

Changelog

[General] [Added] - Added a check to if scrollEnabled is not false, if so then fire the VirtualizedList error in VirtualizedList.js to match VirtualizedList_EXPERIMENTAL.js

[CATEGORY] [TYPE] - Message

Test Plan

Passes all provided automatic tests. In a personal app, there is a situation of nested ScrollViews that had triggered the error. After defining scrollEnabled={false} and adding the check, the error no longer appears.

Previously spoke with @NickGerleman so @ mentioning him again to get attention.

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Sep 8, 2022
@annepham25 annepham25 changed the title Add a check for scrollEnabled to VirtualizedList error (fix) [FIX] Add a check for scrollEnabled to VirtualizedList error Sep 8, 2022
@react-native-bot react-native-bot added the Type: Enhancement A new feature or enhancement of an existing feature. label Sep 8, 2022
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,639,265 +0
android hermes armeabi-v7a 7,051,493 +0
android hermes x86 7,940,845 +0
android hermes x86_64 7,912,833 +0
android jsc arm64-v8a 9,513,356 +0
android jsc armeabi-v7a 8,288,964 +0
android jsc x86 9,452,682 +0
android jsc x86_64 10,043,754 +0

Base commit: 0e8050d
Branch: main

@NickGerleman
Copy link
Contributor

We can merge this in, but VirtualizedList_EXPERIMENTAL should soon be replacing VirtualizedList, if that also works.

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 73abcba
Branch: main

@annepham25
Copy link
Author

@NickGerleman any update?

@NickGerleman
Copy link
Contributor

VietualizedList_EXPERIMENTAL will be merged into VirtualizedList in about a week, so it should likely be in the next release.

@NickGerleman
Copy link
Contributor

VirtualizedList_EXPERIMENTAL replaced the original for 0.71, so I think this should no longer be needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. Type: Enhancement A new feature or enhancement of an existing feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants