Adding Point & Rect primitives

Adding Point & Rect primitives

For making DOM APIs more user-friendly, a number of them should be exposed as points and rects. Adding basic classes with a core set of useful methods.

BUG=

Committed: https://code.google.com/p/dart/source/detail?r=19577

[blois, 2013-03-05 02:08:51]

Please take a look, let me know if you think the API is overkill or
insufficient, naming suggestions, etc.

These are roughly based off of Closure's primitives with a few additions &
modifications (primarily these are immutable, Closure's are mutable).

https://codereview.chromium.org/12395036/diff/2001/sdk/lib/html/dart2js/html_...
File sdk/lib/html/dart2js/html_dart2js.dart (left):

https://codereview.chromium.org/12395036/diff/2001/sdk/lib/html/dart2js/html_...
sdk/lib/html/dart2js/html_dart2js.dart:18895: -class Rect native "*Rect" {
Ignore removal of this for now- this is being renamed to CssRect in another CL
(and RgbColor to CssRgbColor)

https://codereview.chromium.org/12395036/diff/2001/tools/dom/src/Rect.dart
File tools/dom/src/Rect.dart (right):

https://codereview.chromium.org/12395036/diff/2001/tools/dom/src/Rect.dart#ne...
tools/dom/src/Rect.dart:25: factory Rect.fromCorners(Point a, Point b) {
Seems useful but not sure about naming.

[Jacob, 2013-03-05 02:20:21]

Looks reasonable. I'd like to see all current cases where the DOM returns a
point or a rect use these classes before we switch.
E.g. get rid or merge with ClientRect.

https://codereview.chromium.org/12395036/diff/2001/tools/dom/src/Rect.dart
File tools/dom/src/Rect.dart (right):

https://codereview.chromium.org/12395036/diff/2001/tools/dom/src/Rect.dart#ne...
tools/dom/src/Rect.dart:9: */
Will you use this class any time the DOM currently returns a rect?

[Jennifer Messerly, 2013-03-05 02:35:21]

yeah, this LGTM but i'm also curious about how it's going to be used :)

https://codereview.chromium.org/12395036/diff/2001/tests/html/point_test.dart
File tests/html/point_test.dart (right):

https://codereview.chromium.org/12395036/diff/2001/tests/html/point_test.dart...
tests/html/point_test.dart:6: import '../../pkg/unittest/lib/unittest.dart';
does "package:" work from tests?

https://codereview.chromium.org/12395036/diff/2001/tests/html/rect_test.dart
File tests/html/rect_test.dart (right):

https://codereview.chromium.org/12395036/diff/2001/tests/html/rect_test.dart#...
tests/html/rect_test.dart:30: }, throws);
test exception type?

https://codereview.chromium.org/12395036/diff/2001/tools/dom/src/Point.dart
File tools/dom/src/Point.dart (right):

https://codereview.chromium.org/12395036/diff/2001/tools/dom/src/Point.dart#n...
tools/dom/src/Point.dart:7: /// A utility class for representing two-dimensional
positions.
just curious, why /// here but /** */ below?
(i don't care either way, just noticed it :) )

https://codereview.chromium.org/12395036/diff/2001/tools/dom/src/Point.dart#n...
tools/dom/src/Point.dart:12: Point([num x = 0, num y = 0]): x = x, y = y;
this could be "const" to allow construction of const points

https://codereview.chromium.org/12395036/diff/2001/tools/dom/src/Point.dart#n...
tools/dom/src/Point.dart:14: String toString() {
both are valid, but sometimes I like using inline style for this:

    String toString() => '($x, $y)';

https://codereview.chromium.org/12395036/diff/2001/tools/dom/src/Rect.dart
File tools/dom/src/Rect.dart (right):

https://codereview.chromium.org/12395036/diff/2001/tools/dom/src/Rect.dart#ne...
tools/dom/src/Rect.dart:14: final num height;
right, bottom getters?

https://codereview.chromium.org/12395036/diff/2001/tools/dom/src/Rect.dart#ne...
tools/dom/src/Rect.dart:25: factory Rect.fromCorners(Point a, Point b) {
On 2013/03/05 02:08:51, blois wrote:
> Seems useful but not sure about naming.

new Rect.fromPoints?

https://codereview.chromium.org/12395036/diff/2001/tools/dom/src/Rect.dart#ne...
tools/dom/src/Rect.dart:60: * Returns null if there is no intersection
period at the end

https://codereview.chromium.org/12395036/diff/2001/tools/dom/src/Rect.dart#ne...
tools/dom/src/Rect.dart:83: top <= other.top + other.height && other.top <= top
+ height);
+2 indent

[blois, 2013-03-05 21:43:33]

Updated with review changes.

I'm working on exposing exposing these as well, but the work is mostly
independent of this code. 

Can hold off submitting this until I get the types being exposed (working on
ClientRect right now).

https://codereview.chromium.org/12395036/diff/2001/tests/html/point_test.dart
File tests/html/point_test.dart (right):

https://codereview.chromium.org/12395036/diff/2001/tests/html/point_test.dart...
tests/html/point_test.dart:6: import '../../pkg/unittest/lib/unittest.dart';
On 2013/03/05 02:35:21, John Messerly wrote:
> does "package:" work from tests?

Appears to, but no other tests are using it. Will give it a try.

https://codereview.chromium.org/12395036/diff/2001/tests/html/rect_test.dart
File tests/html/rect_test.dart (right):

https://codereview.chromium.org/12395036/diff/2001/tests/html/rect_test.dart#...
tests/html/rect_test.dart:30: }, throws);
On 2013/03/05 02:35:21, John Messerly wrote:
> test exception type?

