Update RCTScrollView.m #13180

Feng999 · 2017-03-28T13:28:48Z

fix problem of function scrollToEnd: There are some strange thing happened when contentSize.height(width) is smaller than bounds.size.height(width). In fact, there is no need to scroll in this case.

Thanks for submitting a PR! Please read these instructions carefully:

Explain the motivation for making this change.
Provide a test plan demonstrating that the code is solid.
Match the code formatting of the rest of the codebase.
Target the master branch, NOT a "stable" branch.

Motivation (required)

What existing problem does the pull request solve?

Test Plan (required)

A good test plan has the exact commands you ran and their output, provides screenshots or videos if the pull request changes UI or updates the website. See What is a Test Plan? to learn more.

If you have added code that should be tested, add tests.

Next Steps

Sign the CLA, if you haven't already.

Small pull requests are much easier to review and more likely to get merged. Make sure the PR does only one thing, otherwise please split it.

Make sure all tests pass on both Travis and Circle CI. PRs that break tests are unlikely to be merged.

For more info, see the "Pull Requests" section of our "Contributing" guidelines.

fix function scrollToEnd: There are some strange thing when contentSize.height(width) is smaller than bounds.size.height(width). In fact, there is no need to scroll in this case.

facebook-github-bot · 2017-03-28T13:29:16Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact cla@fb.com if you have any questions.

Guardiannw · 2017-03-28T21:07:44Z

This is about the same that I have in my PR #12889. Definitely need one of these implemented.

facebook-github-bot · 2017-03-29T06:49:44Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

Feng999 · 2017-03-29T07:07:42Z

Nothing changed to android code, don't know why test failed.

javache · 2017-04-19T10:41:09Z

React/Views/RCTScrollView.m

-    offset = CGPointMake(_scrollView.contentSize.width - _scrollView.bounds.size.width, 0);
+    CGFloat offsetX = _scrollView.contentSize.width - _scrollView.bounds.size.width;
+    offsetX = offsetX > 0 ? offsetX : 0;
+    offset = CGPointMake(offsetX, 0);


Can you do CGPointMake(MAX(offsetX, 0), 0)) instead?

javache · 2017-04-19T10:41:24Z

React/Views/RCTScrollView.m

-    offset = CGPointMake(0, _scrollView.contentSize.height - _scrollView.bounds.size.height);
+    CGFloat offsetY = _scrollView.contentSize.height - _scrollView.bounds.size.height;
+    offsetY = offsetY > 0 ? offsetY : 0;
+    offset = CGPointMake(0, offsetY);


Can you do CGPointMake(0, MAX(offsetY, 0)) instead?

Good advice！ Thank you.

Optimize code about scrollToEnd_bug_fix

facebook-github-bot · 2017-04-21T09:18:54Z

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

facebook-github-bot · 2017-04-21T23:48:14Z

I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification.

facebook-github-bot · 2017-04-25T09:31:38Z

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

facebook-github-bot · 2017-04-25T17:08:29Z

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

facebook-github-bot · 2017-04-25T17:17:55Z

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

facebook-github-bot · 2017-04-27T12:53:14Z

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

facebook-github-bot · 2017-04-28T16:18:11Z

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

facebook-github-bot · 2017-04-28T16:48:59Z

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

facebook-github-bot · 2017-05-19T17:30:23Z

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

facebook-github-bot · 2017-05-19T17:30:51Z

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

facebook-github-bot · 2017-05-23T17:20:16Z

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

shergin · 2017-05-26T06:43:16Z

