Replace `ScrollView.scrollTo()` API with something less ambiguous.

Summary:
public
The current `ScrollView.scrollTo()` API is confusing due to the `(y, x)` parameter order, and the boolean `animated` argument. E.g.

    ScrollView.scrollTo(5, 0, true) // what do these arguments mean?

This diff replaces the API with a configuration object, so the arguments are all explicit:

    ScrollView.scrollTo({x: 0, y: 5, animated: true}) // much better

The `scrollTo()` method checks the argument types, and provides backwards compatibility with the old argument format for now. Using the old API will generate a warning, and this will eventually be upgraded to an error.

Reviewed By: davidaurelio

Differential Revision: D2892287

fb-gh-sync-id: cec4d504242391267c6e863816b6180ced7a7d5e
This commit is contained in:
Nick Lockwood 2016-02-03 03:59:20 -08:00 committed by facebook-github-bot-4
parent 5f4390bf03
commit 6941c4e027
3 changed files with 89 additions and 26 deletions

View File

@ -19,6 +19,8 @@ var React = require('react-native');
var { var {
ScrollView, ScrollView,
StyleSheet, StyleSheet,
Text,
TouchableOpacity,
View, View,
Image Image
} = React; } = React;
@ -31,27 +33,45 @@ exports.examples = [
title: '<ScrollView>', title: '<ScrollView>',
description: 'To make content scrollable, wrap it within a <ScrollView> component', description: 'To make content scrollable, wrap it within a <ScrollView> component',
render: function() { render: function() {
var _scrollView: ScrollView;
return ( return (
<ScrollView <View>
automaticallyAdjustContentInsets={false} <ScrollView
onScroll={() => { console.log('onScroll!'); }} ref={(scrollView) => { _scrollView = scrollView; }}
scrollEventThrottle={200} automaticallyAdjustContentInsets={false}
style={styles.scrollView}> onScroll={() => { console.log('onScroll!'); }}
{THUMBS.map(createThumbRow)} scrollEventThrottle={200}
</ScrollView> style={styles.scrollView}>
{THUMBS.map(createThumbRow)}
</ScrollView>
<TouchableOpacity
style={styles.button}
onPress={() => { _scrollView.scrollTo({y: 0}); }}>
<Text>Scroll to top</Text>
</TouchableOpacity>
</View>
); );
} }
}, { }, {
title: '<ScrollView> (horizontal = true)', title: '<ScrollView> (horizontal = true)',
description: 'You can display <ScrollView>\'s child components horizontally rather than vertically', description: 'You can display <ScrollView>\'s child components horizontally rather than vertically',
render: function() { render: function() {
var _scrollView: ScrollView;
return ( return (
<ScrollView <View>
automaticallyAdjustContentInsets={false} <ScrollView
horizontal={true} ref={(scrollView) => { _scrollView = scrollView; }}
style={[styles.scrollView, styles.horizontalScrollView]}> automaticallyAdjustContentInsets={false}
{THUMBS.map(createThumbRow)} horizontal={true}
</ScrollView> style={[styles.scrollView, styles.horizontalScrollView]}>
{THUMBS.map(createThumbRow)}
</ScrollView>
<TouchableOpacity
style={styles.button}
onPress={() => { _scrollView.scrollTo({x: 0}); }}>
<Text>Scroll to start</Text>
</TouchableOpacity>
</View>
); );
} }
}]; }];

View File

@ -349,14 +349,29 @@ var ScrollResponderMixin = {
/** /**
* A helper function to scroll to a specific point in the scrollview. * A helper function to scroll to a specific point in the scrollview.
* This is currently used to help focus on child textviews, but this * This is currently used to help focus on child textviews, but can also
* can also be used to quickly scroll to any element we want to focus * be used to quickly scroll to any element we want to focus. Syntax:
*
* scrollResponderScrollTo(options: {x: number = 0; y: number = 0; animated: boolean = true})
*
* 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.
*/ */
scrollResponderScrollTo: function(offsetX: number, offsetY: number, animated: boolean = true) { scrollResponderScrollTo: function(
x?: number | { x?: number; y?: number; animated?: boolean },
y?: number,
animated?: boolean
) {
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 || {});
}
UIManager.dispatchViewManagerCommand( UIManager.dispatchViewManagerCommand(
React.findNodeHandle(this), React.findNodeHandle(this),
UIManager.RCTScrollView.Commands.scrollTo, UIManager.RCTScrollView.Commands.scrollTo,
[offsetX, offsetY, animated], [x || 0, y || 0, animated !== false],
); );
}, },
@ -369,15 +384,24 @@ var ScrollResponderMixin = {
}, },
/** /**
* A helper function to zoom to a specific rect in the scrollview. * A helper function to zoom to a specific rect in the scrollview. The argument has the shape
* @param {object} rect Should have shape {x, y, width, height} * {x: number; y: number; width: number; height: number; animated: boolean = true}
* @param {bool} animated Specify whether zoom is instant or animated *
* @platform ios
*/ */
scrollResponderZoomTo: function(rect: { x: number; y: number; width: number; height: number; }, animated: boolean = true) { scrollResponderZoomTo: function(
rect: { x: number; y: number; width: number; height: number; animated?: boolean },
animated?: boolean // deprecated, put this inside the rect argument instead
) {
if (Platform.OS === 'android') { if (Platform.OS === 'android') {
invariant('zoomToRect is not implemented'); invariant('zoomToRect is not implemented');
} else { } else {
ScrollViewManager.zoomToRect(React.findNodeHandle(this), rect, animated); if ('animated' in rect) {
var { animated, ...rect } = rect;
} else if (typeof animated !== 'undefined') {
console.warn('`scrollResponderZoomTo` `animated` argument is deprecated. Use `options.animated` instead');
}
ScrollViewManager.zoomToRect(React.findNodeHandle(this), rect, animated !== false);
} }
}, },

View File

@ -359,17 +359,36 @@ var ScrollView = React.createClass({
return React.findNodeHandle(this.refs[INNERVIEW]); return React.findNodeHandle(this.refs[INNERVIEW]);
}, },
scrollTo: function(destY: number = 0, destX: number = 0, animated: boolean = true) { /**
* Scrolls to a given x, y offset, either immediately or with a smooth animation.
* Syntax:
*
* scrollTo(options: {x: number = 0; y: number = 0; animated: boolean = true})
*
* 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.
*/
scrollTo: function(
y?: number | { x?: number, y?: number, animated?: boolean },
x?: number,
animated?: boolean
) {
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 || {});
}
// $FlowFixMe - Don't know how to pass Mixin correctly. Postpone for now // $FlowFixMe - Don't know how to pass Mixin correctly. Postpone for now
this.getScrollResponder().scrollResponderScrollTo(destX, destY, animated); this.getScrollResponder().scrollResponderScrollTo({x: x || 0, y: y || 0, animated: animated !== false});
}, },
/** /**
* Deprecated, do not use. * Deprecated, do not use.
*/ */
scrollWithoutAnimationTo: function(destY: number = 0, destX: number = 0) { scrollWithoutAnimationTo: function(y: number = 0, x: number = 0) {
console.warn('`scrollWithoutAnimationTo` is deprecated. Use `scrollTo` instead'); console.warn('`scrollWithoutAnimationTo` is deprecated. Use `scrollTo` instead');
this.scrollTo(destX, destY, false); this.scrollTo({x, y, animated: false});
}, },
handleScroll: function(e: Object) { handleScroll: function(e: Object) {