Skip to content

Conversation

kmichalikk
Copy link
Collaborator

@kmichalikk kmichalikk commented Sep 22, 2025

Closes https://github.com/software-mansion/react-native-screens-labs/issues/456

Description

This PR implements ScrollViewProviding protocol as per internal discussion. The protocol enables us to be smarter about finding content ScrollView than simply descending into 0th child. A class that implements this protocol can decide how to continue the search. To complement the feature, ScrollViewFinder is updated with new method, findContentScrollViewWithDelegatingToProvider that takes advantage of the protocol.

Note that we don't add the support for SplitView, because there should be no reason for it to be nested inside another container.

Changes

  • Added ScrollViewProviding protocol, with implementations for RNSScreenStack, BottomTabsHostComponentView. The implementations continue search directly from their currently selected screens. This has the desired side-effect of completing the search even if the screen is not yet mounted in host hierarchy.
  • Modified ScrollViewFinder to include a method that takes advantage of the new protocol, while leaving the simple version

Test code and steps to reproduce

Use Test3212; For now, nested containers may not behave as expected, because the new "smart" method is not called yet; things will change in the following PRs

@kmichalikk kmichalikk requested a review from kkafar September 22, 2025 08:30
@kmichalikk kmichalikk self-assigned this Sep 22, 2025
@kmichalikk kmichalikk force-pushed the @kmichalikk/scroll-view-providing branch 2 times, most recently from 108ebe5 to 74644fe Compare September 23, 2025 10:23
Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

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

Hey, this looks really promising. Good job.

Please remove all usages of the new method. I want to see dedicated PR for each swapped method & analysis of before & after applying the change. So that it is documented why it is needed & what problems it solves.

@kmichalikk kmichalikk marked this pull request as draft September 29, 2025 07:53
@kmichalikk kmichalikk force-pushed the @kmichalikk/scroll-view-providing branch from 74644fe to dd6f7cc Compare September 29, 2025 12:18
@kmichalikk kmichalikk changed the base branch from main to @kmichalikk/scroll-edge-effects-test September 29, 2025 12:18
@kmichalikk kmichalikk marked this pull request as ready for review September 29, 2025 13:28
@kmichalikk kmichalikk requested a review from kkafar September 29, 2025 13:28
Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

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

This looks solid now.

Please update the PR description before merging.

Base automatically changed from @kmichalikk/scroll-edge-effects-test to main October 2, 2025 04:56
@kmichalikk kmichalikk force-pushed the @kmichalikk/scroll-view-providing branch from 4c3489c to 565df0a Compare October 2, 2025 06:37
@kmichalikk kmichalikk marked this pull request as draft October 2, 2025 06:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants