Unverified Commit 17c50693 authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

Rationalize the ScrollMetrics cloning logic (#14281)

parent d9ef7df9
...@@ -130,12 +130,41 @@ class FixedExtentScrollController extends ScrollController { ...@@ -130,12 +130,41 @@ class FixedExtentScrollController extends ScrollController {
/// This is distinct from `Fixed` in the parent class name's [FixedScrollMetric] /// This is distinct from `Fixed` in the parent class name's [FixedScrollMetric]
/// which refers to its immutability. /// which refers to its immutability.
class FixedExtentMetrics extends FixedScrollMetrics { class FixedExtentMetrics extends FixedScrollMetrics {
/// Creates fixed extent metrics that add the item index information to the /// Creates an immutable snapshot of values associated with a
/// `parent` [FixedScrollMetrics]. /// [ListWheelScrollView].
FixedExtentMetrics({ FixedExtentMetrics({
ScrollMetrics parent, @required double minScrollExtent,
@required double maxScrollExtent,
@required double pixels,
@required double viewportDimension,
@required AxisDirection axisDirection,
@required this.itemIndex, @required this.itemIndex,
}) : super.clone(parent); }) : super(
minScrollExtent: minScrollExtent,
maxScrollExtent: maxScrollExtent,
pixels: pixels,
viewportDimension: viewportDimension,
axisDirection: axisDirection,
);
@override
FixedExtentMetrics copyWith({
double minScrollExtent,
double maxScrollExtent,
double pixels,
double viewportDimension,
AxisDirection axisDirection,
int itemIndex,
}) {
return new FixedExtentMetrics(
minScrollExtent: minScrollExtent ?? this.minScrollExtent,
maxScrollExtent: maxScrollExtent ?? this.maxScrollExtent,
pixels: pixels ?? this.pixels,
viewportDimension: viewportDimension ?? this.viewportDimension,
axisDirection: axisDirection ?? this.axisDirection,
itemIndex: itemIndex ?? this.itemIndex,
);
}
/// The scroll view's currently selected item index. /// The scroll view's currently selected item index.
final int itemIndex; final int itemIndex;
...@@ -160,7 +189,7 @@ double _clipOffsetToScrollableRange( ...@@ -160,7 +189,7 @@ double _clipOffsetToScrollableRange(
/// A [ScrollPositionWithSingleContext] that can only be created based on /// A [ScrollPositionWithSingleContext] that can only be created based on
/// [_FixedExtentScrollable] and can access its `itemExtent` to derive [itemIndex]. /// [_FixedExtentScrollable] and can access its `itemExtent` to derive [itemIndex].
class _FixedExtentScrollPosition extends ScrollPositionWithSingleContext { class _FixedExtentScrollPosition extends ScrollPositionWithSingleContext implements FixedExtentMetrics {
_FixedExtentScrollPosition({ _FixedExtentScrollPosition({
@required ScrollPhysics physics, @required ScrollPhysics physics,
@required ScrollContext context, @required ScrollContext context,
...@@ -188,6 +217,7 @@ class _FixedExtentScrollPosition extends ScrollPositionWithSingleContext { ...@@ -188,6 +217,7 @@ class _FixedExtentScrollPosition extends ScrollPositionWithSingleContext {
double get itemExtent => _getItemExtentFromScrollContext(context); double get itemExtent => _getItemExtentFromScrollContext(context);
@override
int get itemIndex { int get itemIndex {
return _getItemFromOffset( return _getItemFromOffset(
offset: pixels, offset: pixels,
...@@ -198,8 +228,22 @@ class _FixedExtentScrollPosition extends ScrollPositionWithSingleContext { ...@@ -198,8 +228,22 @@ class _FixedExtentScrollPosition extends ScrollPositionWithSingleContext {
} }
@override @override
FixedExtentMetrics cloneMetrics() { FixedExtentMetrics copyWith({
return new FixedExtentMetrics(parent: this, itemIndex: itemIndex); double minScrollExtent,
double maxScrollExtent,
double pixels,
double viewportDimension,
AxisDirection axisDirection,
int itemIndex,
}) {
return new FixedExtentMetrics(
minScrollExtent: minScrollExtent ?? this.minScrollExtent,
maxScrollExtent: maxScrollExtent ?? this.maxScrollExtent,
pixels: pixels ?? this.pixels,
viewportDimension: viewportDimension ?? this.viewportDimension,
axisDirection: axisDirection ?? this.axisDirection,
itemIndex: itemIndex ?? this.itemIndex,
);
} }
} }
......
...@@ -425,6 +425,29 @@ class _NestedScrollMetrics extends FixedScrollMetrics { ...@@ -425,6 +425,29 @@ class _NestedScrollMetrics extends FixedScrollMetrics {
axisDirection: axisDirection, axisDirection: axisDirection,
); );
@override
_NestedScrollMetrics copyWith({
double minScrollExtent,
double maxScrollExtent,
double pixels,
double viewportDimension,
AxisDirection axisDirection,
double minRange,
double maxRange,
double correctionOffset,
}) {
return new _NestedScrollMetrics(
minScrollExtent: minScrollExtent ?? this.minScrollExtent,
maxScrollExtent: maxScrollExtent ?? this.maxScrollExtent,
pixels: pixels ?? this.pixels,
viewportDimension: viewportDimension ?? this.viewportDimension,
axisDirection: axisDirection ?? this.axisDirection,
minRange: minRange ?? this.minRange,
maxRange: maxRange ?? this.maxRange,
correctionOffset: correctionOffset ?? this.correctionOffset,
);
}
final double minRange; final double minRange;
final double maxRange; final double maxRange;
......
...@@ -178,18 +178,54 @@ class PageController extends ScrollController { ...@@ -178,18 +178,54 @@ class PageController extends ScrollController {
/// The metrics are available on [ScrollNotification]s generated from /// The metrics are available on [ScrollNotification]s generated from
/// [PageView]s. /// [PageView]s.
class PageMetrics extends FixedScrollMetrics { class PageMetrics extends FixedScrollMetrics {
/// Creates page metrics that add the given information to the `parent` /// Creates an immutable snapshot of values associated with a [PageView].
/// metrics.
PageMetrics({ PageMetrics({
ScrollMetrics parent, @required double minScrollExtent,
this.page, @required double maxScrollExtent,
}) : super.clone(parent); @required double pixels,
@required double viewportDimension,
@required AxisDirection axisDirection,
@required this.viewportFraction,
}) : super(
minScrollExtent: minScrollExtent,
maxScrollExtent: maxScrollExtent,
pixels: pixels,
viewportDimension: viewportDimension,
axisDirection: axisDirection,
);
@override
PageMetrics copyWith({
double minScrollExtent,
double maxScrollExtent,
double pixels,
double viewportDimension,
AxisDirection axisDirection,
double viewportFraction,
}) {
return new PageMetrics(
minScrollExtent: minScrollExtent ?? this.minScrollExtent,
maxScrollExtent: maxScrollExtent ?? this.maxScrollExtent,
pixels: pixels ?? this.pixels,
viewportDimension: viewportDimension ?? this.viewportDimension,
axisDirection: axisDirection ?? this.axisDirection,
viewportFraction: viewportFraction ?? this.viewportFraction,
);
}
/// The current page displayed in the [PageView]. /// The current page displayed in the [PageView].
final double page; double get page {
return math.max(0.0, pixels.clamp(minScrollExtent, maxScrollExtent)) /
math.max(1.0, viewportDimension * viewportFraction);
}
/// The fraction of the viewport that each page occupies.
///
/// Used to compute [page] from the current [pixels].
final double viewportFraction;
} }
class _PagePosition extends ScrollPositionWithSingleContext { class _PagePosition extends ScrollPositionWithSingleContext implements PageMetrics {
_PagePosition({ _PagePosition({
ScrollPhysics physics, ScrollPhysics physics,
ScrollContext context, ScrollContext context,
...@@ -214,6 +250,7 @@ class _PagePosition extends ScrollPositionWithSingleContext { ...@@ -214,6 +250,7 @@ class _PagePosition extends ScrollPositionWithSingleContext {
final int initialPage; final int initialPage;
double _pageToUseOnStartup; double _pageToUseOnStartup;
@override
double get viewportFraction => _viewportFraction; double get viewportFraction => _viewportFraction;
double _viewportFraction; double _viewportFraction;
set viewportFraction(double value) { set viewportFraction(double value) {
...@@ -233,6 +270,7 @@ class _PagePosition extends ScrollPositionWithSingleContext { ...@@ -233,6 +270,7 @@ class _PagePosition extends ScrollPositionWithSingleContext {
return page * viewportDimension * viewportFraction; return page * viewportDimension * viewportFraction;
} }
@override
double get page => pixels == null ? null : getPageFromPixels(pixels.clamp(minScrollExtent, maxScrollExtent), viewportDimension); double get page => pixels == null ? null : getPageFromPixels(pixels.clamp(minScrollExtent, maxScrollExtent), viewportDimension);
@override @override
...@@ -264,10 +302,21 @@ class _PagePosition extends ScrollPositionWithSingleContext { ...@@ -264,10 +302,21 @@ class _PagePosition extends ScrollPositionWithSingleContext {
} }
@override @override
PageMetrics cloneMetrics() { PageMetrics copyWith({
double minScrollExtent,
double maxScrollExtent,
double pixels,
double viewportDimension,
AxisDirection axisDirection,
double viewportFraction,
}) {
return new PageMetrics( return new PageMetrics(
parent: this, minScrollExtent: minScrollExtent ?? this.minScrollExtent,
page: page, maxScrollExtent: maxScrollExtent ?? this.maxScrollExtent,
pixels: pixels ?? this.pixels,
viewportDimension: viewportDimension ?? this.viewportDimension,
axisDirection: axisDirection ?? this.axisDirection,
viewportFraction: viewportFraction ?? this.viewportFraction,
); );
} }
} }
......
...@@ -36,7 +36,25 @@ abstract class ScrollMetrics { ...@@ -36,7 +36,25 @@ abstract class ScrollMetrics {
/// ///
/// This is useful if this object is mutable, but you want to get a snapshot /// This is useful if this object is mutable, but you want to get a snapshot
/// of the current state. /// of the current state.
ScrollMetrics cloneMetrics() => new FixedScrollMetrics.clone(this); ///
/// The named arguments allow the values to be adjusted in the process. This
/// is useful to examine hypothetical situations, for example "would applying
/// this delta unmodified take the position [outOfRange]?".
ScrollMetrics copyWith({
double minScrollExtent,
double maxScrollExtent,
double pixels,
double viewportDimension,
AxisDirection axisDirection,
}) {
return new FixedScrollMetrics(
minScrollExtent: minScrollExtent ?? this.minScrollExtent,
maxScrollExtent: maxScrollExtent ?? this.maxScrollExtent,
pixels: pixels ?? this.pixels,
viewportDimension: viewportDimension ?? this.viewportDimension,
axisDirection: axisDirection ?? this.axisDirection,
);
}
/// The minimum in-range value for [pixels]. /// The minimum in-range value for [pixels].
/// ///
...@@ -93,7 +111,6 @@ abstract class ScrollMetrics { ...@@ -93,7 +111,6 @@ abstract class ScrollMetrics {
/// An immutable snapshot of values associated with a [Scrollable] viewport. /// An immutable snapshot of values associated with a [Scrollable] viewport.
/// ///
/// For details, see [ScrollMetrics], which defines this object's interfaces. /// For details, see [ScrollMetrics], which defines this object's interfaces.
@immutable
class FixedScrollMetrics extends ScrollMetrics { class FixedScrollMetrics extends ScrollMetrics {
/// Creates an immutable snapshot of values associated with a [Scrollable] viewport. /// Creates an immutable snapshot of values associated with a [Scrollable] viewport.
FixedScrollMetrics({ FixedScrollMetrics({
...@@ -104,14 +121,6 @@ class FixedScrollMetrics extends ScrollMetrics { ...@@ -104,14 +121,6 @@ class FixedScrollMetrics extends ScrollMetrics {
@required this.axisDirection, @required this.axisDirection,
}); });
/// Creates an immutable snapshot of the given metrics.
FixedScrollMetrics.clone(ScrollMetrics parent) :
minScrollExtent = parent.minScrollExtent,
maxScrollExtent = parent.maxScrollExtent,
pixels = parent.pixels,
viewportDimension = parent.viewportDimension,
axisDirection = parent.axisDirection;
@override @override
final double minScrollExtent; final double minScrollExtent;
......
...@@ -589,19 +589,19 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics { ...@@ -589,19 +589,19 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
/// Called by [beginActivity] to report when an activity has started. /// Called by [beginActivity] to report when an activity has started.
void didStartScroll() { void didStartScroll() {
activity.dispatchScrollStartNotification(cloneMetrics(), context.notificationContext); activity.dispatchScrollStartNotification(copyWith(), context.notificationContext);
} }
/// Called by [setPixels] to report a change to the [pixels] position. /// Called by [setPixels] to report a change to the [pixels] position.
void didUpdateScrollPositionBy(double delta) { void didUpdateScrollPositionBy(double delta) {
activity.dispatchScrollUpdateNotification(cloneMetrics(), context.notificationContext, delta); activity.dispatchScrollUpdateNotification(copyWith(), context.notificationContext, delta);
} }
/// Called by [beginActivity] to report when an activity has ended. /// Called by [beginActivity] to report when an activity has ended.
/// ///
/// This also saves the scroll offset using [saveScrollOffset]. /// This also saves the scroll offset using [saveScrollOffset].
void didEndScroll() { void didEndScroll() {
activity.dispatchScrollEndNotification(cloneMetrics(), context.notificationContext); activity.dispatchScrollEndNotification(copyWith(), context.notificationContext);
if (keepScrollOffset) if (keepScrollOffset)
saveScrollOffset(); saveScrollOffset();
} }
...@@ -611,14 +611,14 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics { ...@@ -611,14 +611,14 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
/// not applied to the [pixels] value. /// not applied to the [pixels] value.
void didOverscrollBy(double value) { void didOverscrollBy(double value) {
assert(activity.isScrolling); assert(activity.isScrolling);
activity.dispatchOverscrollNotification(cloneMetrics(), context.notificationContext, value); activity.dispatchOverscrollNotification(copyWith(), context.notificationContext, value);
} }
/// Dispatches a notification that the [userScrollDirection] has changed. /// Dispatches a notification that the [userScrollDirection] has changed.
/// ///
/// Subclasses should call this function when they change [userScrollDirection]. /// Subclasses should call this function when they change [userScrollDirection].
void didUpdateScrollDirection(ScrollDirection direction) { void didUpdateScrollDirection(ScrollDirection direction) {
new UserScrollNotification(metrics: cloneMetrics(), context: context.notificationContext, direction: direction).dispatch(context.notificationContext); new UserScrollNotification(metrics: copyWith(), context: context.notificationContext, direction: direction).dispatch(context.notificationContext);
} }
@override @override
......
...@@ -712,4 +712,20 @@ void main() { ...@@ -712,4 +712,20 @@ void main() {
semantics.dispose(); semantics.dispose();
}); });
testWidgets('PageMetrics', (WidgetTester tester) async {
final PageMetrics page = new PageMetrics(
minScrollExtent: 100.0,
maxScrollExtent: 200.0,
pixels: 150.0,
viewportDimension: 25.0,
axisDirection: AxisDirection.right,
viewportFraction: 1.0,
);
expect(page.page, 6);
final PageMetrics page2 = page.copyWith(
pixels: page.pixels - 100.0,
);
expect(page2.page, 4.0);
});
} }
...@@ -48,7 +48,7 @@ void main() { ...@@ -48,7 +48,7 @@ void main() {
expect(behavior, isNotNull); expect(behavior, isNotNull);
expect(behavior.flag, isTrue); expect(behavior.flag, isTrue);
expect(position.physics, const isInstanceOf<ClampingScrollPhysics>()); expect(position.physics, const isInstanceOf<ClampingScrollPhysics>());
ScrollMetrics metrics = position.cloneMetrics(); ScrollMetrics metrics = position.copyWith();
expect(metrics.extentAfter, equals(400.0)); expect(metrics.extentAfter, equals(400.0));
expect(metrics.viewportDimension, equals(600.0)); expect(metrics.viewportDimension, equals(600.0));
...@@ -64,7 +64,7 @@ void main() { ...@@ -64,7 +64,7 @@ void main() {
expect(behavior.flag, isFalse); expect(behavior.flag, isFalse);
expect(position.physics, const isInstanceOf<BouncingScrollPhysics>()); expect(position.physics, const isInstanceOf<BouncingScrollPhysics>());
// Regression test for https://github.com/flutter/flutter/issues/5856 // Regression test for https://github.com/flutter/flutter/issues/5856
metrics = position.cloneMetrics(); metrics = position.copyWith();
expect(metrics.extentAfter, equals(400.0)); expect(metrics.extentAfter, equals(400.0));
expect(metrics.viewportDimension, equals(600.0)); expect(metrics.viewportDimension, equals(600.0));
}); });
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment