diff --git a/sdk/example/stocks/lib/stock_menu.dart b/sdk/example/stocks/lib/stock_menu.dart index 4fb0e0d8808..9fc5721f5d5 100644 --- a/sdk/example/stocks/lib/stock_menu.dart +++ b/sdk/example/stocks/lib/stock_menu.dart @@ -36,8 +36,8 @@ class StockMenu extends Component { ], level: 4 ), - right: 8.0, - top: 8.0 + kStatusBarHeight + right: 0.0, + top: 0.0 + kStatusBarHeight ); } } diff --git a/sdk/lib/rendering/block.dart b/sdk/lib/rendering/block.dart index a143acda0c9..939e62ceaba 100644 --- a/sdk/lib/rendering/block.dart +++ b/sdk/lib/rendering/block.dart @@ -77,8 +77,8 @@ class RenderBlock extends RenderBox with ContainerRenderObjectMixin _cachedBaselines; + bool _ancestorUsesBaseline = false; + static bool _debugDoingBaseline = false; + static bool _debugSetDoingBaseline(bool value) { + _debugDoingBaseline = value; + return true; + } + // getDistanceToBaseline() returns the distance from the // y-coordinate of the position of the box to the y-coordinate of // the first given baseline in the box's contents. This is used by // certain layout models to align adjacent boxes on a common // baseline, regardless of padding, font size differences, etc. If - // there is no baseline, then it should return the distance from the + // there is no baseline, then it returns the distance from the // y-coordinate of the position of the box to the y-coordinate of - // the bottom of the box, i.e., the height of the box. - // Only call this after layout has been performed. + // the bottom of the box, i.e., the height of the box. Only call + // this after layout has been performed. You are only allowed to + // call this from the parent of this node, and only during + // that parent's performLayout(). double getDistanceToBaseline(TextBaseline baseline) { assert(!needsLayout); + assert(!_debugDoingBaseline); + final parent = this.parent; // TODO(ianh): Remove this once the analyzer is cleverer + assert(parent is RenderObject); + assert(parent == RenderObject.debugActiveLayout); + assert(parent.debugDoingThisLayout); + assert(_debugSetDoingBaseline(true)); double result = getDistanceToActualBaseline(baseline); + assert(_debugSetDoingBaseline(false)); + assert(parent == this.parent); // TODO(ianh): Remove this once the analyzer is cleverer if (result == null) return size.height; return result; } - // getDistanceToActualBaseline() should return the distance from the - // y-coordinate of the position of the box to the y-coordinate of - // the first given baseline in the box's contents, if any, or null - // otherwise. + // getDistanceToActualBaseline() must only be called from + // getDistanceToBaseline() and computeDistanceToActualBaseline(). Do + // not call it directly from outside those two methods. It just + // calls computeDistanceToActualBaseline() and caches the result. double getDistanceToActualBaseline(TextBaseline baseline) { - assert(!needsLayout); + assert(_debugDoingBaseline); + _ancestorUsesBaseline = true; + if (_cachedBaselines == null) + _cachedBaselines = new Map(); + _cachedBaselines.putIfAbsent(baseline, () => computeDistanceToActualBaseline(baseline)); + return _cachedBaselines[baseline]; + } + // computeDistanceToActualBaseline() should return the distance from + // the y-coordinate of the position of the box to the y-coordinate + // of the first given baseline in the box's contents, if any, or + // null otherwise. This is the method that you should override in + // subclasses. This method (computeDistanceToActualBaseline()) + // should not be called directly. Use getDistanceToBaseline() if you + // need to know the baseline of a child from performLayout(). If you + // need the baseline during paint, cache it during performLayout(). + // Use getDistanceToActualBaseline() if you are implementing + // computeDistanceToActualBaseline() and need to defer to a child. + double computeDistanceToActualBaseline(TextBaseline baseline) { + assert(_debugDoingBaseline); return null; } @@ -326,6 +361,26 @@ abstract class RenderBox extends RenderObject { print("${this.runtimeType} does not meet its constraints. Constraints: $constraints, size: $_size"); return result; } + + void markNeedsLayout() { + if (_cachedBaselines != null && _cachedBaselines.isNotEmpty) { + // if we have cached data, then someone must have used our data + assert(_ancestorUsesBaseline); + final parent = this.parent; // TODO(ianh): Remove this once the analyzer is cleverer + assert(parent is RenderObject); + parent.markNeedsLayout(); + assert(parent == this.parent); // TODO(ianh): Remove this once the analyzer is cleverer + // Now that they're dirty, we can forget that they used the + // baseline. If they use it again, then we'll set the bit + // again, and if we get dirty again, we'll notify them again. + _ancestorUsesBaseline = false; + _cachedBaselines.clear(); + } else { + // if we've never cached any data, then nobody can have used it + assert(!_ancestorUsesBaseline); + } + super.markNeedsLayout(); + } void performResize() { // default behaviour for subclasses that have sizedByParent = true size = constraints.constrain(Size.zero); @@ -410,10 +465,10 @@ class RenderProxyBox extends RenderBox with RenderObjectWithChildMixin '${super.debugDescribeSettings(prefix)}${prefix}stepWidth: ${stepWidth}\n${prefix}stepHeight: ${stepHeight}\n'; + } class RenderOpacity extends RenderProxyBox { @@ -719,12 +788,31 @@ abstract class RenderShiftedBox extends RenderBox with RenderObjectWithChildMixi this.child = child; } - void paint(PaintingCanvas canvas, Offset offset) { + double getMinIntrinsicWidth(BoxConstraints constraints) { if (child != null) - canvas.paintChild(child, child.parentData.position + offset); + return child.getMinIntrinsicWidth(constraints); + return super.getMinIntrinsicWidth(constraints); } - double getDistanceToActualBaseline(TextBaseline baseline) { + double getMaxIntrinsicWidth(BoxConstraints constraints) { + if (child != null) + return child.getMaxIntrinsicWidth(constraints); + return super.getMaxIntrinsicWidth(constraints); + } + + double getMinIntrinsicHeight(BoxConstraints constraints) { + if (child != null) + return child.getMinIntrinsicHeight(constraints); + return super.getMinIntrinsicHeight(constraints); + } + + double getMaxIntrinsicHeight(BoxConstraints constraints) { + if (child != null) + return child.getMaxIntrinsicHeight(constraints); + return super.getMaxIntrinsicHeight(constraints); + } + + double computeDistanceToActualBaseline(TextBaseline baseline) { double result; if (child != null) { assert(!needsLayout); @@ -733,11 +821,16 @@ abstract class RenderShiftedBox extends RenderBox with RenderObjectWithChildMixi if (result != null) result += child.parentData.position.y; } else { - result = super.getDistanceToActualBaseline(baseline); + result = super.computeDistanceToActualBaseline(baseline); } return result; } + void paint(PaintingCanvas canvas, Offset offset) { + if (child != null) + canvas.paintChild(child, child.parentData.position + offset); + } + void hitTestChildren(HitTestResult result, { Point position }) { if (child != null) { assert(child.parentData is BoxParentData); @@ -820,6 +913,12 @@ class RenderPadding extends RenderShiftedBox { class RenderPositionedBox extends RenderShiftedBox { + // This box aligns a child box within itself. It's only useful for + // children that don't always size to fit their parent. For example, + // to align a box at the bottom right, you would pass this box a + // tight constraint that is bigger than the child's natural size, + // with horizontal and vertical set to 1.0. + RenderPositionedBox({ RenderBox child, double horizontal: 0.5, @@ -851,30 +950,6 @@ class RenderPositionedBox extends RenderShiftedBox { markNeedsLayout(); } - double getMinIntrinsicWidth(BoxConstraints constraints) { - if (child != null) - return child.getMinIntrinsicWidth(constraints); - return super.getMinIntrinsicWidth(constraints); - } - - double getMaxIntrinsicWidth(BoxConstraints constraints) { - if (child != null) - return child.getMaxIntrinsicWidth(constraints); - return super.getMaxIntrinsicWidth(constraints); - } - - double getMinIntrinsicHeight(BoxConstraints constraints) { - if (child != null) - return child.getMinIntrinsicHeight(constraints); - return super.getMinIntrinsicHeight(constraints); - } - - double getMaxIntrinsicHeight(BoxConstraints constraints) { - if (child != null) - return child.getMaxIntrinsicHeight(constraints); - return super.getMaxIntrinsicHeight(constraints); - } - void performLayout() { if (child != null) { child.layout(constraints.loosen(), parentUsesSize: true); @@ -890,6 +965,54 @@ class RenderPositionedBox extends RenderShiftedBox { String debugDescribeSettings(String prefix) => '${super.debugDescribeSettings(prefix)}${prefix}horizontal: ${horizontal}\n${prefix}vertical: ${vertical}\n'; } +class RenderBaseline extends RenderShiftedBox { + + RenderBaseline({ + RenderBox child, + double baseline, + TextBaseline baselineType + }) : _baseline = baseline, + _baselineType = baselineType, + super(child) { + assert(baseline != null); + assert(baselineType != null); + } + + double _baseline; + double get baseline => _baseline; + void set baseline (double value) { + assert(value != null); + if (_baseline == value) + return; + _baseline = value; + markNeedsLayout(); + } + + TextBaseline _baselineType; + TextBaseline get baselineType => _baselineType; + void set baselineType (TextBaseline value) { + assert(value != null); + if (_baselineType == value) + return; + _baselineType = value; + markNeedsLayout(); + } + + void performLayout() { + if (child != null) { + child.layout(constraints.loosen(), parentUsesSize: true); + size = constraints.constrain(child.size); + assert(child.parentData is BoxParentData); + double delta = baseline - child.getDistanceToBaseline(baselineType); + child.parentData.position = new Point(0.0, delta); + } else { + performResize(); + } + } + + String debugDescribeSettings(String prefix) => '${super.debugDescribeSettings(prefix)}${prefix}baseline: ${baseline}\nbaselineType: ${baselineType}'; +} + class RenderImage extends RenderBox { RenderImage(String url, Size dimensions) { @@ -1255,7 +1378,7 @@ abstract class RenderBoxContainerDefaultsMixin c.getMaxIntrinsicHeight(innerConstraints)); } - double getDistanceToActualBaseline(TextBaseline baseline) { - assert(!needsLayout); + double computeDistanceToActualBaseline(TextBaseline baseline) { if (_direction == FlexDirection.horizontal) - return defaultGetDistanceToHighestActualBaseline(baseline); - return defaultGetDistanceToFirstActualBaseline(baseline); + return defaultComputeDistanceToHighestActualBaseline(baseline); + return defaultComputeDistanceToFirstActualBaseline(baseline); } int _getFlex(RenderBox child) { diff --git a/sdk/lib/rendering/object.dart b/sdk/lib/rendering/object.dart index 9624dc32ee0..b384d029deb 100644 --- a/sdk/lib/rendering/object.dart +++ b/sdk/lib/rendering/object.dart @@ -69,8 +69,8 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { assert(!debugDoingPaint); assert(child != null); assert(child.parentData != null); - child.parentData.detach(); child._cleanRelayoutSubtreeRoot(); + child.parentData.detach(); super.dropChild(child); markNeedsLayout(); } @@ -122,7 +122,6 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { assert(parent == this.parent); // TODO(ianh): Remove this once the analyzer is cleverer } else { _nodesNeedingLayout.add(this); - scheduler.ensureVisualUpdate(); } } void _cleanRelayoutSubtreeRoot() { @@ -168,11 +167,12 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { _debugDoingThisLayout = false; _debugCanParentUseSize = null; } catch (e, stack) { - print('Exception raised during layout of ${this}: ${e}'); + print('Exception raised during layout:\n${e}\nContext:\n${this}'); print(stack); return; } _needsLayout = false; + markNeedsPaint(); } void layout(Constraints constraints, { bool parentUsesSize: false }) { final parent = this.parent; // TODO(ianh): Remove this once the analyzer is cleverer @@ -217,7 +217,7 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { // // When calling layout() on your children, pass in // "parentUsesSize: true" if your size or layout is dependent on - // your child's size. + // your child's size or intrinsic dimensions. // when the parent has rotated (e.g. when the screen has been turned // 90 degrees), immediately prior to layout() being called for the diff --git a/sdk/lib/rendering/paragraph.dart b/sdk/lib/rendering/paragraph.dart index dcce468cb7b..605e0950f7b 100644 --- a/sdk/lib/rendering/paragraph.dart +++ b/sdk/lib/rendering/paragraph.dart @@ -154,7 +154,7 @@ class RenderParagraph extends RenderBox { return _getIntrinsicHeight(constraints); } - double getDistanceToActualBaseline(TextBaseline baseline) { + double computeDistanceToActualBaseline(TextBaseline baseline) { assert(!needsLayout); _layout(constraints); sky.Element root = _layoutRoot.rootElement; diff --git a/sdk/lib/rendering/stack.dart b/sdk/lib/rendering/stack.dart index 449b967e3c2..c28a59f93e9 100644 --- a/sdk/lib/rendering/stack.dart +++ b/sdk/lib/rendering/stack.dart @@ -106,8 +106,8 @@ class RenderStack extends RenderBox with ContainerRenderObjectMixin super.root; + + final double baseline; + final TextBaseline baselineType; + + RenderBaseline createNode() => new RenderBaseline(baseline: baseline, baselineType: baselineType); + + void syncRenderObject(Baseline old) { + super.syncRenderObject(old); + root.baseline = baseline; + root.baselineType = baselineType; + } + +} + class SizeObserver extends OneChildRenderObjectWrapper { SizeObserver({ String key, this.callback, Widget child }) diff --git a/sdk/lib/widgets/popup_menu.dart b/sdk/lib/widgets/popup_menu.dart index 6fdf366dc30..bf357eece39 100644 --- a/sdk/lib/widgets/popup_menu.dart +++ b/sdk/lib/widgets/popup_menu.dart @@ -18,7 +18,9 @@ const double _kMenuOpenDuration = 300.0; const double _kMenuCloseDuration = 200.0; const double _kMenuCloseDelay = 100.0; const double _kMenuWidthStep = 56.0; -const double _kMenuMinWidth = 1.5 * _kMenuWidthStep; +const double _kMenuMargin = 16.0; // 24.0 on tablet +const double _kMenuMinWidth = 2.0 * _kMenuWidthStep; +const double _kMenuMaxWidth = 5.0 * _kMenuWidthStep; const double _kMenuHorizontalPadding = 16.0; const double _kMenuVerticalPadding = 8.0; @@ -82,6 +84,7 @@ class PopupMenu extends AnimatedComponent { BoxPainter _painter; double _opacityFor(int i) { + assert(controller.position.value != null); if (controller.position.value == null || controller.position.value == 1.0) return 1.0; double unit = 1.0 / items.length; @@ -99,24 +102,28 @@ class PopupMenu extends AnimatedComponent { return new Opacity( opacity: math.min(1.0, controller.position.value * 3.0), - child: new CustomPaint( - callback: (sky.Canvas canvas, Size size) { - double width = math.min(size.width, size.width * (0.5 + controller.position.value * 2.0)); - double height = math.min(size.height, size.height * controller.position.value * 1.5); - _painter.paint(canvas, new Rect.fromLTRB(size.width - width, 0.0, width, height)); - }, - child: new ConstrainedBox( - constraints: new BoxConstraints( - minWidth: _kMenuMinWidth - ), - child: new ShrinkWrapWidth( - stepWidth: _kMenuWidthStep, - child: new Container( - padding: const EdgeDims.symmetric( - horizontal: _kMenuHorizontalPadding, - vertical: _kMenuVerticalPadding - ), - child: new Block(children) + child: new Container( + margin: new EdgeDims.all(_kMenuMargin), + child: new CustomPaint( + callback: (sky.Canvas canvas, Size size) { + double width = math.min(size.width, size.width * (0.5 + controller.position.value * 2.0)); + double height = math.min(size.height, size.height * controller.position.value * 1.5); + _painter.paint(canvas, new Rect.fromLTRB(size.width - width, 0.0, width, height)); + }, + child: new ConstrainedBox( + constraints: new BoxConstraints( + minWidth: _kMenuMinWidth, + maxWidth: _kMenuMaxWidth + ), + child: new ShrinkWrapWidth( + stepWidth: _kMenuWidthStep, + child: new Container( + padding: const EdgeDims.symmetric( + horizontal: _kMenuHorizontalPadding, + vertical: _kMenuVerticalPadding + ), + child: new Block(children) + ) ) ) ) diff --git a/sdk/lib/widgets/popup_menu_item.dart b/sdk/lib/widgets/popup_menu_item.dart index 17147cd10a8..c87dc0d5ca9 100644 --- a/sdk/lib/widgets/popup_menu_item.dart +++ b/sdk/lib/widgets/popup_menu_item.dart @@ -8,6 +8,9 @@ import 'default_text_style.dart'; import 'ink_well.dart'; import 'theme.dart'; +const double kMenuItemHeight = 48.0; +const double kBaselineOffsetFromBottom = 20.0; + class PopupMenuItem extends Component { PopupMenuItem({ String key, this.child, this.opacity}) : super(key: key); @@ -21,10 +24,13 @@ class PopupMenuItem extends Component { opacity: opacity, child: new InkWell( child: new Container( - height: 48.0, + height: kMenuItemHeight, child: new DefaultTextStyle( style: textStyle, - child: child + child: new Baseline( + baseline: kMenuItemHeight - kBaselineOffsetFromBottom, + child: child + ) ) ) ) diff --git a/specs/style-guide.md b/specs/style-guide.md index 123acd453eb..e475fa810ab 100644 --- a/specs/style-guide.md +++ b/specs/style-guide.md @@ -87,16 +87,16 @@ the expression, even if it is short. (Doing so makes it unobvious that there is relevant code there. This is especially important for early returns.) -If a flow control structure has just one statement, then don't use -braces around it except where doing so would help readability or avoid -the dangling-else problem. Keeping the code free of boilerplate or -redundant punctuation keeps it concise and readable. +If a flow control structure's statement is one line long, then don't +use braces around it. (Keeping the code free of boilerplate or +redundant punctuation keeps it concise and readable.) > For example, > ```dart -> if (children != null) +> if (children != null) { > for (RenderBox child in children) > add(child); +> } > ``` > ...rather than: > ```dart