Call onPageChanged at the halfway mark (#8302)

Previously we called onPageChanged when the scroll ended, but that is too late.
Now we call onPageChanged when we cross the halfway mark, which, for example,
makes the tab indicator update earlier.

Fixes #8265

packages/flutter/lib/src/material/tabs.dart

@@ -639,8 +639,6 @@ class _TabBarViewState extends State<TabBarView> {
   TabController _controller;
   PageController _pageController;
   List<Widget> _children;
-  double _offsetAnchor;
-  double _offsetBias = 0.0;
   int _currentIndex;
   int _warpUnderwayCount = 0;
 
@@ -742,30 +740,11 @@ class _TabBarViewState extends State<TabBarView> {
     if (notification.depth != 0)
       return false;
 
-    if (notification is ScrollStartNotification) {
-      _offsetAnchor = null;
-    } else if (notification is ScrollUpdateNotification) {
-      if (!_controller.indexIsChanging) {
-        _offsetAnchor ??= _pageController.page;
-        _controller.offset = (_offsetBias + _pageController.page - _offsetAnchor).clamp(-1.0, 1.0);
-      }
-    } else if (notification is ScrollEndNotification) {
-      // Either the the animation that follows a fling has completed and we've landed
-      // on a new tab view, or a new pointer gesture has interrupted the fling
-      // animation before it has completed.
-      final double integralScrollOffset = _pageController.page.floorToDouble();
-      if (integralScrollOffset == _pageController.page) {
-        _offsetBias = 0.0;
-        // The animation duration is short since the tab indicator and this
-        // page view have already moved.
-        _controller.animateTo(
-          integralScrollOffset.floor(),
-          duration: const Duration(milliseconds: 30)
-        );
-      } else {
-        // The fling scroll animation was interrupted.
-        _offsetBias = _controller.offset;
-      }
+    if (notification is ScrollUpdateNotification && !_controller.indexIsChanging) {
+      _currentIndex = _pageController.page.round();
+      if (_currentIndex != _controller.index)
+        _controller.index = _currentIndex;
+      _controller.offset = (_pageController.page - _controller.index).clamp(-1.0, 1.0);
     }
 
     return false;

packages/flutter/lib/src/widgets/page_view.dart

@@ -4,6 +4,7 @@
 
 import 'dart:async';
 
+import 'package:flutter/rendering.dart';
 import 'package:meta/meta.dart';
 
 import 'basic.dart';
@@ -14,7 +15,9 @@ import 'scroll_notification.dart';
 import 'scroll_physics.dart';
 import 'scroll_position.dart';
 import 'scroll_view.dart';
+import 'scrollable.dart';
 import 'sliver.dart';
+import 'viewport.dart';
 
 class PageController extends ScrollController {
   PageController({
@@ -110,91 +113,119 @@ final PageController _defaultPageController = new PageController();
 /// * [SingleChildScrollView], when you need to make a single child scrollable.
 /// * [ListView], for a scrollable list of boxes.
 /// * [GridView], for a scrollable grid of boxes.
-class PageView extends BoxScrollView {
+class PageView extends StatefulWidget {
   PageView({
     Key key,
-    Axis scrollDirection: Axis.horizontal,
-    bool reverse: false,
+    this.scrollDirection: Axis.horizontal,
+    this.reverse: false,
     PageController controller,
-    ScrollPhysics physics: const PageScrollPhysics(),
-    bool shrinkWrap: false,
-    EdgeInsets padding,
+    this.physics: const PageScrollPhysics(),
     this.onPageChanged,
     List<Widget> children: const <Widget>[],
-  }) : childrenDelegate = new SliverChildListDelegate(children), super(
-    key: key,
-    scrollDirection: scrollDirection,
-    reverse: reverse,
-    controller: controller ?? _defaultPageController,
-    physics: physics,
-    shrinkWrap: shrinkWrap,
-    padding: padding,
-  );
+  }) : controller = controller ?? _defaultPageController,
+       childrenDelegate = new SliverChildListDelegate(children),
+       super(key: key);
 
   PageView.builder({
     Key key,
-    Axis scrollDirection: Axis.horizontal,
-    bool reverse: false,
+    this.scrollDirection: Axis.horizontal,
+    this.reverse: false,
     PageController controller,
-    ScrollPhysics physics: const PageScrollPhysics(),
-    bool shrinkWrap: false,
-    EdgeInsets padding,
+    this.physics: const PageScrollPhysics(),
     this.onPageChanged,
     IndexedWidgetBuilder itemBuilder,
     int itemCount,
-  }) : childrenDelegate = new SliverChildBuilderDelegate(itemBuilder, childCount: itemCount), super(
-    key: key,
-    scrollDirection: scrollDirection,
-    reverse: reverse,
-    controller: controller ?? _defaultPageController,
-    physics: physics,
-    shrinkWrap: shrinkWrap,
-    padding: padding,
-  );
+  }) : controller = controller ?? _defaultPageController,
+       childrenDelegate = new SliverChildBuilderDelegate(itemBuilder, childCount: itemCount),
+       super(key: key);
 
   PageView.custom({
     Key key,
-    Axis scrollDirection: Axis.horizontal,
-    bool reverse: false,
+    this.scrollDirection: Axis.horizontal,
+    this.reverse: false,
     PageController controller,
-    ScrollPhysics physics: const PageScrollPhysics(),
-    bool shrinkWrap: false,
-    EdgeInsets padding,
+    this.physics: const PageScrollPhysics(),
     this.onPageChanged,
     @required this.childrenDelegate,
-  }) : super(
-    key: key,
-    scrollDirection: scrollDirection,
-    reverse: reverse,
-    controller: controller ?? _defaultPageController,
-    physics: physics,
-    shrinkWrap: shrinkWrap,
-    padding: padding,
-  ) {
+  }) : controller = controller ?? _defaultPageController, super(key: key) {
     assert(childrenDelegate != null);
   }
 
+  final Axis scrollDirection;
+
+  final bool reverse;
+
+  final PageController controller;
+
+  final ScrollPhysics physics;
+
   final ValueChanged<int> onPageChanged;
 
   final SliverChildDelegate childrenDelegate;
 
   @override
-  Widget buildChildLayout(BuildContext context) {
-    return new SliverFill(delegate: childrenDelegate);
+  _PageViewState createState() => new _PageViewState();
+}
+
+class _PageViewState extends State<PageView> {
+  int _lastReportedPage = 0;
+
+  @override
+  void initState() {
+    super.initState();
+    _lastReportedPage = config.controller.initialPage;
+  }
+
+  AxisDirection _getDirection(BuildContext context) {
+    // TODO(abarth): Consider reading direction.
+    switch (config.scrollDirection) {
+      case Axis.horizontal:
+        return config.reverse ? AxisDirection.left : AxisDirection.right;
+      case Axis.vertical:
+        return config.reverse ? AxisDirection.up : AxisDirection.down;
+    }
+    return null;
   }
 
   @override
   Widget build(BuildContext context) {
-    final Widget scrollable = super.build(context);
+    AxisDirection axisDirection = _getDirection(context);
     return new NotificationListener<ScrollNotification>(
       onNotification: (ScrollNotification notification) {
-        if (notification.depth == 0 && onPageChanged != null && notification is ScrollEndNotification) {
+        if (notification.depth == 0 && config.onPageChanged != null && notification is ScrollUpdateNotification) {
           final ScrollMetrics metrics = notification.metrics;
-          onPageChanged(metrics.extentBefore ~/ metrics.viewportDimension);
+          final int currentPage = (metrics.extentBefore / metrics.viewportDimension).round();
+          if (currentPage != _lastReportedPage) {
+            _lastReportedPage = currentPage;
+            config.onPageChanged(currentPage);
+          }
         }
         return false;
       },
-      child: scrollable,
+      child: new Scrollable(
+        axisDirection: axisDirection,
+        controller: config.controller,
+        physics: config.physics,
+        viewportBuilder: (BuildContext context, ViewportOffset offset) {
+          return new Viewport(
+            axisDirection: axisDirection,
+            offset: offset,
+            slivers: <Widget>[
+              new SliverFill(delegate: config.childrenDelegate),
+            ],
+          );
+        },
+      ),
     );
   }
+
+  @override
+  void debugFillDescription(List<String> description) {
+    super.debugFillDescription(description);
+    description.add('${config.scrollDirection}');
+    if (config.reverse)
+      description.add('reversed');
+    description.add('${config.controller}');
+    description.add('${config.physics}');
+  }
 }

packages/flutter/test/material/tabs_test.dart

@@ -618,4 +618,46 @@ void main() {
     expect(secondColor, equals(Colors.blue[500]));
   });
 
+  testWidgets('TabBar unselectedLabelColor control test', (WidgetTester tester) async {
+    TabController controller = new TabController(
+      vsync: const TestVSync(),
+      length: 2,
+    );
+
+    await tester.pumpWidget(
+      new Material(
+        child: new TabBarView(
+          controller: controller,
+          children: <Widget>[ new Text('First'), new Text('Second') ],
+        ),
+      ),
+    );
+
+    expect(controller.index, equals(0));
+
+    TestGesture gesture = await tester.startGesture(const Point(100.0, 100.0));
+
+    expect(controller.index, equals(0));
+
+    await gesture.moveBy(const Offset(-380.0, 0.0));
+
+    expect(controller.index, equals(0));
+
+    await gesture.moveBy(const Offset(-40.0, 0.0));
+
+    expect(controller.index, equals(1));
+
+    await gesture.moveBy(const Offset(-40.0, 0.0));
+    await tester.pump();
+
+    expect(controller.index, equals(1));
+
+    await gesture.up();
+    await tester.pumpUntilNoTransientCallbacks();
+    expect(controller.index, equals(1));
+
+    expect(find.text('First'), findsNothing);
+    expect(find.text('Second'), findsOneWidget);
+  });
+
 }

packages/flutter/test/widgets/page_view_test.dart

@@ -217,4 +217,51 @@ void main() {
 
     expect(find.text('Alabama'), findsOneWidget);
   });
+
+  testWidgets('Page changes at halfway point', (WidgetTester tester) async {
+    final List<int> log = <int>[];
+    await tester.pumpWidget(new PageView(
+      onPageChanged: (int page) { log.add(page); },
+      children: kStates.map<Widget>((String state) => new Text(state)).toList(),
+    ));
+
+    expect(log, isEmpty);
+
+    TestGesture gesture = await tester.startGesture(const Point(100.0, 100.0));
+    // The page view is 800.0 wide, so this move is just short of halfway.
+    await gesture.moveBy(const Offset(-380.0, 0.0));
+
+    expect(log, isEmpty);
+
+    // We've crossed the halfway mark.
+    await gesture.moveBy(const Offset(-40.0, 0.0));
+
+    expect(log, equals(const <int>[1]));
+    log.clear();
+
+    // Moving a bit more should not generate redundant notifications.
+    await gesture.moveBy(const Offset(-40.0, 0.0));
+
+    expect(log, isEmpty);
+
+    await gesture.moveBy(const Offset(-40.0, 0.0));
+    await tester.pump();
+
+    await gesture.moveBy(const Offset(-40.0, 0.0));
+    await tester.pump();
+
+    await gesture.moveBy(const Offset(-40.0, 0.0));
+    await tester.pump();
+
+    expect(log, isEmpty);
+
+    await gesture.up();
+    await tester.pumpUntilNoTransientCallbacks();
+
+    expect(log, isEmpty);
+
+    expect(find.text('Alabama'), findsNothing);
+    expect(find.text('Alaska'), findsOneWidget);
+  });
+
 }