Skip to content
Closed
23 changes: 15 additions & 8 deletions Libraries/Components/ScrollResponder.js
Original file line number Diff line number Diff line change
Expand Up @@ -405,46 +405,53 @@ const ScrollResponderMixin = {
* This is currently used to help focus child TextViews, but can also
* be used to quickly scroll to any element we want to focus. Syntax:
*
* `scrollResponderScrollTo(options: {x: number = 0; y: number = 0; animated: boolean = true})`
* `scrollResponderScrollTo(options: {x: number = 0; y: number = 0; animated: boolean = true, duration: number = 0})`
*
* Note: The weird argument signature is due to the fact that, for historical reasons,
* the function also accepts separate arguments as as alternative to the options object.
* This is deprecated due to ambiguity (y before x), and SHOULD NOT BE USED.
*
* Also note "duration" is currently only supported for Android.
*/
scrollResponderScrollTo: function(
x?: number | { x?: number, y?: number, animated?: boolean },
x?: number | { x?: number, y?: number, animated?: boolean, duration?: number },
y?: number,
animated?: boolean
animated?: boolean,
duration?: number
) {
if (typeof x === 'number') {
console.warn('`scrollResponderScrollTo(x, y, animated)` is deprecated. Use `scrollResponderScrollTo({x: 5, y: 5, animated: true})` instead.');
} else {
({x, y, animated} = x || {});
({x, y, animated, duration} = x || {});
}
UIManager.dispatchViewManagerCommand(
nullthrows(this.scrollResponderGetScrollableNode()),
UIManager.RCTScrollView.Commands.scrollTo,
[x || 0, y || 0, animated !== false],
[x || 0, y || 0, animated !== false, duration || 0],
);
},

/**
* Scrolls to the end of the ScrollView, either immediately or with a smooth
* animation.
* animation. For Android, you may specify a "duration" number instead of the
* "animated" boolean.
*
* Example:
*
* `scrollResponderScrollToEnd({animated: true})`
* or for Android, you can do:
* `scrollResponderScrollToEnd({duration: 500})`
*/
scrollResponderScrollToEnd: function(
options?: { animated?: boolean },
options?: { animated?: boolean, duration?: boolean },
) {
// Default to true
const animated = (options && options.animated) !== false;
const duration = options ? options.duration || 0 : 0;
UIManager.dispatchViewManagerCommand(
this.scrollResponderGetScrollableNode(),
UIManager.RCTScrollView.Commands.scrollToEnd,
[animated],
[animated, duration],
);
},

Expand Down
22 changes: 15 additions & 7 deletions Libraries/Components/ScrollView/ScrollView.js
Original file line number Diff line number Diff line change
Expand Up @@ -526,29 +526,33 @@ const ScrollView = createReactClass({
},

/**
* Scrolls to a given x, y offset, either immediately or with a smooth animation.
* Scrolls to a given x, y offset, either immediately, with a smooth animation, or,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you open a PR that applies these same changes to https://github.com/facebook/react-native-website/blob/master/docs/scrollview.md ?

These comments are not synced. At some point we need to update these comments with links to the generated docs at http://facebook.github.io/react-native instead (I've done this with a few source files already but had not gotten to ScrollView yet).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

* for Android only, a custom animation duration time.
*
* Example:
*
* `scrollTo({x: 0, y: 0, animated: true})`
* `scrollTo({x: 0, y: 0, animated: true, duration: 0})`
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not a good example because it's unclear what this should do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a separate example just for duration.

*
* Note: The weird function signature is due to the fact that, for historical reasons,
* the function also accepts separate arguments as an alternative to the options object.
* This is deprecated due to ambiguity (y before x), and SHOULD NOT BE USED.
*
* Also note "duration" is currently only supported for Android.
*/
scrollTo: function(
y?: number | { x?: number, y?: number, animated?: boolean },
y?: number | { x?: number, y?: number, animated?: boolean, duration?: number },
x?: number,
animated?: boolean
animated?: boolean,
duration?: number
) {
if (typeof y === 'number') {
console.warn('`scrollTo(y, x, animated)` is deprecated. Use `scrollTo({x: 5, y: 5, ' +
'animated: true})` instead.');
} else {
({x, y, animated} = y || {});
({x, y, animated, duration} = y || {});
}
this.getScrollResponder().scrollResponderScrollTo(
{x: x || 0, y: y || 0, animated: animated !== false}
{x: x || 0, y: y || 0, animated: animated !== false, duration: duration || 0}
Copy link
Contributor

Choose a reason for hiding this comment

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

duration || 0 seems wrong - should probably just send the null. I could imagine people explicitly setting 0 as an equivalent to animated: false.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was following the existing pattern with this API where a default value is always provided.

In ReactScrollViewCommandHelper.java, a duration of zero is effectively ignored.

It's tricky, though, because we want to support the existing animated API so this is backwards compatible. So what happens if a user says "animated: true, duration: 0", or "animated: false, duration: 250"? In ReactScrollViewCommandHelper, I defer first to duration value. If duration is non-zero, we animate with that duration. Otherwise, if animated is true, we use the legacy animation duration (250).

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, thought I replied earlier.

I'm not too concerned about cases where users give ambiguous input. If a user says "animated: true, duration: 0" there is no right answer, so we can do whatever - undefined behavior.

If the user calls scrollTo({y, duration: 0}) it's very clear that they don't want it to animate, and we should respect that clear intention and not animate it rather than making them figure out they should set animated: false instead. At the very least we could add an invariant that catches that case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I see, I was missing the part where animated defaults to true in this code, and we'd probably want to preserve that, but then not respect it if a user passes duration: 0.

);
},

Expand All @@ -558,15 +562,19 @@ const ScrollView = createReactClass({
*
* Use `scrollToEnd({animated: true})` for smooth animated scrolling,
* `scrollToEnd({animated: false})` for immediate scrolling.
* For Android, you may specify a duration, e.g. `scrollToEnd({duration: 500})`
* for a controlled duration scroll.
* If no options are passed, `animated` defaults to true.
*/
scrollToEnd: function(
options?: { animated?: boolean },
options?: { animated?: boolean, duration?: number },
) {
// Default to true
const animated = (options && options.animated) !== false;
const duration = options ? options.duration || 0 : 0;
this.getScrollResponder().scrollResponderScrollToEnd({
animated: animated,
duration: duration
});
},

Expand Down
8 changes: 6 additions & 2 deletions React/Views/ScrollView/RCTScrollViewManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,9 @@ - (UIView *)view
RCT_EXPORT_METHOD(scrollTo:(nonnull NSNumber *)reactTag
offsetX:(CGFloat)x
offsetY:(CGFloat)y
animated:(BOOL)animated)
animated:(BOOL)animated
// TODO(dannycochran) Use the duration here for a ScrollView.
duration:(CGFloat __unused)duration)
{
[self.bridge.uiManager addUIBlock:
^(__unused RCTUIManager *uiManager, NSDictionary<NSNumber *, UIView *> *viewRegistry){
Expand All @@ -162,7 +164,9 @@ - (UIView *)view
}

RCT_EXPORT_METHOD(scrollToEnd:(nonnull NSNumber *)reactTag
animated:(BOOL)animated)
animated:(BOOL)animated
// TODO(dannycochran) Use the duration here for a ScrollView.
duration:(CGFloat __unused)duration)
{
[self.bridge.uiManager addUIBlock:
^(__unused RCTUIManager *uiManager, NSDictionary<NSNumber *, UIView *> *viewRegistry){
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@

package com.facebook.react.views.scroll;

import android.animation.ObjectAnimator;
import android.graphics.Color;
import android.util.DisplayMetrics;

import com.facebook.react.bridge.ReadableArray;
import com.facebook.react.module.annotations.ReactModule;
import com.facebook.react.uimanager.DisplayMetricsHolder;
Expand Down Expand Up @@ -43,6 +45,7 @@ public class ReactHorizontalScrollViewManager
};

private @Nullable FpsListener mFpsListener = null;
private @Nullable ObjectAnimator mAnimator = null;

public ReactHorizontalScrollViewManager() {
this(null);
Expand Down Expand Up @@ -138,8 +141,8 @@ public void flashScrollIndicators(ReactHorizontalScrollView scrollView) {
@Override
public void scrollTo(
ReactHorizontalScrollView scrollView, ReactScrollViewCommandHelper.ScrollToCommandData data) {
if (data.mAnimated) {
scrollView.smoothScrollTo(data.mDestX, data.mDestY);
if (data.mDuration > 0) {
animateScroll(scrollView, data.mDestX, data.mDestY, data.mDuration);
} else {
scrollView.scrollTo(data.mDestX, data.mDestY);
}
Expand All @@ -152,8 +155,8 @@ public void scrollToEnd(
// ScrollView always has one child - the scrollable area
int right =
scrollView.getChildAt(0).getWidth() + scrollView.getPaddingRight();
if (data.mAnimated) {
scrollView.smoothScrollTo(right, scrollView.getScrollY());
if (data.mDuration > 0) {
animateScroll(scrollView, right, scrollView.getScrollY(), data.mDuration);
} else {
scrollView.scrollTo(right, scrollView.getScrollY());
}
Expand Down Expand Up @@ -217,4 +220,11 @@ public void setBorderColor(ReactHorizontalScrollView view, int index, Integer co
float alphaComponent = color == null ? YogaConstants.UNDEFINED : (float) ((int)color >>> 24);
view.setBorderColor(SPACING_TYPES[index], rgbComponent, alphaComponent);
}

private void animateScroll(ReactHorizontalScrollView view, int mDestX, int mDestY, int mDuration) {
if (mAnimator != null) {
mAnimator.cancel();
}
mAnimator = ReactScrollViewHelper.animateScroll(view, mDestX, mDestY, mDuration);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@ public class ReactScrollViewCommandHelper {
public static final int COMMAND_SCROLL_TO_END = 2;
public static final int COMMAND_FLASH_SCROLL_INDICATORS = 3;

/**
* Prior to users being able to specify a duration when calling "scrollTo",
* they could specify an "animate" boolean, which would use Android's
* "smoothScrollTo" method, which defaulted to a 250 millisecond
* animation:
* https://developer.android.com/reference/android/widget/Scroller.html#startScroll
*/
public static final int LEGACY_ANIMATION_DURATION = 250;

public interface ScrollCommandHandler<T> {
void scrollTo(T scrollView, ScrollToCommandData data);
void scrollToEnd(T scrollView, ScrollToEndCommandData data);
Expand All @@ -34,22 +43,21 @@ public interface ScrollCommandHandler<T> {

public static class ScrollToCommandData {

public final int mDestX, mDestY;
public final boolean mAnimated;
public final int mDestX, mDestY, mDuration;

ScrollToCommandData(int destX, int destY, boolean animated) {
ScrollToCommandData(int destX, int destY, int duration) {
mDestX = destX;
mDestY = destY;
mAnimated = animated;
mDuration = duration;
}
}

public static class ScrollToEndCommandData {

public final boolean mAnimated;
public final int mDuration;

ScrollToEndCommandData(boolean animated) {
mAnimated = animated;
ScrollToEndCommandData(int duration) {
mDuration = duration;
}
}

Expand All @@ -75,13 +83,30 @@ public static <T> void receiveCommand(
case COMMAND_SCROLL_TO: {
int destX = Math.round(PixelUtil.toPixelFromDIP(args.getDouble(0)));
int destY = Math.round(PixelUtil.toPixelFromDIP(args.getDouble(1)));
boolean animated = args.getBoolean(2);
viewManager.scrollTo(scrollView, new ScrollToCommandData(destX, destY, animated));

// TODO(dannycochran) If the duration argument is specified and non-zero, use it,
// otherwise use the legacy "animated" boolean. Eventually this can be removed
// in favor of just looking at "duration".
int duration = 0;
if (args.size() == 4 && args.getDouble(3) > 0) {
duration = (int) Math.round(args.getDouble(3));
} else {
duration = args.getBoolean(2) ? LEGACY_ANIMATION_DURATION : 0;
}
viewManager.scrollTo(scrollView, new ScrollToCommandData(destX, destY, duration));
return;
}
case COMMAND_SCROLL_TO_END: {
boolean animated = args.getBoolean(0);
viewManager.scrollToEnd(scrollView, new ScrollToEndCommandData(animated));
// TODO(dannycochran) If the duration argument is specified and non-zero, use it,
// otherwise use the legacy "animated" boolean. Eventually this can be removed
// in favor of just looking at "duration".
int duration = 0;
if (args.size() == 2 && args.getDouble(1) > 0) {
duration = (int) Math.round(args.getDouble(1));
} else {
duration = args.getBoolean(2) ? LEGACY_ANIMATION_DURATION : 0;
}
viewManager.scrollToEnd(scrollView, new ScrollToEndCommandData(duration));
return;
}
case COMMAND_FLASH_SCROLL_INDICATORS:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@

package com.facebook.react.views.scroll;

import android.animation.ObjectAnimator;
import android.animation.PropertyValuesHolder;

import android.view.MotionEvent;
import android.view.View;
import android.view.ViewGroup;

Expand Down Expand Up @@ -96,4 +100,28 @@ public static int parseOverScrollMode(String jsOverScrollMode) {
throw new JSApplicationIllegalArgumentException("wrong overScrollMode: " + jsOverScrollMode);
}
}

/**
* Helper method for animating to a ScrollView position with a given duration,
* instead of using "smoothScrollTo", which does not expose a duration argument.
*/
public static ObjectAnimator animateScroll(final ViewGroup scrollView, int mDestX, int mDestY, int mDuration) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I have no idea if this Animator object is legit - @janicduplessis or @mdvacca?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It has support from API 11:

https://developer.android.com/reference/android/animation/ObjectAnimator.html

FWIW we've been using this ScrollView (via overwriting the ScrollView package in MainApplication.Java) in production for the past 6 weeks, in production / release. We also use it in a fairly expensive SectionList which does all sorts of crazy optimizations and is a good stress test for performance.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would still like signoff from someone that knows our android codebase...@janicduplessis, @mdvacca, or @axe-fb?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good.

PropertyValuesHolder scrollX = PropertyValuesHolder.ofInt("scrollX", mDestX);
PropertyValuesHolder scrollY = PropertyValuesHolder.ofInt("scrollY", mDestY);

final ObjectAnimator animator = ObjectAnimator.ofPropertyValuesHolder(scrollView, scrollX, scrollY);

// Cancel the animation if a user interacts with the ScrollView.
scrollView.setOnTouchListener(new View.OnTouchListener() {
@Override
public boolean onTouch(View v, MotionEvent event) {
scrollView.setOnTouchListener(null);
animator.cancel();
return false;
}
});

animator.setDuration(mDuration).start();
return animator;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@

package com.facebook.react.views.scroll;

import android.animation.ObjectAnimator;
import android.annotation.TargetApi;
import android.graphics.Color;

import com.facebook.react.bridge.ReadableArray;
import com.facebook.react.common.MapBuilder;
import com.facebook.react.module.annotations.ReactModule;
Expand Down Expand Up @@ -45,6 +47,7 @@ public class ReactScrollViewManager
};

private @Nullable FpsListener mFpsListener = null;
private @Nullable ObjectAnimator mAnimator = null;

public ReactScrollViewManager() {
this(null);
Expand Down Expand Up @@ -144,8 +147,8 @@ public void flashScrollIndicators(ReactScrollView scrollView) {
@Override
public void scrollTo(
ReactScrollView scrollView, ReactScrollViewCommandHelper.ScrollToCommandData data) {
if (data.mAnimated) {
scrollView.smoothScrollTo(data.mDestX, data.mDestY);
if (data.mDuration > 0) {
animateScroll(scrollView, data.mDestX, data.mDestY, data.mDuration);
} else {
scrollView.scrollTo(data.mDestX, data.mDestY);
}
Expand Down Expand Up @@ -205,8 +208,8 @@ public void scrollToEnd(
// ScrollView always has one child - the scrollable area
int bottom =
scrollView.getChildAt(0).getHeight() + scrollView.getPaddingBottom();
if (data.mAnimated) {
scrollView.smoothScrollTo(scrollView.getScrollX(), bottom);
if (data.mDuration > 0) {
animateScroll(scrollView, scrollView.getScrollX(), bottom, data.mDuration);
} else {
scrollView.scrollTo(scrollView.getScrollX(), bottom);
}
Expand All @@ -226,4 +229,11 @@ public static Map<String, Object> createExportedCustomDirectEventTypeConstants()
.put(ScrollEventType.MOMENTUM_END.getJSEventName(), MapBuilder.of("registrationName", "onMomentumScrollEnd"))
.build();
}

private void animateScroll(ReactScrollView view, int mDestX, int mDestY, int mDuration) {
if (mAnimator != null) {
mAnimator.cancel();
}
mAnimator = ReactScrollViewHelper.animateScroll(view, mDestX, mDestY, mDuration);
}
}