From fd7be69df09654571176a9155a6bef911e12f612 Mon Sep 17 00:00:00 2001 From: Hixie Date: Fri, 12 Feb 2016 11:12:22 -0800 Subject: [PATCH] SizeObserver crusade: snap alignment Remove the SizeObserver you need to use snap alignment in lists by having the lists provide the size themselves in the callback. Also, remove snapAlignmentOffset since we couldn't figure out how to explain it and it's not really needed. --- examples/widgets/card_collection.dart | 38 +++++++--------- .../lib/src/widgets/pageable_list.dart | 4 +- .../flutter/lib/src/widgets/scrollable.dart | 45 +++++++++++-------- .../lib/src/widgets/scrollable_grid.dart | 4 +- .../lib/src/widgets/scrollable_list.dart | 8 +--- .../test/widget/snap_scrolling_test.dart | 2 +- 6 files changed, 46 insertions(+), 55 deletions(-) diff --git a/examples/widgets/card_collection.dart b/examples/widgets/card_collection.dart index 241dee5a2a..c2683d0ba1 100644 --- a/examples/widgets/card_collection.dart +++ b/examples/widgets/card_collection.dart @@ -28,6 +28,8 @@ class CardCollectionState extends State { // TODO(hansmuller): need a local image asset static const _sunshineURL = "http://www.walltor.com/images/wallpaper/good-morning-sunshine-58540.jpg"; + static const kCardMargins = 8.0; + final TextStyle backgroundTextStyle = Typography.white.title.copyWith(textAlign: TextAlign.center); @@ -41,7 +43,6 @@ class CardCollectionState extends State { bool _sunshine = false; bool _varyFontSizes = false; InvalidatorCallback _invalidator; - Size _cardCollectionSize = new Size(200.0, 200.0); void _initVariableSizedCardModels() { List cardHeights = [ @@ -78,15 +79,14 @@ class CardCollectionState extends State { double _variableSizeToSnapOffset(double scrollOffset) { double cumulativeHeight = 0.0; - double margins = 8.0; List cumulativeHeights = _cardModels.map((CardModel card) { - cumulativeHeight += card.height + margins; + cumulativeHeight += card.height + kCardMargins; return cumulativeHeight; }) .toList(); double offsetForIndex(int i) { - return 12.0 + (margins + _cardModels[i].height) / 2.0 + ((i == 0) ? 0.0 : cumulativeHeights[i - 1]); + return (kCardMargins + _cardModels[i].height) / 2.0 + ((i == 0) ? 0.0 : cumulativeHeights[i - 1]); } for (int i = 0; i < cumulativeHeights.length; i++) { @@ -99,11 +99,14 @@ class CardCollectionState extends State { double _fixedSizeToSnapOffset(double scrollOffset) { double cardHeight = _cardModels[0].height; int cardIndex = (scrollOffset.clamp(0.0, cardHeight * (_cardModels.length - 1)) / cardHeight).floor(); - return 12.0 + cardIndex * cardHeight + cardHeight * 0.5; + return cardIndex * cardHeight + cardHeight * 0.5; } - double _toSnapOffset(double scrollOffset) { - return _fixedSizeCards ? _fixedSizeToSnapOffset(scrollOffset) : _variableSizeToSnapOffset(scrollOffset); + double _toSnapOffset(double scrollOffset, Size containerSize) { + double halfHeight = containerSize.height / 2.0; + scrollOffset += halfHeight; + double result = _fixedSizeCards ? _fixedSizeToSnapOffset(scrollOffset) : _variableSizeToSnapOffset(scrollOffset); + return result - halfHeight; } void dismissCard(CardModel card) { @@ -300,7 +303,7 @@ class CardCollectionState extends State { color: Theme.of(context).primarySwatch[cardModel.color], child: new Container( height: cardModel.height, - padding: const EdgeDims.all(8.0), + padding: const EdgeDims.all(kCardMargins), child: _editable ? new Center( child: new Input( @@ -387,12 +390,6 @@ class CardCollectionState extends State { ); } - void _updateCardCollectionSize(Size newSize) { - setState(() { - _cardCollectionSize = newSize; - }); - } - Shader _createShader(Rect bounds) { return new LinearGradient( begin: bounds.topLeft, @@ -408,7 +405,6 @@ class CardCollectionState extends State { if (_fixedSizeCards) { cardCollection = new ScrollableList ( snapOffsetCallback: _snapToCenter ? _toSnapOffset : null, - snapAlignmentOffset: _cardCollectionSize.height / 2.0, itemExtent: _cardModels[0].height, children: _cardModels.map((CardModel card) => _buildCard(context, card.value)) ); @@ -417,7 +413,6 @@ class CardCollectionState extends State { builder: _buildCard, token: _cardModels.length, snapOffsetCallback: _snapToCenter ? _toSnapOffset : null, - snapAlignmentOffset: _cardCollectionSize.height / 2.0, onInvalidatorAvailable: (InvalidatorCallback callback) { _invalidator = callback; } ); } @@ -431,13 +426,10 @@ class CardCollectionState extends State { ); } - Widget body = new SizeObserver( - onSizeChanged: _updateCardCollectionSize, - child: new Container( - padding: const EdgeDims.symmetric(vertical: 12.0, horizontal: 8.0), - decoration: new BoxDecoration(backgroundColor: Theme.of(context).primarySwatch[50]), - child: cardCollection - ) + Widget body = new Container( + padding: const EdgeDims.symmetric(vertical: 12.0, horizontal: 8.0), + decoration: new BoxDecoration(backgroundColor: Theme.of(context).primarySwatch[50]), + child: cardCollection ); if (_snapToCenter) { diff --git a/packages/flutter/lib/src/widgets/pageable_list.dart b/packages/flutter/lib/src/widgets/pageable_list.dart index 238680420f..68722256b2 100644 --- a/packages/flutter/lib/src/widgets/pageable_list.dart +++ b/packages/flutter/lib/src/widgets/pageable_list.dart @@ -28,7 +28,6 @@ class PageableList extends Scrollable { ScrollListener onScroll, ScrollListener onScrollEnd, SnapOffsetCallback snapOffsetCallback, - double snapAlignmentOffset: 0.0, this.itemsWrap: false, this.itemsSnapAlignment: ItemsSnapAlignment.adjacentItem, this.onPageChanged, @@ -44,8 +43,7 @@ class PageableList extends Scrollable { onScrollStart: onScrollStart, onScroll: onScroll, onScrollEnd: onScrollEnd, - snapOffsetCallback: snapOffsetCallback, - snapAlignmentOffset: snapAlignmentOffset + snapOffsetCallback: snapOffsetCallback ); final bool itemsWrap; diff --git a/packages/flutter/lib/src/widgets/scrollable.dart b/packages/flutter/lib/src/widgets/scrollable.dart index 6a90b01ffd..c26391f8d4 100644 --- a/packages/flutter/lib/src/widgets/scrollable.dart +++ b/packages/flutter/lib/src/widgets/scrollable.dart @@ -25,7 +25,7 @@ final Tolerance kPixelScrollTolerance = new Tolerance( ); typedef void ScrollListener(double scrollOffset); -typedef double SnapOffsetCallback(double scrollOffset); +typedef double SnapOffsetCallback(double scrollOffset, Size containerSize); /// A base class for scrollable widgets. /// @@ -43,12 +43,10 @@ abstract class Scrollable extends StatefulComponent { this.onScrollStart, this.onScroll, this.onScrollEnd, - this.snapOffsetCallback, - this.snapAlignmentOffset: 0.0 + this.snapOffsetCallback }) : super(key: key) { assert(scrollDirection == Axis.vertical || scrollDirection == Axis.horizontal); assert(scrollAnchor == ViewportAnchor.start || scrollAnchor == ViewportAnchor.end); - assert(snapAlignmentOffset != null); } /// The scroll offset this widget should use when first created. @@ -68,11 +66,21 @@ abstract class Scrollable extends StatefulComponent { /// Called whenever this widget stops scrolling. final ScrollListener onScrollEnd; - /// Called to determine the offset to which scrolling should snap. + /// Called to determine the offset to which scrolling should snap, + /// when handling a fling. + /// + /// This callback, if set, will be called with the offset that the + /// Scrollable would have scrolled to in the absence of this + /// callback, and a Size describing the size of the Scrollable + /// itself. + /// + /// The callback's return value is used as the new scroll offset to + /// aim for. + /// + /// If the callback simply returns its first argument (the offset), + /// then it is as if the callback was null. final SnapOffsetCallback snapOffsetCallback; - final double snapAlignmentOffset; // What does this do? - /// The state from the closest instance of this class that encloses the given context. static ScrollableState of(BuildContext context) { return context.ancestorStateOfType(const TypeMatcher()); @@ -294,7 +302,8 @@ abstract class ScrollableState extends State { /// Returns the snapped offset closest to the given scroll offset. double snapScrollOffset(double scrollOffset) { - return config.snapOffsetCallback == null ? scrollOffset : config.snapOffsetCallback(scrollOffset); + RenderBox box = context.findRenderObject(); + return config.snapOffsetCallback == null ? scrollOffset : config.snapOffsetCallback(scrollOffset, box.size); } /// Whether this scrollable should attempt to snap scroll offsets. @@ -312,20 +321,20 @@ abstract class ScrollableState extends State { if (endScrollOffset.isNaN) return null; - final double snappedScrollOffset = snapScrollOffset(endScrollOffset + config.snapAlignmentOffset); - final double alignedScrollOffset = snappedScrollOffset - config.snapAlignmentOffset; - if (!_scrollOffsetIsInBounds(alignedScrollOffset)) + final double snappedScrollOffset = snapScrollOffset(endScrollOffset); + if (!_scrollOffsetIsInBounds(snappedScrollOffset)) return null; - final double snapVelocity = scrollVelocity.abs() * (alignedScrollOffset - scrollOffset).sign; + final double snapVelocity = scrollVelocity.abs() * (snappedScrollOffset - scrollOffset).sign; final double endVelocity = pixelOffsetToScrollOffset(kPixelScrollTolerance.velocity).abs() * scrollVelocity.sign; - Simulation toSnapSimulation = - scrollBehavior.createSnapScrollSimulation(scrollOffset, alignedScrollOffset, snapVelocity, endVelocity); + Simulation toSnapSimulation = scrollBehavior.createSnapScrollSimulation( + scrollOffset, snappedScrollOffset, snapVelocity, endVelocity + ); if (toSnapSimulation == null) return null; - final double scrollOffsetMin = math.min(scrollOffset, alignedScrollOffset); - final double scrollOffsetMax = math.max(scrollOffset, alignedScrollOffset); + final double scrollOffsetMin = math.min(scrollOffset, snappedScrollOffset); + final double scrollOffsetMax = math.max(scrollOffset, snappedScrollOffset); return new ClampedSimulation(toSnapSimulation, xMin: scrollOffsetMin, xMax: scrollOffsetMax); } @@ -631,7 +640,6 @@ class ScrollableMixedWidgetList extends Scrollable { double initialScrollOffset, ScrollListener onScroll, SnapOffsetCallback snapOffsetCallback, - double snapAlignmentOffset: 0.0, this.builder, this.token, this.onInvalidatorAvailable @@ -639,8 +647,7 @@ class ScrollableMixedWidgetList extends Scrollable { key: key, initialScrollOffset: initialScrollOffset, onScroll: onScroll, - snapOffsetCallback: snapOffsetCallback, - snapAlignmentOffset: snapAlignmentOffset + snapOffsetCallback: snapOffsetCallback ); final IndexedBuilder builder; diff --git a/packages/flutter/lib/src/widgets/scrollable_grid.dart b/packages/flutter/lib/src/widgets/scrollable_grid.dart index efe8ab6116..a8793129ab 100644 --- a/packages/flutter/lib/src/widgets/scrollable_grid.dart +++ b/packages/flutter/lib/src/widgets/scrollable_grid.dart @@ -20,7 +20,6 @@ class ScrollableGrid extends Scrollable { double initialScrollOffset, ScrollListener onScroll, SnapOffsetCallback snapOffsetCallback, - double snapAlignmentOffset: 0.0, this.delegate, this.children }) : super( @@ -31,8 +30,7 @@ class ScrollableGrid extends Scrollable { // delegate that places children in column-major order. scrollDirection: Axis.vertical, onScroll: onScroll, - snapOffsetCallback: snapOffsetCallback, - snapAlignmentOffset: snapAlignmentOffset + snapOffsetCallback: snapOffsetCallback ); final GridDelegate delegate; diff --git a/packages/flutter/lib/src/widgets/scrollable_list.dart b/packages/flutter/lib/src/widgets/scrollable_list.dart index 363a03a990..04c26394cd 100644 --- a/packages/flutter/lib/src/widgets/scrollable_list.dart +++ b/packages/flutter/lib/src/widgets/scrollable_list.dart @@ -19,7 +19,6 @@ class ScrollableList extends Scrollable { ViewportAnchor scrollAnchor: ViewportAnchor.start, ScrollListener onScroll, SnapOffsetCallback snapOffsetCallback, - double snapAlignmentOffset: 0.0, this.itemExtent, this.itemsWrap: false, this.padding, @@ -31,8 +30,7 @@ class ScrollableList extends Scrollable { scrollDirection: scrollDirection, scrollAnchor: scrollAnchor, onScroll: onScroll, - snapOffsetCallback: snapOffsetCallback, - snapAlignmentOffset: snapAlignmentOffset + snapOffsetCallback: snapOffsetCallback ) { assert(itemExtent != null); } @@ -269,7 +267,6 @@ class ScrollableLazyList extends Scrollable { ViewportAnchor scrollAnchor: ViewportAnchor.start, ScrollListener onScroll, SnapOffsetCallback snapOffsetCallback, - double snapAlignmentOffset: 0.0, this.itemExtent, this.itemCount, this.itemBuilder, @@ -281,8 +278,7 @@ class ScrollableLazyList extends Scrollable { scrollDirection: scrollDirection, scrollAnchor: scrollAnchor, onScroll: onScroll, - snapOffsetCallback: snapOffsetCallback, - snapAlignmentOffset: snapAlignmentOffset + snapOffsetCallback: snapOffsetCallback ) { assert(itemExtent != null); assert(itemBuilder != null); diff --git a/packages/flutter/test/widget/snap_scrolling_test.dart b/packages/flutter/test/widget/snap_scrolling_test.dart index e2e895f998..413c04702b 100644 --- a/packages/flutter/test/widget/snap_scrolling_test.dart +++ b/packages/flutter/test/widget/snap_scrolling_test.dart @@ -20,7 +20,7 @@ Widget buildItem(int item) { ); } -double snapOffsetCallback(double offset) { +double snapOffsetCallback(double offset, Size size) { return (offset / itemExtent).floor() * itemExtent; }