From 6298a1aeb1f044ea5ced0ee96bcffab17bddccf8 Mon Sep 17 00:00:00 2001 From: Matt Perry Date: Thu, 30 Jun 2016 15:43:47 -0400 Subject: [PATCH] Fix a bug where ScrollableGrid/ScrollableList would flicker when (#4794) navigating away. Details are in the bug, but when we navigate away, the overscroll indicator around the Scrollable would change the widget hierarchy. This would trigger the Scrollable to rebuild, which would cause its scrollOffset to be clamped to 0 for a frame. BUG=https://github.com/flutter/flutter/issues/4597 --- .../lib/src/material/overscroll_indicator.dart | 6 +++--- packages/flutter/lib/src/rendering/proxy_box.dart | 4 ++-- .../flutter/lib/src/widgets/clamp_overscrolls.dart | 12 ++++++++---- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/packages/flutter/lib/src/material/overscroll_indicator.dart b/packages/flutter/lib/src/material/overscroll_indicator.dart index 08a0b2e920..74b8d3f9f9 100644 --- a/packages/flutter/lib/src/material/overscroll_indicator.dart +++ b/packages/flutter/lib/src/material/overscroll_indicator.dart @@ -226,10 +226,10 @@ class _OverscrollIndicatorState extends State { child: new AnimatedBuilder( animation: _extentAnimation, builder: (BuildContext context, Widget child) { - if (_scrollDirection == null) // Haven't seen a scroll yet. - return child; + // We keep the same widget hierarchy here, even when we're not + // painting anything, to avoid rebuilding the children. return new CustomPaint( - foregroundPainter: new _Painter( + foregroundPainter: _scrollDirection == null ? null : new _Painter( scrollDirection: _scrollDirection, extent: _extentAnimation.value, isLeading: _scrollOffset < _minScrollOffset, diff --git a/packages/flutter/lib/src/rendering/proxy_box.dart b/packages/flutter/lib/src/rendering/proxy_box.dart index 2986850827..e7603eecb4 100644 --- a/packages/flutter/lib/src/rendering/proxy_box.dart +++ b/packages/flutter/lib/src/rendering/proxy_box.dart @@ -1610,8 +1610,8 @@ class RenderCustomPaint extends RenderProxyBox { markNeedsPaint(); } if (attached) { - oldPainter._repaint?.removeListener(markNeedsPaint); - newPainter._repaint?.addListener(markNeedsPaint); + oldPainter?._repaint?.removeListener(markNeedsPaint); + newPainter?._repaint?.addListener(markNeedsPaint); } } diff --git a/packages/flutter/lib/src/widgets/clamp_overscrolls.dart b/packages/flutter/lib/src/widgets/clamp_overscrolls.dart index bab99998dc..20eb7634c7 100644 --- a/packages/flutter/lib/src/widgets/clamp_overscrolls.dart +++ b/packages/flutter/lib/src/widgets/clamp_overscrolls.dart @@ -50,14 +50,18 @@ class ClampOverscrolls extends InheritedWidget { /// /// 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 bool clampOverscrolls = ClampOverscrolls.of(context); final double clampedScrollOffset = clampOverscrolls ? state.scrollOffset.clamp(state.scrollBehavior.minScrollOffset, state.scrollBehavior.maxScrollOffset) : state.scrollOffset; - Widget viewport = builder(context, state, clampedScrollOffset); - if (clampOverscrolls) - viewport = new ClampOverscrolls(value: false, child: viewport); - return viewport; + Widget viewport = builder(context, state, clampedScrollOffset); + if (clampOverscrolls) + viewport = new ClampOverscrolls(value: false, child: viewport); + return viewport; } @override