Summary:
This diff fixed an issue that caused regression in fb4a and React Native panel apps regarding scrolling behavior. When scrolling in either horizontal or vertical scroll view, if the consecutive touch interrupted the previous fling (post touch scrolling), the scroll view would block touch event from dispatching to the content view. Thus, the items in scroll view are not responding to touch events anymore.
The diff that caused this issue is D34627330 (https://github.com/facebook/react-native/commit/0368081858193d7c2537acd9080d11bb701ee98b). In that diff, I added code to cancel the scheduled post touch runnable when touch down is received in scroll view. That is expected as the post touch runnable is to handle snapping scroll case, where [an extra fling](https://fburl.com/code/7qza1ece) is triggered to make sure scroll view stops at the right position. When user touch the screen before the previous scroll fling finishes, this post processing is no longer needed -- the new touch should take full control of scroll view.
However, in D34627330 (https://github.com/facebook/react-native/commit/0368081858193d7c2537acd9080d11bb701ee98b), I failed to reset the runnable instance `mPostTouchRunnable` to null when cancelling it. This caused the future post touch handle logic [stops to run](https://fburl.com/code/lh8pi7l0), as it thinks the runnable is non-null and has been scheduled. This prevents fabric from receiving proper scroll state updates from android side, thus causing a state corruption and affects logic to decide where the scroll offset is and where the child item is after that.
This diff fixed the issue by resetting the runnable instance, as well as making sure if the runnable is already running and the extra fling starts, we are canceling that fling animation properly.
Changelog:
[Android][Fixed] - Fixed regression on content in scroll view not responding to touch when fling got interrupted
Reviewed By: ShikaSD
Differential Revision: D34734129
fbshipit-source-id: 7d7689d203ce76c59cd44e16e31582317bb409bd
Summary:
Typically, ReactTextView#setText is called via ReactTextViewManager#updateExtraData, but natively animated color changes bypass render and layout pass via direct call to SurfaceMountingManager#updateProps from UIManager#synchronouslyUpdateViewOnUIThread.
Thus, for animated color changes to get applied, we need to handle the color prop in ReactTextAnchorViewManager.
In addition, native driver updates are not synchronized with Fabric's mounting; if the native driver update happens before mount, the update is done in updateState.
Changelog:
[Android][Added] - Support animating text color with native driver
Reviewed By: mdvacca
Differential Revision: D34630294
fbshipit-source-id: c0f1e19c801c0e909e84387d623a6556ce6f2d67
Summary:
The issue to be fixed here is an edge case when in nested scroll view, for example vertical scroll wraps horizontal scroll, when user scrolls horizontally then move the joystick downwards, the horizontal scroll would bounce back. This is shown in the demo:
https://pxl.cl/209f8
The root cause of the issue is due to two workflows fight to set the scroll position and confusing the scroll change dispatcher to calculate fling velocity direction, which caused a wrong fling behavior. Check this log and description to get more details:
{F706620839}
To fix the issue, in this diff we overridden the `onInterceptTouchEvent` method in panel app scroll views, and make it return true when the previous scroll is still running. This effectively skips the drag slop in the default scroll view behavior, as we know the touch event should've been handled by the scroll view if it's still scrolling.
This way, the vertical scroll part on the vertical scroll view will not be dispatched to the horizontal scroll at all. This allows the nested scroll view to work separately.
Changelog:
[Android][Internal] - Share the post intercept touch event logic in scroll views
Reviewed By: javache
Differential Revision: D34627329
fbshipit-source-id: d500ec3bfbe349a45ef5c1360b5c8807c168f682
Summary:
This diff aims to fix an issue I found while working on T113264056. It's to address the [comment](https://www.internalfb.com/diff/D3207608 (https://github.com/facebook/react-native/commit/a3146e41a259175f0c79e3c213523a0fb21613d7)?dst_version_fbid=507451319451602&transaction_fbid=1615731455398227) in an old diff D3207608 (https://github.com/facebook/react-native/commit/a3146e41a259175f0c79e3c213523a0fb21613d7). That issue begins to surface after I made the change in the next diff, which is discussed in detail in the task description (T113385381).
The fix here is to cancel the post touch processing when a new touch down is received. This should've cancel any previous post touch processing as the new touch should take full control of scrolling.
Changelog:
[Android][Fixed] - Cancel post touch process when new touch is received
Reviewed By: javache
Differential Revision: D34627330
fbshipit-source-id: 1fb8cfc1a4d94349b5290915a0a9f190186f2af3
Summary:
Per Android docs (https://developer.android.com/reference/android/graphics/Paint#FILTER_BITMAP_FLAG):
> This should generally be on when drawing bitmaps, unless performance-bound (rendering to software canvas) or preferring pixelation artifacts to blurriness when scaling significantly.
Changelog:
[Android][Fixed] - Improve rendering of images when resampled and corner radius applied
Reviewed By: genkikondo
Differential Revision: D34551184
fbshipit-source-id: 649da53af816be829e5b7f8e2a100f07a8b412b7
Summary:
Provides an alternative way of diffing props on Android side. Instead of passing dynamic JSON values from JS, the new approach diff C++ props and serializes difference into a `MapBuffer`. Current implementation does this only for `RCTView` component to test performance, correctness and memory impact (as MapBuffers are supposed to be more compact and performant).
This way of passing props allows for additional alignment between platforms, as C++ props are now source of truth for the view state, similar to iOS. At the same time, changing serialization method requires reimplementing `ViewManager` codegen (done manually for now) to account for `MapBuffer`s.
Changelog: [Added][Android] - Added an experimental prop serialization path based on MapBuffer
Reviewed By: mdvacca
Differential Revision: D33735245
fbshipit-source-id: 1515b5fe92f6557ae55abe215ce48277c83974a6
Summary:
Was trying out some behaviour when using the MapBuffer experiment and fixed some small issues.
Changelog: [Internal]
Reviewed By: ShikaSD
Differential Revision: D34108859
fbshipit-source-id: 550ca0847419006ec17472cc4b70d38fc8d05396
Summary:
Brings the same fix https://www.internalfb.com/diff/D34015853 (https://github.com/facebook/react-native/commit/be260b9f479a3b55ee43d2959d2c49fd3c1eb4ac) for ReactScrollView to ReactHorizontalScrollView
When setting ScrollView's contentOffset, if the ScrollView hasn't been laid out yet when ReactHorizontalScrollViewManager.setContentOffset is called, then scroll position is never set properly. This is because the actual scroll offset (0, 0) was being passed into setPendingContentOffsets, instead of the desired scroll offset. Thus, when ReactHorizontalScrollView.onLayout gets called, ReactHorizontalScrollView.scrollTo gets called with (0, 0).
Changelog:
[Android][Fixed] - Fix ReactHorizontalScrollView contentOffset
Reviewed By: bvanderhoof
Differential Revision: D34246489
fbshipit-source-id: d923f7c9f136f7275d64bd658ffd5c2cc049d392
Summary:
Not setting locale for language/country neutral operation may cause bug depending on the default locale.
See https://docs.oracle.com/javase/7/docs/api/java/util/Locale.html#ROOT
Note: I am just searching for toLowerCase() and toUppercase() in my project's dependencies and send the same PR, in order to just be considered. Although I've seen the lack of explicit locale has caused issues for us, I am not sure if react-native is actually affected. I haven't checked for `String.format()` yet.
Example related issue: joltup/rn-fetch-blob#573
## Changelog
[Android] [Fixed] - Use root locale when converting string case.
Pull Request resolved: https://github.com/facebook/react-native/pull/33028
Reviewed By: ShikaSD
Differential Revision: D33943446
Pulled By: cortinico
fbshipit-source-id: d5be9392ea7c21a33436acac5b5e8c50b7c7e31e
Summary:
When setting ScrollView's contentOffset, if the ScrollView hasn't been laid out yet when ReactScrollViewManager.setContentOffset is called, then scroll position is never set properly. This is because the actual scroll offset (0, 0) was being passed into setPendingContentOffsets, instead of the desired scroll offset. Thus,
when ReactScrollView.onLayout gets called, ReactScrollView.scrollTo gets called with (0, 0).
Also updates out of date comments,
Changelog:
[Android][Fixed] - Fix ScrollView contentOffset
Reviewed By: ryancat
Differential Revision: D34015853
fbshipit-source-id: 84141a663fdb0ace2be7cef61f14944cb08125d1
Summary:
Adds support for Animated.Color with native driver for Android. Reads the native config for the rbga channel AnimatedNodes, and on update(), converts the values into an integer (0xaarrggbb)
Followup changes will include support for iOS and platform colors.
Changelog:
[Android][Added] - Support running animations with AnimatedColor with native driver
Reviewed By: javache
Differential Revision: D33833600
fbshipit-source-id: 2bf05c9715b603cf014ace09e9308b2bfd67f30a
Summary:
Per discussion in the previous diff D33672110, it's ok to add the `pointerEvents` prop to scrollview. This will help prevent scrolling on the ScrollView if pointerEvents is set to `box-none`, or `none`.
Corresponding doc changes are in https://github.com/facebook/react-native-website/pull/2936
Changelog:
[Android][Added] - Add new API in ScrollView and HorizontalScrollView to process pointerEvents prop.
Reviewed By: javache
Differential Revision: D33699223
fbshipit-source-id: 1cae5113e9e7d988fc4c4765c41d817a321804c4
Summary:
Making the native ViewConfig for RCTModalHostView match its static ViewConfig.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D33303419
fbshipit-source-id: 6fac237d670ee221ad867f79e54d5a3576c156da
Summary:
Serializes type information along with key/value in MapBuffer, asserting the data type on Java side during read. At the moment, accessing value with incorrect will result in a crash.
Other changes:
Adds a `getType` method to verify types before accessing them.
Removes `null` as a type, as just not inserting value and checking for its existence with `hasKey` is more optimal.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D33656841
fbshipit-source-id: 23a78daa0d84704aab141088b5dfe881e9609472
Summary:
In the case of T110060790, there is a JavaScript crash that causes RN teardown to race with a ScrollView event being emitted, which ends up being reported as the "real" cause of the crash.
This is not correct, and it's better to just ignore this case. If there's no EventDispatcher, it means something has gone wrong (usually teardown) before, RN is in an inconsistent state, and we should
not crash here or report here.
Changelog: [Android][Fixed] Fix crash from ScrollView that occurs while reporting an error from JS
Reviewed By: mdvacca
Differential Revision: D33644692
fbshipit-source-id: 41c3defde795b804239cc8401c8aff71d017d59d
Summary:
MapBuffer uses unsigned short in C++, but Java doesn't really have a type that represents that. That means that MapBuffer would be limited to max 32768 values instead of 65536, which doesn't make much sense as a limitation.
Considering weird (and usually not performant) handling of short values in Java in general, this change replaces them with ints, converting keys from short when needed with `key & 0xFFFF`.
Changelog: [Internal]
Reviewed By: javache
Differential Revision: D33595308
fbshipit-source-id: a7adde8a207bb4aa1d81d367ab5d7b41ace2e291
Summary:
This diff adds `overflowInset` values in RN Android. These values are used to give an enlarged boundary of a view group that also contains all its children layout. Here is [the post](https://fb.workplace.com/groups/yogalayout/permalink/2264980363573947/) that discuss more on why this is useful. I steal the pic in that post here as TLDR:
{F687030994}
In the above case, we will get overflowInset for ViewGroup A as something like `top: 0, right: -20, bottom: -20, left: 0`.
This has been added in the [Fabric core](https://fburl.com/code/f8c5tg7b) and [in IOS](https://fburl.com/code/vkh0hpt6). In Android, since we used to ignore all event coordinates outside of a ViewGroup boundary, this is not an issue. However, that caused unregistered touch area problem and got fixed in D30104853 (https://github.com/facebook/react-native/commit/e35a963bfb93bbbdd92f4dd74d14e2ad6df5e14a), which dropped the boundary check and made the hit test algorithm in [TouchTargetHelper.java](https://fburl.com/code/dj8jiz22) worse as we now need to explore all the child node under ReactRootNode.
This perf issue is getting obvious when a view loads too many items, which matches our experience with "Hover getting slow after scrolling", "Hover getting slow after going back from PDP view", and "The saved list view (in Explore) is very fast (because it has very few components)"
To fix this issue, I added the support to `overflowInset` to RN Android by
1. Sending the `overflowInset` values from Binding.cpp in RN Android as a separate mount instruction
2. Update `IntBufferBatchMountItem.java` to read the int buffer sent over JNI, and pass the `overflowInset` values to `SurfaceMountingManager.java`
3. Creating new interface `ReactOverflowViewWithInset.java` and extending the existing `ReactOverflowView.java` usages
4. Adding implementation of getter and setter for `overflowInset` in various views
5. Update `TouchTargetHelper.java` to read the values and check boundaries before exploring ViewGroup's children
Note that in #3 I didn't change `ReactOverflowView.java` interface directly. I am concerned about backward compatibility issues in case this interface is being used in OSS. I suggest we deprecate it as we are not using it anymore in our code.
Changelog:
[Internal][Android]
Reviewed By: JoshuaGross
Differential Revision: D33133977
fbshipit-source-id: 64e3e837fe7ca6e6dbdbc836ab0615182e10f28c
Summary:
Originally introduced in D2022018
Tried to make the processor optional when no rounding is required, but found even that was not strictly necessary.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D32675492
fbshipit-source-id: 8dfdbf0e4347155045f77b1fba00a59086fe7a33
Summary:
This diff add custom prediction for fling distance support. This is needed for customize fling animator to calculate predicted fling distance, instead of using the overscroller that may not be used by the animator.
More context on this -- when fling happens, our code will first predict the final fling position `p`, apply the snapping logic to decide the expected snapping position `pSnapping` given `p`, scroll velocity and children layout, then trigger the overscroller (existing) or custom fling animator to finish the fling.
Currently, the prediction logic is done with overscroller, and custom fling animator has no control over how the predicted fling distance should be. Changes in this diff allow the animator to override `getExtrapolatedDistance` method and provide that information.
Changelog:
[Android][Added] - Add new API for custom fling animator to provide predicted travel distance for its fling animation.
Reviewed By: mdvacca
Differential Revision: D32571734
fbshipit-source-id: d34b969206f8b6cb5c68d2f50a18749bfebbc97e
Summary:
This diff refactors method `predictFinalScrollPosition` in `ReactScrollView` and `ReactHorizontalScrollView` to the helper class. This will make future changes to the prediction logic easier.
Changelog:
[Internal]
Reviewed By: javache
Differential Revision: D32571735
fbshipit-source-id: 7e7e21ac51f929a017cd43de094ed39478fe4032
Summary:
This diff fixes an edge case where scroll to the end of the list may trigger a bounce back effect.
This issue is a regression from D32487846 (https://github.com/facebook/react-native/commit/f70018b37532622f08f20b2c51cdbfca55d730ea) (See [this comment](https://www.internalfb.com/diff/D32487846 (https://github.com/facebook/react-native/commit/f70018b37532622f08f20b2c51cdbfca55d730ea)?dst_version_fbid=263960175698224&transaction_fbid=566201141113715)) that zero velocity fling at the end of the scroll view makes the next fling animator use previous post animation position. This is due to cached `postAnimationValue` is applied mistakenly.
- Pass velocity instead of velocity sign to the helper class
- Update helper class logic to decide if we need to use post animated value from last fling animation
Changelog:
[Internal]
Reviewed By: javache
Differential Revision: D32566010
fbshipit-source-id: 1c61659030151f8f2c7648ca901b8b4158835538
Summary:
This diff fixes an edge case where fling and snap failed to find the correct target position when children views not fill up all the scrollable space. In this case, the target position would be calculated as the end of the scrollable space, which case the snap logic to go to the end of the scrollable area, instead of stop at the expected snapping position.
Changelog:
[Android][Fixed] - Fix fling and snap with recycler viewgroup where fling to the end of scrollable distance when it goes over current rendered children views.
Reviewed By: mdvacca
Differential Revision: D32565459
fbshipit-source-id: 319ef6e2d4e1c4deb9e45ed02c1bff7d807575c3
Summary:
The helper class for ScrollView should not need to detect the scroll direction. If it has to do so, that means the corresponding logic should live in the `ReactScrollView` or `ReactHorizontalScrollView` respectively.
This diff is to remove the scroll direction detection logic from the scroll view helper. Long term we should keep it this way as the shared code got moved to the helper class from the scroll view classes.
Changelog:
[Internal]
Reviewed By: javache
Differential Revision: D32514429
fbshipit-source-id: 2165f2eba90cc25d14834c39148fe8ce8805bea6
Summary:
This diff fixes two edge case (similar to a race condition) that caused unexpected behaviors.
**Problem one**
{F680816408}
The previous fling animation is not canceled when user starts to scroll or drag. This is causing both the animation and scroll are setting the scroll position. Depends on the animation path and scroll speed, there may be cases where the [velocity calculation](https://fburl.com/code/010lsu72) ends up getting reversed values. See P467905091 as an example where you can see `mXFlingVelocity` goes back and forth from positive to negative.
It's hard to see if the wrong values are in the middle, but if that happens in the end of user gesture, the velocity for the next fling would be wrong. It shows a "bounce back" effect, and can be triggered when user makes small quick joystick scrolls in one direction.
**Problem two**
{F680821494}
There is a gap between animator's `onAnimationEnd` lifecycle method [finished](https://fburl.com/code/6baq04ne) and the `Animator#isRunning` API to return false. This is causing issues for `getPostAnimationScrollX` where we [decide to return](https://fburl.com/code/hzzugvch) the animated final value or the scroll value. User may see the `-1` value got used for the next fling start value, and the whole scroll view goes back to the beginning of scroll view and starts to fling.
This happens when the previous fling animation finishes and the animated final value is set to -1, but at the same time the next fling starts before `isRunning` returns false for the previous animation.
**Solution**
The problems are fixed by
- Do not reset animated final value to -1 in `onAnimationEnd` method
- Add `mIsFinished` states and use it to track animation finish signal, instead of using `isRunning` API
- Update logic where we decide to return the correct value for the next animation starts point. We will return previous animated final value when the animation got canceled, and user is going towards that value from the current scroll value.
Changelog:
[Android][Fixed] - Fixed edge case for quick small scrolls causing unexpected scrolling behaviors.
Reviewed By: javache
Differential Revision: D32487846
fbshipit-source-id: f1b0647656e021390e3a05de5846251a4a2647ff
Summary:
This diff makes the fling animator cuztomizable, so that the subclasses can have their own animation for fling behavior.
Before the diff, we rely on the `OverScroller` to `fling`, which has some issues with Spline interpolation being messed up due to the clamped fling distance (See more details in T105464095). This may not be a big issue for mobile when user touches screen, but in VR environment this shows up very clearly with joystick events. To fix that properly without affecting mobile behavior, I added a new interface `HasFlingAnimator` from the helper, and implemented with default fling animator in the OSS ScrollView.
We should consider adopt a suitable animator for mobile platform, as the non-smooth fling effect is also happening in mobile.
- Add interface `HasFlingAnimator` to `ReactScrollView` and `ReactHorizontalScrollView`
- Add default fling animator
- Depend on if the default animator is used, customize the flingAndSnap behavior
Changelog:
[Internal]
Reviewed By: JoshuaGross
Differential Revision: D32382806
fbshipit-source-id: 08f03350f6a9b9fc03414b4dcb9977b9f33603ba
Summary:
This Diff fixes a crash happening as the user uses AppCompat 1.4.0
as a dependency in their App and uses a `TextInput` component.
The crash happens as `mFabricViewStateManager` is accessed during
the ctor of the superclass, and is not yet initialized.
Fixes#31572
Changelog:
[Android] [Fixed] - Fix crash on ReactEditText with AppCompat 1.4.0
Reviewed By: ShikaSD
Differential Revision: D32674975
fbshipit-source-id: efa413f5e33527a29fbcfa729e8b006ecb235978
Summary:
This diff refactors the scroll animation from `ReactScrollView` and `ReactHorizontalScrollView` into the `ReactScrollViewHelper` to reduce repeated code. The `Animator` is now shared between all the scroll views in the app, which I believe is the right behavior.
It also helps to make the animator changes in future diffs apply to both horizontal and vertical scroll view.
- Move `reactSmoothScrollTo` to `smoothScrollTo` in the helper class
- This means one Animator for all ScrollViews
- Move `updateStateOnScroll` to the helper class
- Add interface for accessing instance scroll state properties in ScrollView
- This means each ScrollView keeps their own scrolling state
- Use `Point` class for pairs of x and y values
Changelog:
[Internal]
Reviewed By: javache
Differential Revision: D32372180
fbshipit-source-id: 529180eea788863689c3b440191ed50c5a6f04e5
Summary:
Changelog:
[Android][Added] Adding new API `onChildEndedNativeGesture` to the RootView interface to let its implementations notify the JS side that a child gesture is ended.
Reviewed By: javache
Differential Revision: D32228745
fbshipit-source-id: ad1f26546dd60f9c5a569b0bc3ad5020a01b90cc
Summary:
When calculating the offset range, we assume the first item is always at offset zero position and skipped that (as the smallerOffset is zero). However, this may not be the case in some situations. This diff changes the range measuring loop to always start from the first item.
Changelog:
[Android][Fixed] - Do NOT skip the first child view in the scroll view group when measuring the lower and upper bounds for snapping.
Reviewed By: mdvacca
Differential Revision: D31887086
fbshipit-source-id: af7221a621b2719d057afa6b64aa91c94ac01295
Summary:
The `ReactScrollView.java` constructor is using `ReactContext` instead of `Context`, which is inconsistent to the horizontal scroll view. This is the result from D3863966 (https://github.com/facebook/react-native/commit/2cf2fdbc04bc5309f6942ed464ad61cd06a26ce8) when an OSS issue needs to be addressed. That issue and all call sites to use the `ReactContext` are deprecated now.
Revert this back to use `Context` to be less restrictive.
Changelog:
[Android][Fixed] - Revert `ReactScrollView` to use `Context` instead of `ReactContext` in the constructor to be less restrictive.
Reviewed By: javache
Differential Revision: D31819799
fbshipit-source-id: 3f00d64850aebd2e20615033b2e1f1c721fed37e
Genki Kondo
Xin Chen
caioagiani
David Vacca
Pieter De Baets
Andrei Shikov
Alfonso Curbelo
fabriziobertoglio1987
Hamid
Ramanpreet Nara
Joshua Gross
Andres Suarez
Nicola Corti
Liron Yahdav
Stefanos Markidis
