Skip to content

[ListView] Operate on the true scroll responder instead of the scroll component #1927

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

Closed
wants to merge 1 commit into from

Conversation

ide
Copy link
Contributor

@ide ide commented Jul 9, 2015

When composing scroll views, this.refs[SCROLLVIEW_REF] may refer to another higher-order scroll component instead of a ScrollView. This can cause issues if you expect to need it to be a ScrollView backed by an RCTScrollView.

The solution is to call getScrollResponder() - as long as all higher-order scroll components implement this method, it will make its way down to the true ScrollView, which is what ListView wants here.

Test Plan: Load an app with composed scroll views and no longer get a crash in calculateChildFrames:(NSNumber *)reactTag callback:(RCTResponseSenderBlock)callback.

@ide
Copy link
Contributor Author

ide commented Jul 9, 2015

cc @sahrens, this is the crash and corresponding PR I mentioned on your sync commit.

… component

When composing scroll views, `this.refs[SCROLLVIEW_REF]` may refer to another higher-order scroll component instead of a ScrollView. This can cause issues if you expect to need it to be a ScrollView backed by an RCTScrollView.

The solution is to call `getScrollResponder()` - as long as all higher-order scroll components implement this method, it will make its way down to the true ScrollView, which is what ListView wants here.

Test Plan: Load an app with composed scroll views and no longer get a crash in `calculateChildFrames:(NSNumber *)reactTag callback:(RCTResponseSenderBlock)callback`.
@ide ide force-pushed the fix-scroll-responder-ref branch from f7681a0 to e9a31fa Compare July 9, 2015 20:18
@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 Jul 9, 2015
@sahrens
Copy link
Contributor

sahrens commented Jul 13, 2015

Cool - I have another bug fix that will conflict with this, but shouldn't be an issue to stack them.

@ide ide closed this in 9f07b9a Jul 14, 2015
@ide ide deleted the fix-scroll-responder-ref branch July 15, 2015 20:46
ide added a commit that referenced this pull request Jul 16, 2015
… component

Summary:
When composing scroll views, `this.refs[SCROLLVIEW_REF]` may refer to another higher-order scroll component instead of a ScrollView. This can cause issues if you expect to need it to be a ScrollView backed by an RCTScrollView.

The solution is to call `getScrollResponder()` - as long as all higher-order scroll components implement this method, it will make its way down to the true ScrollView, which is what ListView wants here.

Closes #1927
Github Author: James Ide <ide@jameside.com>
Fanghao pushed a commit to discord/react-native that referenced this pull request Jul 22, 2015
… component

Summary:
When composing scroll views, `this.refs[SCROLLVIEW_REF]` may refer to another higher-order scroll component instead of a ScrollView. This can cause issues if you expect to need it to be a ScrollView backed by an RCTScrollView.

The solution is to call `getScrollResponder()` - as long as all higher-order scroll components implement this method, it will make its way down to the true ScrollView, which is what ListView wants here.

Closes facebook#1927
Github Author: James Ide <ide@jameside.com>
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants