From 8c8837d63ec8dda3bc89978fee1388b8e0bb005f Mon Sep 17 00:00:00 2001 From: Hixie Date: Thu, 24 Sep 2015 17:00:19 -0700 Subject: [PATCH] Port ScrollableMixedWidgetListState to fn3. --- examples/widgets/card_collection.dart | 28 ++++--- sky/packages/sky/lib/src/fn3/framework.dart | 30 +++++-- .../sky/lib/src/fn3/mixed_viewport.dart | 79 ++++++++++++----- sky/packages/sky/lib/src/fn3/scrollable.dart | 84 ++++++++++++++++++- 4 files changed, 179 insertions(+), 42 deletions(-) diff --git a/examples/widgets/card_collection.dart b/examples/widgets/card_collection.dart index 37cb977559b2a..d70d0e3eb43ee 100644 --- a/examples/widgets/card_collection.dart +++ b/examples/widgets/card_collection.dart @@ -4,7 +4,8 @@ import 'package:sky/animation.dart'; import 'package:sky/material.dart'; -import 'package:sky/widgets.dart'; +import 'package:sky/painting.dart'; +import 'package:sky/src/fn3.dart'; class CardModel { CardModel(this.value, this.height, this.color); @@ -15,7 +16,11 @@ class CardModel { Key get key => new ObjectKey(this); } -class CardCollectionApp extends App { +class CardCollectionApp extends StatefulComponent { + CardCollectionAppState createState() => new CardCollectionAppState(); +} + +class CardCollectionAppState extends State { static const TextStyle cardLabelStyle = const TextStyle(color: Colors.white, fontSize: 18.0, fontWeight: bold); @@ -23,14 +28,14 @@ class CardCollectionApp extends App { final TextStyle backgroundTextStyle = Typography.white.title.copyWith(textAlign: TextAlign.center); - MixedViewportLayoutState _layoutState = new MixedViewportLayoutState(); List _cardModels; DismissDirection _dismissDirection = DismissDirection.horizontal; bool _drawerShowing = false; AnimationStatus _drawerStatus = AnimationStatus.dismissed; + InvalidatorCallback _invalidator; - - void initState() { + void initState(BuildContext context) { + super.initState(context); List cardHeights = [ 48.0, 63.0, 82.0, 146.0, 60.0, 55.0, 84.0, 96.0, 50.0, 48.0, 63.0, 82.0, 146.0, 60.0, 55.0, 84.0, 96.0, 50.0, @@ -40,7 +45,6 @@ class CardCollectionApp extends App { Color color = Color.lerp(Colors.red[300], Colors.blue[900], i / cardHeights.length); return new CardModel(i, cardHeights[i], color); }); - super.initState(); } void dismissCard(CardModel card) { @@ -121,14 +125,14 @@ class CardCollectionApp extends App { ); } - Widget buildCard(int index) { + Widget buildCard(BuildContext context, int index) { if (index >= _cardModels.length) return null; CardModel cardModel = _cardModels[index]; Widget card = new Dismissable( direction: _dismissDirection, - onResized: () { _layoutState.invalidate([index]); }, + onResized: () { _invalidator([index]); }, onDismissed: () { dismissCard(cardModel); }, child: new Card( color: cardModel.color, @@ -178,7 +182,7 @@ class CardCollectionApp extends App { child: new Viewport( child: new Container( height: cardModel.height, - decoration: new BoxDecoration(backgroundColor: Theme.of(this).primaryColor), + decoration: new BoxDecoration(backgroundColor: Theme.of(context).primaryColor), child: new Row([ leftArrowIcon, new Flexible(child: new Text(backgroundMessage, style: backgroundTextStyle)), @@ -196,14 +200,14 @@ class CardCollectionApp extends App { ); } - Widget build() { + Widget build(BuildContext context) { Widget cardCollection = new Container( padding: const EdgeDims.symmetric(vertical: 12.0, horizontal: 8.0), - decoration: new BoxDecoration(backgroundColor: Theme.of(this).primarySwatch[50]), + decoration: new BoxDecoration(backgroundColor: Theme.of(context).primarySwatch[50]), child: new ScrollableMixedWidgetList( builder: buildCard, token: _cardModels.length, - layoutState: _layoutState + onInvalidatorAvailable: (InvalidatorCallback callback) { _invalidator = callback; } ) ); diff --git a/sky/packages/sky/lib/src/fn3/framework.dart b/sky/packages/sky/lib/src/fn3/framework.dart index d72b84e7c8f2c..caa2cbf4b8df2 100644 --- a/sky/packages/sky/lib/src/fn3/framework.dart +++ b/sky/packages/sky/lib/src/fn3/framework.dart @@ -315,9 +315,14 @@ abstract class State { /// component will not be scheduled for rebuilding, meaning that its rendering /// will not be updated. void setState(void fn()) { - assert(_debugLifecycleState == _StateLifecycle.ready); + assert(_debugLifecycleState != _StateLifecycle.defunct); fn(); - _element.markNeedsBuild(); + if (_element._builder != null) { + // _element._builder is set after initState(). We verify that we're past + // that before calling markNeedsBuild() so that setState()s triggered + // during initState() during lockState() don't cause any trouble. + _element.markNeedsBuild(); + } } /// Called when this object is removed from the tree. Override this to clean @@ -509,8 +514,10 @@ abstract class Element implements BuildContext { _parent = parent; _slot = newSlot; _depth = _parent != null ? _parent.depth + 1 : 1; - if (widget.key is GlobalKey) - widget.key._register(this); + if (widget.key is GlobalKey) { + final GlobalKey key = widget.key; + key._register(this); + } assert(() { _debugLifecycleState = _ElementLifecycle.mounted; return true; }); } @@ -576,8 +583,10 @@ abstract class Element implements BuildContext { assert(_debugLifecycleState == _ElementLifecycle.mounted); assert(widget != null); assert(depth != null); - if (widget.key is GlobalKey) - widget.key._unregister(this); + if (widget.key is GlobalKey) { + final GlobalKey key = widget.key; + key._unregister(this); + } assert(() { _debugLifecycleState = _ElementLifecycle.defunct; return true; }); } @@ -631,6 +640,7 @@ abstract class BuildableElement extends Element { /// binding when scheduleBuild() has been called to mark this element dirty, /// and by update() when the Widget has changed. void rebuild() { + assert(_debugLifecycleState != _ElementLifecycle.initial); if (!_dirty) return; assert(_debugLifecycleState == _ElementLifecycle.mounted); @@ -738,6 +748,9 @@ class StatefulComponentElement extends BuildableElement { return false; }); assert(() { _state._debugLifecycleState = _StateLifecycle.ready; return true; }); + assert(_builder == null); + // see State.setState() for why it's important that _builder be set after + // initState() is called. _builder = _state.build; } @@ -1184,9 +1197,8 @@ typedef void WidgetsExceptionHandler(String context, dynamic exception, StackTra /// the exception occurred, and may include additional details such as /// descriptions of the objects involved. The 'exception' argument contains the /// object that was thrown, and the 'stack' argument contains the stack trace. -/// The callback is invoked after the information is printed to the console, and -/// could be used to print additional information, such as from -/// [debugDumpApp()]. +/// If no callback is set, then a default behaviour consisting of dumping the +/// context, exception, and stack trace to the console is used instead. WidgetsExceptionHandler debugWidgetsExceptionHandler; void _debugReportException(String context, dynamic exception, StackTrace stack) { if (debugWidgetsExceptionHandler != null) { diff --git a/sky/packages/sky/lib/src/fn3/mixed_viewport.dart b/sky/packages/sky/lib/src/fn3/mixed_viewport.dart index b03385a50f4de..ff3444d77e90c 100644 --- a/sky/packages/sky/lib/src/fn3/mixed_viewport.dart +++ b/sky/packages/sky/lib/src/fn3/mixed_viewport.dart @@ -163,14 +163,20 @@ class MixedViewportElement extends RenderObjectElement { assert(renderObject != null); final int startIndex = _firstVisibleChildIndex; int lastIndex = startIndex + _childrenByKey.length - 1; - for (int index = startIndex; index <= lastIndex; index += 1) { + Element nextSibling = null; + for (int index = lastIndex; index > startIndex; index -= 1) { final Widget newWidget = _buildWidgetAt(index); final _ChildKey key = new _ChildKey.fromWidget(newWidget); final Element oldElement = _childrenByKey[key]; assert(oldElement != null); - final Element newElement = updateChild(oldElement, newWidget, renderObject.childAfter(oldElement.renderObject)); + final Element newElement = updateChild(oldElement, newWidget, nextSibling); assert(newElement != null); _childrenByKey[key] = newElement; + // Verify that it hasn't changed size. + // If this assertion fires, it means you didn't call "invalidate" + // before changing the size of one of your items. + assert(_debugIsSameSize(newElement, index, _lastLayoutConstraints)); + nextSibling = newElement; } } } @@ -245,6 +251,16 @@ class MixedViewportElement extends RenderObjectElement { return newElement; } + // Build the widget at index. + Element _maybeGetElement(int index, BoxConstraints innerConstraints) { + assert(index <= _childOffsets.length - 1); + final Widget newWidget = _maybeBuildWidgetAt(index); + if (newWidget == null) + return null; + final Element newElement = _inflateOrUpdateWidget(newWidget); + return newElement; + } + // Build the widget at index, handling the case where there is no such widget. // Update the offset for that widget. Element _getElementAtLastKnownOffset(int index, BoxConstraints innerConstraints) { @@ -257,14 +273,14 @@ class MixedViewportElement extends RenderObjectElement { final Element newElement = _inflateOrUpdateWidget(newWidget); // Update the offsets based on the newElement's dimensions. - final double newOffset = _getOffset(newElement, innerConstraints); + final double newOffset = _getElementExtent(newElement, innerConstraints); _childOffsets.add(_childOffsets[index] + newOffset); return newElement; } /// Returns the intrinsic size of the given element in the scroll direction - double _getOffset(Element element, BoxConstraints innerConstraints) { + double _getElementExtent(Element element, BoxConstraints innerConstraints) { final RenderBox childRenderObject = element.renderObject; switch (widget.direction) { case ScrollDirection.vertical: @@ -277,6 +293,38 @@ class MixedViewportElement extends RenderObjectElement { } } + BoxConstraints _getInnerConstraints(BoxConstraints constraints) { + switch (widget.direction) { + case ScrollDirection.vertical: + return new BoxConstraints.tightFor(width: constraints.constrainWidth()); + case ScrollDirection.horizontal: + return new BoxConstraints.tightFor(height: constraints.constrainHeight()); + case ScrollDirection.both: + assert(false); // we don't support ScrollDirection.both, see issue 888 + return null; + } + } + + /// This compares the offsets we had for an element with its current + /// intrinsic dimensions. + bool _debugIsSameSize(Element element, int index, BoxConstraints constraints) { + BoxConstraints innerConstraints = _getInnerConstraints(constraints); + // We multiple both sides by 32 and then round to avoid floating + // point errors. (You have to round, not truncate, because otherwise + // if the error is on either side of an integer, you'll magnify it + // rather than hiding it.) + // This is an issue because we don't actually record the raw data + // (the intrinsic dimensions), we record the offsets. + // The offsets therefore accumulate floating point errors. The + // errors are far too small to make the slightest diffference, but + // they're big enough to trip the assertion if we don't do this. + // We multiply by 32 so that we notice errors up to 1/32nd of a + // logical pixel. I'm assuming 32x resolution displays aren't going + // to happen. When I'm invariably proved wrong, just bump this up to + // a higher power of two. + return ((_childOffsets[index+1] - _childOffsets[index]) * 32.0).round() == (_getElementExtent(element, innerConstraints) * 32.0).round(); + } + /// This is the core lazy-build algorithm. It builds widgets incrementally /// from index 0 until it has built enough widgets to cover itself, and /// discards any widgets that are not displayed. @@ -306,16 +354,7 @@ class MixedViewportElement extends RenderObjectElement { final double endOffset = widget.startOffset + extent; // Create the constraints that we will use to measure the children. - BoxConstraints innerConstraints; - switch (widget.direction) { - case ScrollDirection.vertical: - innerConstraints = new BoxConstraints.tightFor(width: constraints.constrainWidth()); - break; - case ScrollDirection.horizontal: - innerConstraints = new BoxConstraints.tightFor(height: constraints.constrainHeight()); - break; - case ScrollDirection.both: assert(false); // we don't support ScrollDirection.both, see issue 888 - } + final BoxConstraints innerConstraints = _getInnerConstraints(constraints); // Before doing the actual layout, fix the offsets for the widgets whose // size or type has changed. @@ -323,7 +362,7 @@ class MixedViewportElement extends RenderObjectElement { assert(_childOffsets.length > 0); List invalidIndices = _invalidIndices.toList(); invalidIndices.sort(); - for (int i = 0; i < invalidIndices.length - 1; i += 1) { + for (int i = 0; i < invalidIndices.length; i += 1) { // Determine the indices for this pass. final int widgetIndex = invalidIndices[i]; @@ -346,7 +385,7 @@ class MixedViewportElement extends RenderObjectElement { final Element newElement = _getElement(widgetIndex, innerConstraints); // Update the offsets based on the newElement's dimensions. - final double newOffset = _getOffset(newElement, innerConstraints); + final double newOffset = _getElementExtent(newElement, innerConstraints); final double oldOffset = _childOffsets[widgetIndex + 1] - _childOffsets[widgetIndex]; final double offsetDelta = newOffset - oldOffset; for (int j = widgetIndex + 1; j <= endIndex; j++) @@ -465,18 +504,20 @@ class MixedViewportElement extends RenderObjectElement { // Build all the widgets we still need. while (_childOffsets[index] < endOffset) { if (!builtChildren.containsKey(index)) { - Element element = _getElement(index, innerConstraints); + Element element = _maybeGetElement(index, innerConstraints); if (element == null) { _didReachLastChild = true; break; } if (index == _childOffsets.length-1) { // Remember this element's offset. - final double newOffset = _getOffset(element, innerConstraints); + final double newOffset = _getElementExtent(element, innerConstraints); _childOffsets.add(_childOffsets[index] + newOffset); } else { // Verify that it hasn't changed size. - assert(_childOffsets[index] - _childOffsets[index-1] == _getOffset(element, innerConstraints)); + // If this assertion fires, it means you didn't call "invalidate" + // before changing the size of one of your items. + assert(_debugIsSameSize(element, index, constraints)); } // Remember the element for when we place the children. final _ChildKey key = new _ChildKey.fromWidget(element.widget); diff --git a/sky/packages/sky/lib/src/fn3/scrollable.dart b/sky/packages/sky/lib/src/fn3/scrollable.dart index ae415533ad8ff..401c11bd1174b 100644 --- a/sky/packages/sky/lib/src/fn3/scrollable.dart +++ b/sky/packages/sky/lib/src/fn3/scrollable.dart @@ -14,6 +14,7 @@ import 'package:sky/src/fn3/basic.dart'; import 'package:sky/src/fn3/framework.dart'; import 'package:sky/src/fn3/gesture_detector.dart'; import 'package:sky/src/fn3/homogeneous_viewport.dart'; +import 'package:sky/src/fn3/mixed_viewport.dart'; // The gesture velocity properties are pixels/second, config min,max limits are pixels/ms const double _kMillisecondsPerSecond = 1000.0; @@ -386,7 +387,7 @@ abstract class ScrollableWidgetListState extends } void _updateScrollBehavior() { - // if you don't call this from build() or syncConstructorArguments(), you must call it from setState(). + // if you don't call this from build(), you must call it from setState(). double contentExtent = config.itemExtent * itemCount; if (config.padding != null) contentExtent += _leadingPadding + _trailingPadding; @@ -536,4 +537,83 @@ class PageableListState extends ScrollableListState> { } } -// TODO(abarth): ScrollableMixedWidgetList +/// A general scrollable list for a large number of children that might not all +/// have the same height. Prefer [ScrollableWidgetList] when all the children +/// have the same height because it can use that property to be more efficient. +/// Prefer [ScrollableViewport] with a single child. +class ScrollableMixedWidgetList extends Scrollable { + ScrollableMixedWidgetList({ + Key key, + double initialScrollOffset, + this.builder, + this.token, + this.onInvalidatorAvailable + }) : super(key: key, initialScrollOffset: initialScrollOffset); + + final IndexedBuilder builder; + final Object token; + final InvalidatorAvailableCallback onInvalidatorAvailable; + + ScrollableMixedWidgetListState createState() => new ScrollableMixedWidgetListState(); +} + +class ScrollableMixedWidgetListState extends ScrollableState { + void initState(BuildContext context) { + super.initState(context); + scrollBehavior.updateExtents( + contentExtent: double.INFINITY + ); + } + + ScrollBehavior createScrollBehavior() => new OverscrollBehavior(); + OverscrollBehavior get scrollBehavior => super.scrollBehavior; + + void _handleSizeChanged(Size newSize) { + setState(() { + scrollBy(scrollBehavior.updateExtents( + containerExtent: newSize.height, + scrollOffset: scrollOffset + )); + }); + } + + bool _contentChanged = false; + + void didUpdateConfig(ScrollableMixedWidgetList oldConfig) { + super.didUpdateConfig(oldConfig); + if (config.token != oldConfig.token) { + // When the token changes the scrollable's contents may have changed. + // Remember as much so that after the new contents have been laid out we + // can adjust the scrollOffset so that the last page of content is still + // visible. + _contentChanged = true; + } + } + + void _handleExtentsUpdate(double newExtents) { + double newScrollOffset; + setState(() { + newScrollOffset = scrollBehavior.updateExtents( + contentExtent: newExtents ?? double.INFINITY, + scrollOffset: scrollOffset + ); + }); + if (_contentChanged) { + _contentChanged = false; + scrollTo(newScrollOffset); + } + } + + Widget buildContent(BuildContext context) { + return new SizeObserver( + callback: _handleSizeChanged, + child: new MixedViewport( + startOffset: scrollOffset, + builder: config.builder, + token: config.token, + onInvalidatorAvailable: config.onInvalidatorAvailable, + onExtentsUpdate: _handleExtentsUpdate + ) + ); + } +}