[Effen] basic layout container class for custom layout

[Effen] Custom layout. For now, FixedHeightScrollable uses a custom BlockLayout algorithm, mostly as a proof of concept.

R=ojan@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/f9d3f0b9d9eaf5123fdb3fa77dcb49e07fc5a94d

[ojan, 2015-04-08 20:08:11]

ojan@chromium.org changed reviewers:
+ ojan@chromium.org

[ojan, 2015-04-08 20:08:12]

https://codereview.chromium.org/1064983002/diff/1/sky/framework/layouts/block...
File sky/framework/layouts/block.dart (right):

https://codereview.chromium.org/1064983002/diff/1/sky/framework/layouts/block...
sky/framework/layouts/block.dart:20: double y = 0.0;
FYI, here you should be doing skyNode.width = skyNode.parentNode.width. Without
that the width of the BlockLayout is always 0, so it looks like you don't paint
anything.

[Hixie, 2015-04-09 16:26:39]

https://codereview.chromium.org/1064983002/diff/1/sky/framework/layouts/block...
File sky/framework/layouts/block.dart (right):

https://codereview.chromium.org/1064983002/diff/1/sky/framework/layouts/block...
sky/framework/layouts/block.dart:20: double y = 0.0;
On 2015/04/08 20:08:11, ojan wrote:
> FYI, here you should be doing skyNode.width = skyNode.parentNode.width.
Without
> that the width of the BlockLayout is always 0, so it looks like you don't
paint
> anything.

why wouldn't the parent set the width?

[ojan, 2015-04-10 23:38:31]

On 2015/04/09 at 16:26:39, ianh wrote:
>
https://codereview.chromium.org/1064983002/diff/1/sky/framework/layouts/block...
> File sky/framework/layouts/block.dart (right):
> 
>
https://codereview.chromium.org/1064983002/diff/1/sky/framework/layouts/block...
> sky/framework/layouts/block.dart:20: double y = 0.0;
> On 2015/04/08 20:08:11, ojan wrote:
> > FYI, here you should be doing skyNode.width = skyNode.parentNode.width.
Without
> > that the width of the BlockLayout is always 0, so it looks like you don't
paint
> > anything.
> 
> why wouldn't the parent set the width?

The way the C++ code works today is that the child itself sets it's width. Part
of this is web legacy, but there's some cases where it's not clear who should
set the width. For example, on an image or an iframe inside a flexbox whose
align-items is set to flex-start (i.e. not stretching). The width should be set
to the intrinsic width of the image/iframe. I guess it should be the job of the
flexbox to set the image/iframe width to it's intrinsic width? That would
simplify the complicated communication that has to happen right now where a
child needs to reach up into it's parent to decide it's size.

[ojan, 2015-04-13 21:49:27]

The only thing I think we should try to work out before committing here is
whether to pass the callback as an argument or use inheritance.

https://codereview.chromium.org/1064983002/diff/20001/sky/framework/fn.dart
File sky/framework/fn.dart (right):

https://codereview.chromium.org/1064983002/diff/20001/sky/framework/fn.dart#n...
sky/framework/fn.dart:591: 
Nit: The style of the other classes doesn't have this extra line break.

https://codereview.chromium.org/1064983002/diff/20001/sky/framework/fn.dart#n...
sky/framework/fn.dart:615: void layout(sky.Element skyNode);
How about we bake this into Container itself and pass the layout callback as an
optional argument to the constructor? That way the layout is not tied to your
inheritance hierarchy.

What I was picturing is that we'd have sky/framework/layouts/block.dart create a
block layout callback that then fixed_height_scrollable.dart would pass in as an
argument to the Container instead of using a BlockLayout node.

https://codereview.chromium.org/1064983002/diff/20001/sky/framework/fn.dart#n...
sky/framework/fn.dart:620: //   set .width if you want to force a width
Setting width here wouldn't work for the BlockLayout case you have below because
then the children's descendants wouldn't be relayouted at this width.

We'll talk about this tomorrow, so go ahead and check this in without us
resolving this stuff, but I think what we want as the final result is that the
parent is 100% responsible for setting the child's x/y/width/height and the
child is responsible for communicating up its intrinsic width/height (e.g. as a
return value from layout?) so that the parent can size it appropriately. Ideally
we'd take this a step further and actually pass any information we need from
ancestors down the tree in a context argument to the layout method and remove
the parentNode pointer from skyNode entirely.

https://codereview.chromium.org/1064983002/diff/20001/sky/framework/fn.dart#n...
sky/framework/fn.dart:623: 
ditto

https://codereview.chromium.org/1064983002/diff/20001/sky/framework/layouts/b...
File sky/framework/layouts/block.dart (right):

https://codereview.chromium.org/1064983002/diff/20001/sky/framework/layouts/b...
sky/framework/layouts/block.dart:6: 
ditto

[Hixie, 2015-04-13 23:16:02]

https://codereview.chromium.org/1064983002/diff/20001/sky/framework/fn.dart
File sky/framework/fn.dart (right):

https://codereview.chromium.org/1064983002/diff/20001/sky/framework/fn.dart#n...
sky/framework/fn.dart:591: 
On 2015/04/13 at 21:49:27, ojan wrote:
> Nit: The style of the other classes doesn't have this extra line break.

As far as I can tell, the style, such as it is, is to have a line break unless
the first thing in the class is a non-static field. (q.v. SkyNodeWrapper,
SkyElementWrapper, Container, Image, and Anchor, which all start with a blank
line).

https://codereview.chromium.org/1064983002/diff/20001/sky/framework/fn.dart#n...
sky/framework/fn.dart:615: void layout(sky.Element skyNode);
On 2015/04/13 at 21:49:27, ojan wrote:
> How about we bake this into Container itself and pass the layout callback as
an optional argument to the constructor? That way the layout is not tied to your
inheritance hierarchy.
> 
> What I was picturing is that we'd have sky/framework/layouts/block.dart create
a block layout callback that then fixed_height_scrollable.dart would pass in as
an argument to the Container instead of using a BlockLayout node.

I could go either way. Will discuss this offline.

https://codereview.chromium.org/1064983002/diff/20001/sky/framework/fn.dart#n...
sky/framework/fn.dart:620: //   set .width if you want to force a width
On 2015/04/13 at 21:49:27, ojan wrote:
> Setting width here wouldn't work for the BlockLayout case you have below
because then the children's descendants wouldn't be relayouted at this width.
> 
> We'll talk about this tomorrow, so go ahead and check this in without us
resolving this stuff, but I think what we want as the final result is that the
parent is 100% responsible for setting the child's x/y/width/height and the
child is responsible for communicating up its intrinsic width/height (e.g. as a
return value from layout?) so that the parent can size it appropriately. Ideally
we'd take this a step further and actually pass any information we need from
ancestors down the tree in a context argument to the layout method and remove
the parentNode pointer from skyNode entirely.

That was basically how I'd designed my custom layout proposal back in the JS
days. Another option is to have the child be responsible for setting its
dimensions, but for the parent to pass in min/max width and height. Then if the
parent wants to force a dimension, it just sets the min/max to the same value.

The problem is this doesn't handle cases like "figure out all the intrinsic
dimensions then distribute the available space in the same ratio". But maybe the
way we handle that is just to do layout twice. I don't really see how to avoid a
double layout anyway in that case.

[ojan, 2015-04-14 00:34:31]

lgtm

I'm OK with trying this for now and seeing if it's painful.

[Hixie, 2015-04-14 17:38:23]

Committed patchset #2 (id:20001) manually as
f9d3f0b9d9eaf5123fdb3fa77dcb49e07fc5a94d (presubmit successful).