Skip to content

prevent scroll event in nested scroll when scrollEnabled={false}#42219

Closed
Icehunter wants to merge 1 commit into
react:mainfrom
Icehunter:main
Closed

prevent scroll event in nested scroll when scrollEnabled={false}#42219
Icehunter wants to merge 1 commit into
react:mainfrom
Icehunter:main

Conversation

@Icehunter

@Icehunter Icehunter commented Jan 9, 2024

Copy link
Copy Markdown
Contributor

Summary:

When a FlatList is in side a scroll view (think Netflix style navigation), the DPAD up/down fires on the scroll view, despite scrollEnabled={false} being set. This additiontially conflicts with any custom scroll event that has been created.

Changelog:

[Android] [Fixed] - fix: prevent scroll event in nested scroll when scrollEnabled={false}

Test Plan

I tested this by making a ScrollView with FlatList of opposite scrolling direction inside with basic card layouts.

Both had scrollEnabled={false}

I scrolled the ScrollView myself as it has multiple rows using:

  const scrollToItem = React.useCallback(
    (itemIndex: number): void => {
      const targetScrollY = itemIndex * height
      scrollViewRef.current?.scrollTo({ y: targetScrollY, animated: true })
    },
    [height]
  )

  React.useEffect(() => {
    // Row 0, is global nav, but it's also the first row of cards
    // when we scroll to "1" what we mean is global nav is hidden
    // we should still be showing the first row of items.
    scrollToItem(rowIndex <= 1 ? 0 : rowIndex - 1)
  }, [rowIndex, scrollToItem])

@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 Jan 9, 2024
@analysis-bot

Copy link
Copy Markdown
Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 16,642,445 -131,252
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 20,038,417 -127,144
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: f7f9250
Branch: main

@mdvacca mdvacca left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm, importing to review and commit internally

@facebook-github-bot

Copy link
Copy Markdown
Contributor

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

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Jan 10, 2024
@facebook-github-bot

Copy link
Copy Markdown
Contributor

@mdvacca merged this pull request in c7873b7.

@facebook-github-bot

Copy link
Copy Markdown
Contributor

This pull request has been reverted by 07a676a.

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. Merged This PR has been merged. Reverted 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.

4 participants