From 3e0c6d586e89149b4366407f2233273db0938577 Mon Sep 17 00:00:00 2001 From: Hixie Date: Thu, 1 Oct 2015 15:11:30 -0700 Subject: [PATCH] EdgeDims changes and other fixes to core classes. - Rename EdgeDims constructor to EdgeDims.TRBL(). - Add operator== to Size and Offset so that you can compare Size to DebugSize in checked mode. - Add Size.lerp(). - Add various operators to EdgeDims. (*, /, ~/, %) - Add EdgeDims.lerp(). - Update style guide. I went there to fix an EdgeDims constructor example, and stayed because some recent things came up and I wanted to add them before I forgot. --- examples/stocks/lib/stock_row.dart | 2 +- sky/engine/core/painting/Offset.dart | 3 + sky/engine/core/painting/OffsetBase.dart | 1 - sky/engine/core/painting/Size.dart | 16 +++ .../sky/lib/src/painting/box_painter.dart | 98 +++++++++++++++---- sky/packages/sky/lib/src/widgets/binding.dart | 2 - sky/packages/sky/lib/src/widgets/dialog.dart | 4 +- sky/specs/style-guide.md | 68 ++++++++++++- sky/unit/test/painting/edge_dims_test.dart | 14 +++ sky/unit/test/rendering/size_test.dart | 18 ++++ 10 files changed, 196 insertions(+), 30 deletions(-) create mode 100644 sky/unit/test/painting/edge_dims_test.dart create mode 100644 sky/unit/test/rendering/size_test.dart diff --git a/examples/stocks/lib/stock_row.dart b/examples/stocks/lib/stock_row.dart index 28e54980d67..4b8f47a65d8 100644 --- a/examples/stocks/lib/stock_row.dart +++ b/examples/stocks/lib/stock_row.dart @@ -47,7 +47,7 @@ class StockRow extends StatelessComponent { onLongPress: onLongPressed, child: new InkWell( child: new Container( - padding: const EdgeDims(16.0, 16.0, 20.0, 16.0), + padding: const EdgeDims.TRBL(16.0, 16.0, 20.0, 16.0), decoration: new BoxDecoration( border: new Border( bottom: new BorderSide(color: Theme.of(context).dividerColor) diff --git a/sky/engine/core/painting/Offset.dart b/sky/engine/core/painting/Offset.dart index e840f09a7eb..ee7ca9382ef 100644 --- a/sky/engine/core/painting/Offset.dart +++ b/sky/engine/core/painting/Offset.dart @@ -45,6 +45,9 @@ class Offset extends OffsetBase { /// Returns the point at (0, 0) plus this offset. Point toPoint() => new Point(this.dx, this.dy); + /// Compares two Offsets for equality. + bool operator ==(other) => other is Offset && super == other; + /// Linearly interpolate between two offsets /// /// If either offset is null, this function interpolates from [Offset.zero]. diff --git a/sky/engine/core/painting/OffsetBase.dart b/sky/engine/core/painting/OffsetBase.dart index 8697abdbbca..d775766421a 100644 --- a/sky/engine/core/painting/OffsetBase.dart +++ b/sky/engine/core/painting/OffsetBase.dart @@ -19,7 +19,6 @@ abstract class OffsetBase { bool operator ==(other) { return other is OffsetBase && - other.runtimeType == runtimeType && other._dx == _dx && other._dy == _dy; } diff --git a/sky/engine/core/painting/Size.dart b/sky/engine/core/painting/Size.dart index e3c6aa98684..a4814980e41 100644 --- a/sky/engine/core/painting/Size.dart +++ b/sky/engine/core/painting/Size.dart @@ -45,5 +45,21 @@ class Size extends OffsetBase { Point bottomLeft(Point origin) => new Point(origin.x, origin.y + height); Point bottomRight(Point origin) => new Point(origin.x + width, origin.y + height); + /// Compares two Sizes for equality. + bool operator ==(other) => other is Size && super == other; + + /// Linearly interpolate between two sizes + /// + /// If either size is null, this function interpolates from [Offset.zero]. + static Size lerp(Size a, Size b, double t) { + if (a == null && b == null) + return null; + if (a == null) + return b * t; + if (b == null) + return a * (1.0 - t); + return new Size(lerpDouble(a.width, b.width, t), lerpDouble(a.height, b.height, t)); + } + String toString() => "Size($width, $height)"; } diff --git a/sky/packages/sky/lib/src/painting/box_painter.dart b/sky/packages/sky/lib/src/painting/box_painter.dart index 6126fcbb1e7..4fad4379027 100644 --- a/sky/packages/sky/lib/src/painting/box_painter.dart +++ b/sky/packages/sky/lib/src/painting/box_painter.dart @@ -14,9 +14,8 @@ import 'package:sky/src/painting/shadows.dart'; /// Typically used for an offset from each of the four sides of a box. For /// example, the padding inside a box can be represented using this class. class EdgeDims { - // TODO(abarth): Remove this constructor or rename it to EdgeDims.fromTRBL. /// Constructs an EdgeDims from offsets from the top, right, bottom and left - const EdgeDims(this.top, this.right, this.bottom, this.left); + const EdgeDims.TRBL(this.top, this.right, this.bottom, this.left); /// Constructs an EdgeDims where all the offsets are value const EdgeDims.all(double value) @@ -47,32 +46,89 @@ class EdgeDims { bool get isNonNegative => top >= 0.0 && right >= 0.0 && bottom >= 0.0 && left >= 0.0; - bool operator ==(other) { - if (identical(this, other)) - return true; - return other is EdgeDims - && top == other.top - && right == other.right - && bottom == other.bottom - && left == other.left; + EdgeDims operator-(EdgeDims other) { + return new EdgeDims.TRBL( + top - other.top, + right - other.right, + bottom - other.bottom, + left - other.left + ); } EdgeDims operator+(EdgeDims other) { - return new EdgeDims(top + other.top, - right + other.right, - bottom + other.bottom, - left + other.left); + return new EdgeDims.TRBL( + top + other.top, + right + other.right, + bottom + other.bottom, + left + other.left + ); } - EdgeDims operator-(EdgeDims other) { - return new EdgeDims(top - other.top, - right - other.right, - bottom - other.bottom, - left - other.left); + EdgeDims operator*(double other) { + return new EdgeDims.TRBL( + top * other, + right * other, + bottom * other, + left * other + ); + } + + EdgeDims operator/(double other) { + return new EdgeDims.TRBL( + top / other, + right / other, + bottom / other, + left / other + ); + } + + EdgeDims operator~/(double other) { + return new EdgeDims.TRBL( + (top ~/ other).toDouble(), + (right ~/ other).toDouble(), + (bottom ~/ other).toDouble(), + (left ~/ other).toDouble() + ); + } + + EdgeDims operator%(double other) { + return new EdgeDims.TRBL( + top % other, + right % other, + bottom % other, + left % other + ); + } + + bool operator ==(other) { + return identical(this, other) || + (other is EdgeDims && + top == other.top && + right == other.right && + bottom == other.bottom && + left == other.left); + } + + /// Linearly interpolate between two EdgeDims + /// + /// If either is null, this function interpolates from [EdgeDims.zero]. + static EdgeDims lerp(EdgeDims a, EdgeDims b, double t) { + if (a == null && b == null) + return null; + if (a == null) + return b * t; + if (b == null) + return a * (1.0 - t); + return new EdgeDims.TRBL( + sky.lerpDouble(a.top, b.top, t), + sky.lerpDouble(a.right, b.right, t), + sky.lerpDouble(a.bottom, b.bottom, t), + sky.lerpDouble(a.left, b.left, t) + ); } /// An EdgeDims with zero offsets in each direction - static const EdgeDims zero = const EdgeDims(0.0, 0.0, 0.0, 0.0); + static const EdgeDims zero = const EdgeDims.TRBL(0.0, 0.0, 0.0, 0.0); int get hashCode { int value = 373; @@ -142,7 +198,7 @@ class Border { /// The widths of the sides of this border represented as an EdgeDims EdgeDims get dimensions { - return new EdgeDims(top.width, right.width, bottom.width, left.width); + return new EdgeDims.TRBL(top.width, right.width, bottom.width, left.width); } int get hashCode { diff --git a/sky/packages/sky/lib/src/widgets/binding.dart b/sky/packages/sky/lib/src/widgets/binding.dart index fd005ad0fb9..792c6a161a0 100644 --- a/sky/packages/sky/lib/src/widgets/binding.dart +++ b/sky/packages/sky/lib/src/widgets/binding.dart @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:async'; - import 'package:sky/animation.dart'; import 'package:sky/rendering.dart'; import 'package:sky/src/widgets/framework.dart'; diff --git a/sky/packages/sky/lib/src/widgets/dialog.dart b/sky/packages/sky/lib/src/widgets/dialog.dart index b9b24ad8903..9098bebc1d4 100644 --- a/sky/packages/sky/lib/src/widgets/dialog.dart +++ b/sky/packages/sky/lib/src/widgets/dialog.dart @@ -70,7 +70,7 @@ class Dialog extends StatelessComponent { if (title != null) { EdgeDims padding = titlePadding; if (padding == null) - padding = new EdgeDims(24.0, 24.0, content == null ? 20.0 : 0.0, 24.0); + padding = new EdgeDims.TRBL(24.0, 24.0, content == null ? 20.0 : 0.0, 24.0); dialogBody.add(new Padding( padding: padding, child: new DefaultTextStyle( @@ -83,7 +83,7 @@ class Dialog extends StatelessComponent { if (content != null) { EdgeDims padding = contentPadding; if (padding == null) - padding = const EdgeDims(20.0, 24.0, 24.0, 24.0); + padding = const EdgeDims.TRBL(20.0, 24.0, 24.0, 24.0); dialogBody.add(new Padding( padding: padding, child: new DefaultTextStyle( diff --git a/sky/specs/style-guide.md b/sky/specs/style-guide.md index 968efc25a2e..fa27b13428b 100644 --- a/sky/specs/style-guide.md +++ b/sky/specs/style-guide.md @@ -27,8 +27,9 @@ variables, constants, enum values, etc) is lowerCamelCase. Constant doubles and strings are prefixed with k. Prefer using a local const or a static const in a relevant class than using a global constant. -Don't name your libraries (no ```library``` keyword). Name the files -in ```lower_under_score.dart``` format. +Don't name your libraries (no ```library``` keyword), unless it's a +documented top-level library, like `painting.dart`. Name the files in +```lower_under_score.dart``` format. Class constructors and methods should be ordered in the order that @@ -39,6 +40,27 @@ declarations. The default (unnamed) constructor should come first, then the named constructors. +If you call super() in your initialiser list, put a space between the +constructor arguments closing parenthesis and the colon. If there's +other things in the initialiser list, align the super() call with the +other arguments. Don't call super if you have no arguments to pass up +to the superclass. + +```dart +class Foo extends Bar { + Foo({ this._argument, baz }) : super(baz: baz); +} + +class Quuz extends Bar { + Quuz({ + TheType argument, baz + }) : _argument = argument, + super( + baz: baz + ); +} +``` + Fields should come before the methods that manipulate them, if they are specific to a particular group of methods. @@ -83,6 +105,14 @@ When breaking a parameter list into multiple lines, do the same. Use `//` and `///`, not `/* */` and `/** */`. +Prefer single quotes for strings. Use double quotes for nested +strings. + +> Example: +> ```dart +> print('Hello ${name.split(" ")[0]}'); +> ``` + Don't put the statement part of an "if" statement on the same line as the expression, even if it is short. (Doing so makes it unobvious that there is relevant code there. This is especially important for early @@ -118,7 +148,7 @@ options. > ``` > ...rather than: > ```dart -> new EdgeDims(0.0, 8.0, 0.0, 8.0); +> new EdgeDims.TRBL(0.0, 8.0, 0.0, 8.0); > ``` @@ -126,6 +156,38 @@ Use for-in loops rather than forEach() where possible, since that saves a stack frame per iteration. +When naming callbacks, use `FooCallback` for the typedef, `onFoo` (or, +if there's only one and the whole purpose of the class is this +callback, `callback`) for the callback argument or property, and +`handleFoo` for the method that is called. + +When defining mutable properties that mark a class dirty when set, use +the following pattern: + +```dart +/// Documentation here (don't wait for a later commit). +TheType get theProperty => _theProperty; +TheType _theProperty; +void set theProperty(TheType value) { + assert(value != null); + if (_theProperty == value) + return; + _theProperty = value; + markNeedsWhatever(); // the method to mark the object dirty +} +``` + +The argument is called 'value' for ease of copy-and-paste reuse of +this pattern. If for some reason you don't want to use 'value', use +'newTheProperty' (where 'theProperty' is the property name). + +Start the method with any asserts you need to validate the value. + + +If you have variables or methods that are only used in release mode, +prefix their names with `debug` or `_debug`. + + C++ --- diff --git a/sky/unit/test/painting/edge_dims_test.dart b/sky/unit/test/painting/edge_dims_test.dart new file mode 100644 index 00000000000..bfcb4583f38 --- /dev/null +++ b/sky/unit/test/painting/edge_dims_test.dart @@ -0,0 +1,14 @@ +import 'package:sky/painting.dart'; + +import 'package:test/test.dart'; + +void main() { + test("EdgeDims.lerp()", () { + EdgeDims a = new EdgeDims.all(10.0); + EdgeDims b = new EdgeDims.all(20.0); + expect(EdgeDims.lerp(a, b, 0.25), equals(a * 1.25)); + expect(EdgeDims.lerp(a, b, 0.25), equals(b * 0.625)); + expect(EdgeDims.lerp(a, b, 0.25), equals(a + const EdgeDims.all(2.5))); + expect(EdgeDims.lerp(a, b, 0.25), equals(b - const EdgeDims.all(7.5))); + }); +} diff --git a/sky/unit/test/rendering/size_test.dart b/sky/unit/test/rendering/size_test.dart new file mode 100644 index 00000000000..76e84bcda2c --- /dev/null +++ b/sky/unit/test/rendering/size_test.dart @@ -0,0 +1,18 @@ +import 'package:sky/rendering.dart'; +import 'package:test/test.dart'; + +import 'rendering_tester.dart'; + +void main() { + test('Stack can layout with top, right, bottom, left 0.0', () { + RenderBox box = new RenderConstrainedBox( + additionalConstraints: new BoxConstraints.tight(const Size(100.0, 100.0))); + + layout(box, constraints: const BoxConstraints()); + + expect(box.size.width, equals(100.0)); + expect(box.size.height, equals(100.0)); + expect(box.size, equals(const Size(100.0, 100.0))); + expect(box.size.runtimeType.toString(), equals('_DebugSize')); + }); +}