Skip to content

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

Closed
annepham25 wants to merge 1 commit into
react:mainfrom
annepham25:fix-virutalizedlist-addition
Closed

[FIX] Add a check for scrollEnabled to VirtualizedList error #34637
annepham25 wants to merge 1 commit into
react:mainfrom
annepham25:fix-virutalizedlist-addition

Conversation

@annepham25

Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
Contributor

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

@analysis-bot

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

Base commit: 73abcba
Branch: main

@annepham25

Copy link
Copy Markdown
Author

@NickGerleman any update?

@NickGerleman

Copy link
Copy Markdown
Contributor

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

@NickGerleman

Copy link
Copy Markdown
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