Done.

https://codereview.chromium.org/12395036/diff/2001/tools/dom/src/Point.dart
File tools/dom/src/Point.dart (right):

https://codereview.chromium.org/12395036/diff/2001/tools/dom/src/Point.dart#n...
tools/dom/src/Point.dart:7: /// A utility class for representing two-dimensional
positions.
On 2013/03/05 02:35:21, John Messerly wrote:
> just curious, why /// here but /** */ below?
> (i don't care either way, just noticed it :) )

No reason, changed.

https://codereview.chromium.org/12395036/diff/2001/tools/dom/src/Point.dart#n...
tools/dom/src/Point.dart:12: Point([num x = 0, num y = 0]): x = x, y = y;
On 2013/03/05 02:35:21, John Messerly wrote:
> this could be "const" to allow construction of const points

Done.

https://codereview.chromium.org/12395036/diff/2001/tools/dom/src/Point.dart#n...
tools/dom/src/Point.dart:14: String toString() {
On 2013/03/05 02:35:21, John Messerly wrote:
> both are valid, but sometimes I like using inline style for this:
> 
>     String toString() => '($x, $y)';

Done.

https://codereview.chromium.org/12395036/diff/2001/tools/dom/src/Rect.dart
File tools/dom/src/Rect.dart (right):

https://codereview.chromium.org/12395036/diff/2001/tools/dom/src/Rect.dart#ne...
tools/dom/src/Rect.dart:9: */
On 2013/03/05 02:20:21, Jacob wrote:
> Will you use this class any time the DOM currently returns a rect?

Yes, we should.

https://codereview.chromium.org/12395036/diff/2001/tools/dom/src/Rect.dart#ne...
tools/dom/src/Rect.dart:14: final num height;
On 2013/03/05 02:35:21, John Messerly wrote:
> right, bottom getters?

Yeah, remembered those shortly after I sent this.

https://codereview.chromium.org/12395036/diff/2001/tools/dom/src/Rect.dart#ne...
tools/dom/src/Rect.dart:25: factory Rect.fromCorners(Point a, Point b) {
On 2013/03/05 02:35:21, John Messerly wrote:
> On 2013/03/05 02:08:51, blois wrote:
> > Seems useful but not sure about naming.
> 
> new Rect.fromPoints?

Done.

https://codereview.chromium.org/12395036/diff/2001/tools/dom/src/Rect.dart#ne...
tools/dom/src/Rect.dart:60: * Returns null if there is no intersection
On 2013/03/05 02:35:21, John Messerly wrote:
> period at the end

Done.

https://codereview.chromium.org/12395036/diff/2001/tools/dom/src/Rect.dart#ne...
tools/dom/src/Rect.dart:83: top <= other.top + other.height && other.top <= top
+ height);
On 2013/03/05 02:35:21, John Messerly wrote:
> +2 indent

Done.

[blois, 2013-03-05 23:18:53]

https://codereview.chromium.org/12395036/diff/20001/tools/dom/src/Rectangle.dart
File tools/dom/src/Rectangle.dart (right):

https://codereview.chromium.org/12395036/diff/20001/tools/dom/src/Rectangle.d...
tools/dom/src/Rectangle.dart:16: const Rect(this.left, this.top, this.width,
this.height);
Made this constructor const as well, which necessitated the removal of the
negative width/height check.

Thoughts?

[Jennifer Messerly, 2013-03-05 23:43:08]

https://codereview.chromium.org/12395036/diff/20001/tools/dom/src/Rectangle.dart
File tools/dom/src/Rectangle.dart (right):

https://codereview.chromium.org/12395036/diff/20001/tools/dom/src/Rectangle.d...
tools/dom/src/Rectangle.dart:16: const Rect(this.left, this.top, this.width,
this.height);
On 2013/03/05 23:18:54, blois wrote:
> Made this constructor const as well, which necessitated the removal of the
> negative width/height check.
> 
> Thoughts?

Hmmm. Interesting. Yeah, maybe that isn't the right tradeoff. I emailed misc for
other options, but I'd be inclined to drop "const" and keep validation. WDYT?

https://codereview.chromium.org/12395036/diff/20001/tools/dom/src/Rectangle.d...
tools/dom/src/Rectangle.dart:45: // TODO: implement with mixins when available.
TODO(blois) -- just so we know who has the context about it?

[blois, 2013-03-06 19:20:12]

Committed patchset #4 manually as r19577 (presubmit successful).