Commit fc83640c authored by Hans Muller's avatar Hans Muller Committed by GitHub

ClampOverscrolls clamps Scrollable, not its Viewport (#5909)

parent 035afc2c
......@@ -82,7 +82,7 @@ void main() {
await tester.scroll(findGalleryItemByRouteName(tester, routeName), new Offset(0.0, scrollDeltas[i]));
await tester.pump(); // start the scroll
await tester.pump(const Duration(milliseconds: 500)); // wait for overscroll to timeout, if necessary
await tester.pump(const Duration(milliseconds: 2000)); // wait for overscroll to fade away, if necessary
await tester.pump(const Duration(seconds: 3)); // wait for overscroll to fade away, if necessary
tester.binding.debugAssertNoTransientCallbacks('A transient callback was still active after leaving route $routeName');
}
......
......@@ -151,7 +151,7 @@ class _OverscrollIndicatorState extends State<OverscrollIndicator> {
return;
final ExtentScrollBehavior scrollBehavior = scrollable.scrollBehavior;
_scrollDirection = scrollable.config.scrollDirection;
_scrollOffset = scrollable.scrollOffset;
_scrollOffset = scrollable.virtualScrollOffset;
_minScrollOffset = scrollBehavior.minScrollOffset;
_maxScrollOffset = scrollBehavior.maxScrollOffset;
}
......@@ -166,7 +166,7 @@ class _OverscrollIndicatorState extends State<OverscrollIndicator> {
if (!_scrollUnderway) // The hide timer has run.
return;
final double value = scrollable.scrollOffset;
final double value = scrollable.virtualScrollOffset;
if (_isOverscroll(value)) {
_refreshHideTimer();
// Hide the indicator as soon as user starts scrolling in the reverse direction of overscroll.
......
......@@ -83,9 +83,10 @@ class ClampOverscrolls extends InheritedWidget {
/// constraints are applied.
final ScrollableEdge edge;
/// Return the [scrollable]'s scrollOffset clamped according to [edge].
double clampScrollOffset(ScrollableState scrollable) {
final double scrollOffset = scrollable.scrollOffset;
/// Return the [newScrollOffset] clamped according to [edge] and [scrollable]'s
/// scroll behavior. The value of [newScrollOffset] defaults to `scrollable.scrollOffset`.
double clampScrollOffset(ScrollableState scrollable, [double newScrollOffset]) {
final double scrollOffset = newScrollOffset ?? scrollable.scrollOffset;
final double minScrollOffset = scrollable.scrollBehavior.minScrollOffset;
final double maxScrollOffset = scrollable.scrollBehavior.maxScrollOffset;
switch (edge) {
......@@ -106,29 +107,6 @@ class ClampOverscrolls extends InheritedWidget {
return context.inheritFromWidgetOfExactType(ClampOverscrolls);
}
/// Clamps the new viewport's scroll offset according to the value of
/// `ClampOverscrolls.of(context).edge`.
///
/// The clamped overscroll edge is reset to [ScrollableEdge.none] for the viewport's
/// descendants.
///
/// This utility function is typically used by [Scrollable.builder] callbacks.
static Widget buildViewport(BuildContext context, ScrollableState state, ViewportBuilder builder) {
// TODO(ianh): minScrollOffset and maxScrollOffset are typically determined
// by the container and content size. But we don't know those until we
// layout the viewport, which happens after build phase. We need to rethink
// this.
final ClampOverscrolls clampOverscrolls = ClampOverscrolls.of(context);
if (clampOverscrolls == null)
return builder(context, state, state.scrollOffset);
final double clampedScrollOffset = clampOverscrolls.clampScrollOffset(state);
Widget viewport = builder(context, state, clampedScrollOffset);
if (clampOverscrolls.edge != ScrollableEdge.none)
viewport = new ClampOverscrolls(edge: ScrollableEdge.none, child: viewport);
return viewport;
}
@override
bool updateShouldNotify(ClampOverscrolls old) => edge != old.edge;
......
......@@ -8,7 +8,6 @@ import 'package:flutter/rendering.dart';
import 'package:meta/meta.dart';
import 'basic.dart';
import 'clamp_overscrolls.dart';
import 'framework.dart';
import 'scroll_configuration.dart';
import 'scrollable.dart';
......@@ -259,9 +258,9 @@ class LazyBlock extends StatelessWidget {
/// See [LazyBlockDelegate] for details.
final LazyBlockDelegate delegate;
Widget _buildViewport(BuildContext context, ScrollableState state, double scrollOffset) {
Widget _buildViewport(BuildContext context, ScrollableState state) {
return new LazyBlockViewport(
startOffset: scrollOffset,
startOffset: state.scrollOffset,
mainAxis: scrollDirection,
padding: padding,
onExtentsChanged: (int firstIndex, int lastIndex, double firstStartOffset, double lastEndOffset, double minScrollOffset, double containerExtent) {
......@@ -278,10 +277,6 @@ class LazyBlock extends StatelessWidget {
);
}
Widget _buildContent(BuildContext context, ScrollableState state) {
return ClampOverscrolls.buildViewport(context, state, _buildViewport);
}
@override
Widget build(BuildContext context) {
final Widget result = new Scrollable(
......@@ -292,7 +287,7 @@ class LazyBlock extends StatelessWidget {
onScroll: onScroll,
onScrollEnd: onScrollEnd,
snapOffsetCallback: snapOffsetCallback,
builder: _buildContent
builder: _buildViewport
);
return ScrollConfiguration.wrap(context, result);
}
......
......@@ -270,12 +270,14 @@ class ScrollableState<T extends Scrollable> extends State<T> {
..addListener(_handleAnimationChanged)
..addStatusListener(_handleAnimationStatusChanged);
_scrollOffset = PageStorage.of(context)?.readState(context) ?? config.initialScrollOffset ?? 0.0;
_virtualScrollOffset = _scrollOffset;
}
Simulation _simulation; // if we're flinging, then this is the animation with which we're doing it
AnimationController _controller;
double _contentExtent;
double _containerExtent;
bool _scrollUnderway = false;
@override
void dispose() {
......@@ -300,9 +302,31 @@ class ScrollableState<T extends Scrollable> extends State<T> {
/// The scroll offset is applied to the child widget along the scroll
/// direction before painting. A positive scroll offset indicates that
/// more content in the preferred reading direction is visible.
///
/// The scroll offset's value may be above or below the limits defined
/// by the [scrollBehavior]. This is called "overscrolling" and it can be
/// prevented with the [ClampOverscrolls] widget.
///
/// See also:
///
/// * [virtualScrollOffset]
/// * [initialScrollOffset]
/// * [onScrollStart]
/// * [onScroll]
/// * [onScrollEnd]
/// * [ScrollNotification]
double get scrollOffset => _scrollOffset;
double _scrollOffset;
/// The current scroll offset, irrespective of the constraints defined
/// by any [ClampOverscrolls] widget ancestors.
///
/// See also:
///
/// * [scrollOffset]
double get virtualScrollOffset => _virtualScrollOffset;
double _virtualScrollOffset;
/// Convert a position or velocity measured in terms of pixels to a scrollOffset.
/// Scrollable gesture handlers convert their incoming values with this method.
/// Subclasses that define scrollOffset in units other than pixels must
......@@ -387,7 +411,7 @@ class ScrollableState<T extends Scrollable> extends State<T> {
bool _scrollOffsetIsInBounds(double scrollOffset) {
if (scrollBehavior is! ExtentScrollBehavior)
return false;
ExtentScrollBehavior behavior = scrollBehavior;
final ExtentScrollBehavior behavior = scrollBehavior;
return scrollOffset >= behavior.minScrollOffset && scrollOffset < behavior.maxScrollOffset;
}
......@@ -398,16 +422,23 @@ class ScrollableState<T extends Scrollable> extends State<T> {
void _handleAnimationStatusChanged(AnimationStatus status) {
// this is not called when stop() is called on the controller
setState(() {
if (!_controller.isAnimating)
if (!_controller.isAnimating) {
_simulation = null;
_scrollUnderway = false;
}
});
}
void _setScrollOffset(double newScrollOffset, { DragUpdateDetails details }) {
if (_scrollOffset == newScrollOffset)
return;
final ClampOverscrolls clampOverscrolls = ClampOverscrolls.of(context);
final double clampedScrollOffset = clampOverscrolls?.clampScrollOffset(this, newScrollOffset) ?? newScrollOffset;
setState(() {
_scrollOffset = newScrollOffset;
_virtualScrollOffset = newScrollOffset;
_scrollUnderway = _scrollOffset != clampedScrollOffset;
_scrollOffset = clampedScrollOffset;
});
PageStorage.of(context)?.writeState(context, _scrollOffset);
_startScroll();
......@@ -429,7 +460,7 @@ class ScrollableState<T extends Scrollable> extends State<T> {
Curve curve: Curves.ease,
DragUpdateDetails details
}) {
double newScrollOffset = scrollBehavior.applyCurve(_scrollOffset, scrollDelta);
double newScrollOffset = scrollBehavior.applyCurve(virtualScrollOffset, scrollDelta);
return scrollTo(newScrollOffset, duration: duration, curve: curve, details: details);
}
......@@ -461,7 +492,7 @@ class ScrollableState<T extends Scrollable> extends State<T> {
Future<Null> _animateTo(double newScrollOffset, Duration duration, Curve curve) {
_stop();
_controller.value = scrollOffset;
_controller.value = virtualScrollOffset;
_startScroll();
return _controller.animateTo(newScrollOffset, duration: duration, curve: curve).then((Null _) {
_endScroll();
......@@ -526,7 +557,8 @@ class ScrollableState<T extends Scrollable> extends State<T> {
// If a scroll animation isn't underway already and we're overscrolled or we're
// going to have to snap the scroll offset, then animate the scroll offset to its
// final value.
if (!_controller.isAnimating && (shouldSnapScrollOffset || !_scrollOffsetIsInBounds(scrollOffset)))
if (!_controller.isAnimating &&
(shouldSnapScrollOffset || !_scrollOffsetIsInBounds(scrollOffset)))
return settleScrollOffset();
return new Future<Null>.value();
......@@ -573,14 +605,14 @@ class ScrollableState<T extends Scrollable> extends State<T> {
if (endScrollOffset.isNaN)
return null;
final double snappedScrollOffset = snapScrollOffset(endScrollOffset); // Calls the config.snapOffsetCallback callback
final double snappedScrollOffset = snapScrollOffset(endScrollOffset);
if (!_scrollOffsetIsInBounds(snappedScrollOffset))
return null;
final double snapVelocity = scrollVelocity.abs() * (snappedScrollOffset - scrollOffset).sign;
final double endVelocity = pixelOffsetToScrollOffset(kPixelScrollTolerance.velocity).abs() * (scrollVelocity < 0.0 ? -1.0 : 1.0);
Simulation toSnapSimulation = scrollBehavior.createSnapScrollSimulation(
scrollOffset, snappedScrollOffset, snapVelocity, endVelocity
virtualScrollOffset, snappedScrollOffset, snapVelocity, endVelocity
);
if (toSnapSimulation == null)
return null;
......@@ -591,7 +623,7 @@ class ScrollableState<T extends Scrollable> extends State<T> {
}
Simulation _createFlingSimulation(double scrollVelocity) {
final Simulation simulation = scrollBehavior.createScrollSimulation(scrollOffset, scrollVelocity);
final Simulation simulation = scrollBehavior.createScrollSimulation(virtualScrollOffset, scrollVelocity);
if (simulation != null) {
final double endVelocity = pixelOffsetToScrollOffset(kPixelScrollTolerance.velocity).abs();
final double endDistance = pixelOffsetToScrollOffset(kPixelScrollTolerance.distance).abs();
......@@ -672,6 +704,14 @@ class ScrollableState<T extends Scrollable> extends State<T> {
_numberOfInProgressScrolls -= 1;
if (_numberOfInProgressScrolls == 0) {
_simulation = null;
if (_scrollUnderway && mounted) {
// If the scroll hasn't already stopped because we've hit a clamped
// edge or the controller stopped animating, then rebuild the Scrollable
// with the IgnorePointer widget turned off.
setState(() {
_scrollUnderway = false;
});
}
dispatchOnScrollEnd();
if (mounted) {
new ScrollNotification(
......@@ -701,7 +741,7 @@ class ScrollableState<T extends Scrollable> extends State<T> {
gestures: buildGestureDetectors(),
behavior: HitTestBehavior.opaque,
child: new IgnorePointer(
ignoring: _controller.isAnimating,
ignoring: _scrollUnderway,
child: buildContent(context)
)
);
......@@ -940,9 +980,9 @@ class ScrollableViewport extends StatelessWidget {
/// The widget that will be scrolled. It will become the child of a Scrollable.
final Widget child;
Widget _buildViewport(BuildContext context, ScrollableState state, double scrollOffset) {
Widget _buildViewport(BuildContext context, ScrollableState state) {
return new Viewport(
paintOffset: state.scrollOffsetToPixelDelta(scrollOffset),
paintOffset: state.scrollOffsetToPixelDelta(state.scrollOffset),
mainAxis: scrollDirection,
anchor: scrollAnchor,
onPaintOffsetUpdateNeeded: (ViewportDimensions dimensions) {
......@@ -955,10 +995,6 @@ class ScrollableViewport extends StatelessWidget {
);
}
Widget _buildContent(BuildContext context, ScrollableState state) {
return ClampOverscrolls.buildViewport(context, state, _buildViewport);
}
@override
Widget build(BuildContext context) {
final Widget result = new Scrollable(
......@@ -970,7 +1006,7 @@ class ScrollableViewport extends StatelessWidget {
onScroll: onScroll,
onScrollEnd: onScrollEnd,
snapOffsetCallback: snapOffsetCallback,
builder: _buildContent
builder: _buildViewport
);
return ScrollConfiguration.wrap(context, result);
}
......
......@@ -8,7 +8,6 @@ import 'package:collection/collection.dart' show lowerBound;
import 'package:flutter/rendering.dart';
import 'package:meta/meta.dart';
import 'clamp_overscrolls.dart';
import 'framework.dart';
import 'scroll_configuration.dart';
import 'scrollable.dart';
......@@ -81,19 +80,15 @@ class ScrollableGrid extends StatelessWidget {
/// The children that will be placed in the grid.
final Iterable<Widget> children;
Widget _buildViewport(BuildContext context, ScrollableState state, double scrollOffset) {
Widget _buildViewport(BuildContext context, ScrollableState state) {
return new GridViewport(
scrollOffset: scrollOffset,
scrollOffset: state.scrollOffset,
delegate: delegate,
onExtentsChanged: state.handleExtentsChanged,
children: children
);
}
Widget _buildContent(BuildContext context, ScrollableState state) {
return ClampOverscrolls.buildViewport(context, state, _buildViewport);
}
@override
Widget build(BuildContext context) {
final Widget result = new Scrollable(
......@@ -107,7 +102,7 @@ class ScrollableGrid extends StatelessWidget {
onScroll: onScroll,
onScrollEnd: onScrollEnd,
snapOffsetCallback: snapOffsetCallback,
builder: _buildContent
builder: _buildViewport,
);
return ScrollConfiguration.wrap(context, result);
}
......
......@@ -7,7 +7,6 @@ import 'dart:math' as math;
import 'package:flutter/rendering.dart';
import 'package:meta/meta.dart';
import 'clamp_overscrolls.dart';
import 'framework.dart';
import 'scroll_configuration.dart';
import 'scrollable.dart';
......@@ -126,12 +125,12 @@ class ScrollableList extends StatelessWidget {
/// The children, some of which might be materialized.
final Iterable<Widget> children;
Widget _buildViewport(BuildContext context, ScrollableState state, double scrollOffset) {
Widget _buildViewport(BuildContext context, ScrollableState state) {
return new ListViewport(
onExtentsChanged: (double contentExtent, double containerExtent) {
state.handleExtentsChanged(itemsWrap ? double.INFINITY : contentExtent, containerExtent);
},
scrollOffset: scrollOffset,
scrollOffset: state.scrollOffset,
mainAxis: scrollDirection,
anchor: scrollAnchor,
itemExtent: itemExtent,
......@@ -141,10 +140,6 @@ class ScrollableList extends StatelessWidget {
);
}
Widget _buildContent(BuildContext context, ScrollableState state) {
return ClampOverscrolls.buildViewport(context, state, _buildViewport);
}
@override
Widget build(BuildContext context) {
final Widget result = new Scrollable(
......@@ -156,7 +151,7 @@ class ScrollableList extends StatelessWidget {
onScroll: onScroll,
onScrollEnd: onScrollEnd,
snapOffsetCallback: snapOffsetCallback,
builder: _buildContent
builder: _buildViewport
);
return ScrollConfiguration.wrap(context, result);
}
......@@ -529,10 +524,10 @@ class ScrollableLazyList extends StatelessWidget {
/// The amount of space by which to inset the children inside the viewport.
final EdgeInsets padding;
Widget _buildViewport(BuildContext context, ScrollableState state, double scrollOffset) {
Widget _buildViewport(BuildContext context, ScrollableState state) {
return new LazyListViewport(
onExtentsChanged: state.handleExtentsChanged,
scrollOffset: scrollOffset,
scrollOffset: state.scrollOffset,
mainAxis: scrollDirection,
anchor: scrollAnchor,
itemExtent: itemExtent,
......@@ -542,10 +537,6 @@ class ScrollableLazyList extends StatelessWidget {
);
}
Widget _buildContent(BuildContext context, ScrollableState state) {
return ClampOverscrolls.buildViewport(context, state, _buildViewport);
}
@override
Widget build(BuildContext context) {
final Widget result = new Scrollable(
......@@ -557,7 +548,7 @@ class ScrollableLazyList extends StatelessWidget {
onScroll: onScroll,
onScrollEnd: onScrollEnd,
snapOffsetCallback: snapOffsetCallback,
builder: _buildContent
builder: _buildViewport
);
return ScrollConfiguration.wrap(context, result);
}
......
......@@ -6,7 +6,7 @@ import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
void main() {
testWidgets('tap-select an day', (WidgetTester tester) async {
testWidgets('tap-select a day', (WidgetTester tester) async {
Key _datePickerKey = new UniqueKey();
DateTime _selectedDate = new DateTime(2016, DateTime.JULY, 26);
......
......@@ -45,6 +45,9 @@ void main() {
return new Future<Point>.value(widgetOrigin);
}
// Each of the blocks below test overscrolling the top and bottom
// with a value for ClampOverscrolls.edge.
await tester.pumpWidget(buildFrame(ScrollableEdge.none));
Point origin = await locationAfterScroll('top', const Offset(0.0, 400.0));
expect(origin.y, greaterThan(0.0));
......@@ -70,4 +73,77 @@ void main() {
origin = await locationAfterScroll('bottom', const Offset(0.0, -400.0));
expect(origin.y, equals(500.0));
});
testWidgets('ClampOverscrolls affects scrollOffset not virtualScrollOffset', (WidgetTester tester) async {
// ClampOverscrolls.edge == ScrollableEdge.none
await tester.pumpWidget(buildFrame(ScrollableEdge.none));
StatefulElement statefulElement = tester.element(find.byType(Scrollable));
ScrollableState scrollable = statefulElement.state;
await tester.scrollAt(tester.getTopLeft(find.text('top')), const Offset(0.0, 400.0));
await tester.pump();
expect(scrollable.scrollOffset, lessThan(0.0));
expect(scrollable.virtualScrollOffset, equals(scrollable.scrollOffset));
await tester.pump(const Duration(seconds: 1)); // Allow overscroll to settle
await tester.scrollAt(tester.getTopLeft(find.text('bottom')), const Offset(0.0, -400.0));
await tester.pump();
expect(scrollable.scrollOffset, greaterThan(0.0));
expect(scrollable.virtualScrollOffset, equals(scrollable.scrollOffset));
await tester.pump(const Duration(seconds: 1)); // Allow overscroll to settle
// ClampOverscrolls.edge == ScrollableEdge.both
await tester.pumpWidget(buildFrame(ScrollableEdge.both));
statefulElement = tester.element(find.byType(Scrollable));
scrollable = statefulElement.state;
await tester.scrollAt(tester.getTopLeft(find.text('top')), const Offset(0.0, 400.0));
await tester.pump();
expect(scrollable.scrollOffset, equals(0.0));
expect(scrollable.virtualScrollOffset, lessThan(0.0));
await tester.pump(const Duration(seconds: 1)); // Allow overscroll to settle
await tester.scrollAt(tester.getTopLeft(find.text('bottom')), const Offset(0.0, -400.0));
await tester.pump();
expect(scrollable.scrollOffset, equals(50.0));
expect(scrollable.virtualScrollOffset, greaterThan(50.0));
// ClampOverscrolls.edge == ScrollableEdge.leading
await tester.pumpWidget(buildFrame(ScrollableEdge.leading));
statefulElement = tester.element(find.byType(Scrollable));
scrollable = statefulElement.state;
await tester.scrollAt(tester.getTopLeft(find.text('top')), const Offset(0.0, 400.0));
await tester.pump();
expect(scrollable.scrollOffset, equals(0.0));
expect(scrollable.virtualScrollOffset, lessThan(0.0));
await tester.pump(const Duration(seconds: 1)); // Allow overscroll to settle
await tester.scrollAt(tester.getTopLeft(find.text('bottom')), const Offset(0.0, -400.0));
await tester.pump();
expect(scrollable.scrollOffset, greaterThan(0.0));
expect(scrollable.virtualScrollOffset, equals(scrollable.scrollOffset));
// ClampOverscrolls.edge == ScrollableEdge.trailing
await tester.pumpWidget(buildFrame(ScrollableEdge.trailing));
statefulElement = tester.element(find.byType(Scrollable));
scrollable = statefulElement.state;
await tester.scrollAt(tester.getTopLeft(find.text('top')), const Offset(0.0, 400.0));
await tester.pump();
expect(scrollable.scrollOffset, lessThan(0.0));
expect(scrollable.virtualScrollOffset, equals(scrollable.scrollOffset));
expect(scrollable.virtualScrollOffset, equals(scrollable.scrollOffset));
await tester.pump(const Duration(seconds: 1)); // Allow overscroll to settle
await tester.scrollAt(tester.getTopLeft(find.text('bottom')), const Offset(0.0, -400.0));
await tester.pump();
expect(scrollable.scrollOffset, equals(50.0));
expect(scrollable.virtualScrollOffset, greaterThan(50.0));
});
}
......@@ -151,4 +151,35 @@ void main() {
await tester.tapAt(new Point(800.0 - 16.0, 200.0));
expect(tapped, equals(<int>[5, 4, 4]));
});
testWidgets('Tap immediately following clamped overscroll', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/5709
GlobalKey<ScrollableState> scrollableKey = new GlobalKey<ScrollableState>();
List<int> tapped = <int>[];
await tester.pumpWidget(
new ClampOverscrolls(
edge: ScrollableEdge.both,
child: new ScrollableList(
scrollableKey: scrollableKey,
itemExtent: 200.0,
children: items.map((int item) {
return new Container(
child: new GestureDetector(
onTap: () { tapped.add(item); },
child: new Text('$item')
)
);
})
)
)
);
await tester.fling(find.text('0'), const Offset(0.0, 400.0), 1000.0);
expect(scrollableKey.currentState.scrollOffset, equals(0.0));
expect(scrollableKey.currentState.virtualScrollOffset, lessThan(0.0));
await tester.tapAt(new Point(200.0, 100.0));
expect(tapped, equals(<int>[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