React/Views/RCTScrollView.m

  } else {
-    offset = CGPointMake(0, _scrollView.contentSize.height - _scrollView.bounds.size.height);
+    CGFloat offsetY = _scrollView.contentSize.height - _scrollView.bounds.size.height;


I am afraid that we forget to consider contentInset here. :(

@public

Summary: @public This sync includes the following changes: - **[ca0941fce](facebook/react@ca0941fce)**: Add regression test for Placeholder fallbacks with lifecycle methods (#13254) //<Andrew Clark>// - **[a32c727f2](facebook/react@a32c727f2)**: Optimize readContext for Subsequent Reads of All Bits (#13248) //<Sebastian Markbåge>// - **[2b509e2c8](facebook/react@2b509e2c8)**: [Experimental] API for reading context from within any render phase function (#13139) //<Andrew Clark>// - **[5776fa3fc](facebook/react@5776fa3fc)**: Update www warning shim (#13244) //<Dan Abramov>// - **[3d3506d37](facebook/react@3d3506d37)**: Include Modes in the component stack (#13240) //<Dan Abramov>// - **[71b4e9990](facebook/react@71b4e9990)**: [react-test-renderer] Jest matchers for async tests (#13236) //<Andrew Clark>// - **[2c560cb99](facebook/react@2c560cb99)**: Fix unwinding starting with a wrong Fiber on error in the complete phase (#13237) //<Dan Abramov>// - **[ead08827d](facebook/react@ead08827d)**: Add more flexibility in testing errors in begin/complete phases (#13235) //<Dan Abramov>// - **[e4e58343e](facebook/react@e4e58343e)**: Move unstable_yield to main export (#13232) //<Andrew Clark>// - **[0e235bb8f](facebook/react@0e235bb8f)**: Removed unused state argument in unsubscribe method of <Subscription /> (#13233) //<Mateusz Burzyński>// - **[236f60872](facebook/react@236f60872)**: Fail tests if toWarnDev() does not wrap warnings in array (#13227) //<Dan Abramov>// - **[acbb4f93f](facebook/react@acbb4f93f)**: Remove the use of proxies for synthetic events in DEV (#13225) //<Dan Abramov>// - **[171e0b7d4](facebook/react@171e0b7d4)**: Fix “no onChange handler” warning to fire on falsy values ("", 0, false) too (#12628) //<Nicole Levy>// - **[606c30aa5](facebook/react@606c30aa5)**: fixed a typo in commentout in ReactFiberUnwindWork.js (#13172) //<Fumiya Shibusawa>// - **[9f78913b2](facebook/react@9f78913b2)**: Update prettier (#13205) //<Johan Henriksson>// - **[6d3e26288](facebook/react@6d3e26288)**: Remove unnecessary `typeof` checks (#13196) //<jddxf>// - **[82c7ca4cc](facebook/react@82c7ca4cc)**: Add component stacks to some warnings (#13218) //<Dan Abramov>// - **[21ac62c77](facebook/react@21ac62c77)**: Fix a portal unmounting crash for renderers with distinct Instance and Container (#13220) //<Thibault Malbranche>// - **[d6a0626b3](facebook/react@d6a0626b3)**: Set current fiber during before-mutation traversal (#13219) //<Dan Abramov>// - **[f9358c51c](facebook/react@f9358c51c)**: Change warning() to automatically inject the stack, and add warningWithoutStack() as opt-out (#13161) //<Dan Abramov>// - **[467d13910](facebook/react@467d13910)**: Enforce presence or absence of component stack in tests (#13215) //<Dan Abramov>// - **[43ffae2d1](facebook/react@43ffae2d1)**: Suspending inside a constructor outside of strict mode (#13200) //<Andrew Clark>// - **[659a29cec](facebook/react@659a29cec)**: Reorganize how shared internals are accessed (#13201) //<Dan Abramov>// - **[58f3b29d9](facebook/react@58f3b29d9)**: Added SSR/hydration tests for modes, forwardRef, and Profiler (#13195) //<Brian Vaughn>// - **[1c89cb62f](facebook/react@1c89cb62f)**: Use ReactDebugCurrentFrame.getStackAddendum() in element validator (#13198) //<Dan Abramov>// - **[e6076ecf4](facebook/react@e6076ecf4)**: Remove ad-hoc forks of getComponentName() and fix it (#13197) //<Dan Abramov>// - **[32f6f258b](facebook/react@32f6f258b)**: Remove event simulation of onChange events (#13176) //<Philipp Spieß>// - **[9ca37f843](facebook/react@9ca37f843)**: docs: update comments (#13043) //<Sen Yang>// - **[f89f25f47](facebook/react@f89f25f47)**: Correct type of `ref` in forwardRef render() (#13100) //<Moti Zilberman>// - **[7b99ceabe](facebook/react@7b99ceabe)**: Deprecate test utils mock component follow up (#13194) //<Brian Vaughn>// - **[6ebc8f3c0](facebook/react@6ebc8f3c0)**: Add support for re-entrant SSR stacks (#13181) //<Dan Abramov>// - **[d64d1ddb5](facebook/react@d64d1ddb5)**: Deprecate ReactTestUtils.mockComponent() (#13193) //<Brian Vaughn>// - **[e79366d54](facebook/react@e79366d54)**: Link create-subscription doc to GH issue with de-opt explanation (#13187) //<Brian Vaughn>// - **[1f32d3c6d](facebook/react@1f32d3c6d)**: Test renderer flushAll method verifies an array of expected yields (#13174) //<Brian Vaughn>// - **[377e1a049](facebook/react@377e1a049)**: Add a test for SSR stack traces (#13180) //<Dan Abramov>// - **[96d38d178](facebook/react@96d38d178)**: Fix concatenation of null to a warning message (#13166) //<Dan Abramov>// - **[095dd5049](facebook/react@095dd5049)**: Add DEV warning if forwardRef function doesn't use the ref param (#13168) //<Brian Vaughn>// - **[566259567](facebook/react@566259567)**: Refactor stack handling (no functional changes) (#13165) //<Dan Abramov>// - **[ebbd22143](facebook/react@ebbd22143)**: Configure react-test-renderer as a secondary (#13164) //<Brandon Dail>// - **[ddc91af79](facebook/react@ddc91af79)**: Decrease nested update limit from 1000 to 50 (#13163) //<Andrew Clark>// - **[3596e40b3](facebook/react@3596e40b3)**: Fix nested update bug (#13160) //<Andrew Clark>// - **[449f6ddd5](facebook/react@449f6ddd5)**: create a new FeatureFlags file for test renderer on www (#13159) //<Chang Yan>// - **[f762b3abb](facebook/react@f762b3abb)**: Run react-dom SSR import test in jsdom-less environment (#13157) //<Dan Abramov>// - **[6f6b560a6](facebook/react@6f6b560a6)**: Renamed selfBaseTime/treeBaseTime Fiber attributes to selfBaseDuration/treeBaseDuration (#13156) //<Brian Vaughn>// - **[1386ccddd](facebook/react@1386ccddd)**: Fix ReferenceError when requestAnimationFrame isn't defined (#13152) //<Dan Abramov>// - **[f5779bbc1](facebook/react@f5779bbc1)**: Run server rendering test on bundles (#13153) //<Dan Abramov>// - **[9faf389e7](facebook/react@9faf389e7)**: Reset profiler timer correctly after errors (#13123) //<Brian Vaughn>// - **[85fe4ddce](facebook/react@85fe4ddce)**: Fix - issue #12765 / the checked attribute is not initially set on the input (#13114) //<XuMM_12>// - **[07fefe333](facebook/react@07fefe333)**: Drop handling for ms and O prefixes for CSS transition and animation events. (#13133) //<Rouven Weßling>// - **[88d7ed8bf](facebook/react@88d7ed8bf)**: React.Timeout -> React.Placeholder (#13105) //<Andrew Clark>// - **[f128fdea4](facebook/react@f128fdea4)**: Suspending outside of strict trees and async trees (#13098) //<Andrew Clark>// - **[aa8266c4f](facebook/react@aa8266c4f)**: Prepare placeholders before timing out (#13092) //<Andrew Clark>// - **[c039c16f2](facebook/react@c039c16f2)**: Fix this in a functional component for ShallowRenderer (#13144) //<Toru Kobayashi>// - **[64e1921aa](facebook/react@64e1921aa)**: Fix Flow type that event target can be null (#13124) //<Sebastian Markbåge>// - **[6d6de6011](facebook/react@6d6de6011)**: Add PROFILE bundles for www+DOM and fbsource+RN/RF (#13112) //<Brian Vaughn>// - **[71a60ddb1](facebook/react@71a60ddb1)**: Add link to another article about React renderers //<Dan Abramov>// - **[6a530e3ba](facebook/react@6a530e3ba)**: adding check for mousemove (#13090) //<Jason Williams>// - **[c35a1e748](facebook/react@c35a1e748)**: Fix crash during server render in react 16.4.1. (#13088) //<Dustin Masters>// - **[076bbeace](facebook/react@076bbeace)**: Fall back to 'setTimeout' when 'requestAnimationFrame' is not called (#13091) //<Flarnie Marchan>// - **[da5c87bdf](facebook/react@da5c87bdf)**: Fixes children when using dangerouslySetInnerHtml in a selected <option> (#13078) //<Michael Ridgway>// - **[a960d18bc](facebook/react@a960d18bc)**: eliminate unnecessary do-while loop in renderRoot() (#13087) //<Nathan Quarles>// - **[5b3d17a5f](facebook/react@5b3d17a5f)**: setting a flag, so that the first movement will have the correct value (#13082) //<Jason Williams>// - **[b0f60895f](facebook/react@b0f60895f)**: Automatically Profile roots when DevTools is present (#13058) //<Brian Vaughn>// - **[ae8c6dd53](facebook/react@ae8c6dd53)**: remove some redundant lines (#13077) //<Nathan Quarles>// - **[0fcf92d06](facebook/react@0fcf92d06)**: Add a link to custom renderer intro article //<Dan Abramov>// - **[97af3e1f3](facebook/react@97af3e1f3)**: Do not add additional work to a batch that is already rendering (#13072) //<Andrew Clark>// - **[4fe6eec15](facebook/react@4fe6eec15)**: Always batch updates of like priority within the same event (#13071) //<Andrew Clark>// - **[8e87c139b](facebook/react@8e87c139b)**: Remove transitive dependency on fbjs (#13075) //<Dan Abramov>// - **[aeda7b745](facebook/react@aeda7b745)**: Remove fbjs dependency (#13069) //<Dan Abramov>// - **[b1b3acbd6](facebook/react@b1b3acbd6)**: Inline fbjs/lib/emptyObject (#13055) //<Dan Abramov>// Release Notes: [GENERAL] [FEATURE] [React] - React sync for revisions ae14317...ca0941f Reviewed By: bvaughn Differential Revision: D8979192 fbshipit-source-id: 7a14d3b0a253a81d162d7f8c899e99cf6ac4fee4

Update RCTScrollView.m

d29efaa

fix function scrollToEnd: There are some strange thing when contentSize.height(width) is smaller than bounds.size.height(width). In fact, there is no need to scroll in this case.

facebook-github-bot added the GH Review: review-needed label Mar 28, 2017

Merge branch 'master' into scrollToEnd_bug_fix

d71c96e

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 Mar 29, 2017

javache reviewed Apr 19, 2017

View reviewed changes

Feng999 added 3 commits April 21, 2017 14:15

Update RCTScrollView.m

9b7f09a

Optimize code about scrollToEnd_bug_fix

Update RCTScrollView.m

45d243f

Merge branch 'master' into scrollToEnd_bug_fix

24b0634

facebook-github-bot added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Apr 21, 2017

facebook-github-bot added Import Failed Import Started This pull request has been imported. This does not imply the PR has been approved. and removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Apr 21, 2017

Merge branch 'master' into scrollToEnd_bug_fix

ee61574

Merge branch 'master' into scrollToEnd_bug_fix

3e9028d

Merge branch 'master' into scrollToEnd_bug_fix

abe17d3

Merge branch 'master' into scrollToEnd_bug_fix

ed4b908

facebook-github-bot closed this in a3d58ba May 24, 2017

shergin reviewed May 26, 2017

View reviewed changes

shergin self-assigned this May 26, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update RCTScrollView.m #13180

Update RCTScrollView.m #13180

Feng999 commented Mar 28, 2017 •

edited

Loading

facebook-github-bot commented Mar 28, 2017

Guardiannw commented Mar 28, 2017

facebook-github-bot commented Mar 29, 2017

Feng999 commented Mar 29, 2017

javache Apr 19, 2017

javache Apr 19, 2017

Feng999 Apr 21, 2017

facebook-github-bot commented Apr 21, 2017

facebook-github-bot commented Apr 21, 2017

facebook-github-bot commented Apr 25, 2017

facebook-github-bot commented Apr 25, 2017

facebook-github-bot commented Apr 25, 2017

facebook-github-bot commented Apr 27, 2017

facebook-github-bot commented Apr 28, 2017

facebook-github-bot commented Apr 28, 2017

facebook-github-bot commented May 19, 2017

facebook-github-bot commented May 19, 2017

facebook-github-bot commented May 23, 2017

shergin May 26, 2017

Update RCTScrollView.m #13180

Update RCTScrollView.m #13180

Conversation

Feng999 commented Mar 28, 2017 • edited Loading

Motivation (required)

Test Plan (required)

Next Steps

facebook-github-bot commented Mar 28, 2017

Guardiannw commented Mar 28, 2017

facebook-github-bot commented Mar 29, 2017

Feng999 commented Mar 29, 2017

javache Apr 19, 2017

Choose a reason for hiding this comment

javache Apr 19, 2017

Choose a reason for hiding this comment

Feng999 Apr 21, 2017

Choose a reason for hiding this comment

facebook-github-bot commented Apr 21, 2017

facebook-github-bot commented Apr 21, 2017

facebook-github-bot commented Apr 25, 2017

facebook-github-bot commented Apr 25, 2017

facebook-github-bot commented Apr 25, 2017

facebook-github-bot commented Apr 27, 2017

facebook-github-bot commented Apr 28, 2017

facebook-github-bot commented Apr 28, 2017

facebook-github-bot commented May 19, 2017

facebook-github-bot commented May 19, 2017

facebook-github-bot commented May 23, 2017

shergin May 26, 2017

Choose a reason for hiding this comment

Feng999 commented Mar 28, 2017 •

edited

Loading