views: Move BoxLayout class into views/examples directory.

views: Move BoxLayout class into views/examples directory.

Also add horizontal support as well.

BUG=None
TEST=out/Debug/view_examples, see Widget tab.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=48460

[tfarina, 2010-05-22 02:45:14]

Please, take a look.

(Note: I'd like to move CenterLayout in next CL, too).

[oshima, 2010-05-22 17:41:43]

Thank you for doing this. While we're at it, could you please add a margin
parameter (pixel between children) as well?


On 2010/05/22 02:45:14, tfarina wrote:
> Please, take a look.
> 
> (Note: I'd like to move CenterLayout in next CL, too).

[oshima, 2010-05-22 18:28:26]

oops, forgot to send comments.

http://codereview.chromium.org/2145001/diff/2001/3001
File views/box_layout.cc (right):

http://codereview.chromium.org/2145001/diff/2001/3001#newcode5
views/box_layout.cc:5: #include "views/box_layout.h"
namespace views {

http://codereview.chromium.org/2145001/diff/2001/3001#newcode15
views/box_layout.cc:15: views::View* child = host->GetChildViewAt(i);
We need to exclude invisible view.

http://codereview.chromium.org/2145001/diff/2001/3001#newcode16
views/box_layout.cc:16: child->SetBounds(0, y, width, height / count);
can you use functor to consolidate vertical/horizontal implementation?
same with GetPreferredSize.

http://codereview.chromium.org/2145001/diff/2001/3002
File views/box_layout.h (right):

http://codereview.chromium.org/2145001/diff/2001/3002#newcode9
views/box_layout.h:9: 
namespace views {

http://codereview.chromium.org/2145001/diff/2001/3002#newcode10
views/box_layout.h:10: // A layout manager that layouts child views vertically
and horizontally.
nit: s/and/or

http://codereview.chromium.org/2145001/diff/2001/3002#newcode15
views/box_layout.h:15: kVert,
just use Horizontal, Vertical

[tfarina, 2010-05-22 19:29:27]

http://codereview.chromium.org/2145001/diff/2001/3001
File views/box_layout.cc (right):

http://codereview.chromium.org/2145001/diff/2001/3001#newcode5
views/box_layout.cc:5: #include "views/box_layout.h"
On 2010/05/22 18:28:26, oshima wrote:
> namespace views {

Done.

http://codereview.chromium.org/2145001/diff/2001/3001#newcode15
views/box_layout.cc:15: views::View* child = host->GetChildViewAt(i);
On 2010/05/22 18:28:26, oshima wrote:
> We need to exclude invisible view.
Sorry again, not sure how to do this. Need we to skip the invisible view by
querying its property (IsVisible) or need we to Remove it from the host?

http://codereview.chromium.org/2145001/diff/2001/3001#newcode16
views/box_layout.cc:16: child->SetBounds(0, y, width, height / count);
On 2010/05/22 18:28:26, oshima wrote:
> can you use functor to consolidate vertical/horizontal implementation?
Sorry, what do you mean? To refactor this into a function?
> same with GetPreferredSize.

http://codereview.chromium.org/2145001/diff/2001/3002
File views/box_layout.h (right):

http://codereview.chromium.org/2145001/diff/2001/3002#newcode9
views/box_layout.h:9: 
On 2010/05/22 18:28:26, oshima wrote:
> namespace views {

Done.

http://codereview.chromium.org/2145001/diff/2001/3002#newcode10
views/box_layout.h:10: // A layout manager that layouts child views vertically
and horizontally.
On 2010/05/22 18:28:26, oshima wrote:
> nit: s/and/or

Done.

http://codereview.chromium.org/2145001/diff/2001/3002#newcode15
views/box_layout.h:15: kVert,
On 2010/05/22 18:28:26, oshima wrote:
> just use Horizontal, Vertical

Done.

[tfarina, 2010-05-22 19:37:33]

http://codereview.chromium.org/2145001/diff/5002/7001
File views/box_layout.cc (right):

http://codereview.chromium.org/2145001/diff/5002/7001#newcode25
views/box_layout.cc:25: z = height * (i + margin_) / count;
this is also wrong. :(

[tfarina, 2010-05-22 19:45:45]

http://codereview.chromium.org/2145001/diff/5002/7001
File views/box_layout.cc (right):

http://codereview.chromium.org/2145001/diff/5002/7001#newcode25
views/box_layout.cc:25: z = height * (i + margin_) / count;
On 2010/05/22 19:37:33, tfarina wrote:
> this is also wrong. :(

Fixed.

[sky, 2010-05-23 00:25:40]

Shouldn't we keep this in the example until we actually need it in Chrome
proper?

[Peter Kasting, 2010-05-23 00:31:13]

On 2010/05/23 00:25:40, sky wrote:
> Shouldn't we keep this in the example until we actually need it in Chrome
> proper?

+1

[tfarina, 2010-05-23 00:36:11]

On 2010/05/23 00:31:13, Peter Kasting wrote:
> On 2010/05/23 00:25:40, sky wrote:
> > Shouldn't we keep this in the example until we actually need it in Chrome
> > proper?
> 
> +1

This means to close this issue, or move the class into examples directory?

[sky, 2010-05-23 00:56:27]

I'm fine with closing it, but if you really feel the need move it into
the examples directory and make sure it's only built for the examples.

  -Scott

On Sat, May 22, 2010 at 5:36 PM,  <tfarina@chromium.org> wrote:
> On 2010/05/23 00:31:13, Peter Kasting wrote:
>>
>> On 2010/05/23 00:25:40, sky wrote:
>> > Shouldn't we keep this in the example until we actually need it in
>> > Chrome
>> > proper?
>
>> +1
>
> This means to close this issue, or move the class into examples directory?
>
> http://codereview.chromium.org/2145001/show
>

[tfarina, 2010-05-23 01:20:36]

On 2010/05/23 00:56:27, sky wrote:
> I'm fine with closing it, but if you really feel the need move it into
> the examples directory and make sure it's only built for the examples.

Moved it to examples directory, also now it's only built for the view_examples.
Please, could you take another look?

[sky, 2010-05-24 15:51:30]

http://codereview.chromium.org/2145001/diff/13002/24001
File views/examples/box_layout.cc (right):

http://codereview.chromium.org/2145001/diff/13002/24001#newcode14
views/examples/box_layout.cc:14: void BoxLayout::Layout(views::View* host) {
Layout and GetPreferredSize seem wrong to me. In particular if you give a
boxlayout it's preferred size, none of the views really get their preferred
size. But I'll let Oshima comment on that. At a minimum the doc for the class
should describe how it works.

http://codereview.chromium.org/2145001/diff/13002/24002
File views/examples/box_layout.h (right):

http://codereview.chromium.org/2145001/diff/13002/24002#newcode5
views/examples/box_layout.h:5: #ifndef VIEWS_BOX_LAYOUT_H_
guards need to be updated.

http://codereview.chromium.org/2145001/diff/13002/24002#newcode30
views/examples/box_layout.h:30: Orientation orientation_;
const

http://codereview.chromium.org/2145001/diff/13002/24002#newcode33
views/examples/box_layout.h:33: int margin_;
const

[oshima, 2010-05-26 19:11:34]

http://codereview.chromium.org/2145001/diff/13002/24001
File views/examples/box_layout.cc (right):

http://codereview.chromium.org/2145001/diff/13002/24001#newcode14
views/examples/box_layout.cc:14: void BoxLayout::Layout(views::View* host) {
On 2010/05/24 15:51:31, sky wrote:
> Layout and GetPreferredSize seem wrong to me. In particular if you give a
> boxlayout it's preferred size, none of the views really get their preferred
> size. But I'll let Oshima comment on that.

I was planning to change this to use preferred size so that I can use
this in notification panel and hopefully in submenuview as well.
it's my fault that i checked in incomplete code, so you can remove
GetPreferredSize for now. It's not used in the example anyway.
 Once change is in, i'll fix it.

 At a minimum the doc for the class
> should describe how it works.

[tfarina, 2010-05-27 12:38:21]

http://codereview.chromium.org/2145001/diff/13002/24001
File views/examples/box_layout.cc (right):

http://codereview.chromium.org/2145001/diff/13002/24001#newcode14
views/examples/box_layout.cc:14: void BoxLayout::Layout(views::View* host) {
On 2010/05/26 19:11:34, oshima wrote:
> On 2010/05/24 15:51:31, sky wrote:
> > Layout and GetPreferredSize seem wrong to me. In particular if you give a
> > boxlayout it's preferred size, none of the views really get their preferred
> > size. But I'll let Oshima comment on that.
> 
> I was planning to change this to use preferred size so that I can use
> this in notification panel and hopefully in submenuview as well.
> it's my fault that i checked in incomplete code, so you can remove
> GetPreferredSize for now. It's not used in the example anyway.
>  Once change is in, i'll fix it.
> 
Right, removed.

http://codereview.chromium.org/2145001/diff/13002/24002
File views/examples/box_layout.h (right):

http://codereview.chromium.org/2145001/diff/13002/24002#newcode5
views/examples/box_layout.h:5: #ifndef VIEWS_BOX_LAYOUT_H_
On 2010/05/24 15:51:31, sky wrote:
> guards need to be updated.

Done.

http://codereview.chromium.org/2145001/diff/13002/24002#newcode30
views/examples/box_layout.h:30: Orientation orientation_;
On 2010/05/24 15:51:31, sky wrote:
> const

Done.

http://codereview.chromium.org/2145001/diff/13002/24002#newcode33
views/examples/box_layout.h:33: int margin_;
On 2010/05/24 15:51:31, sky wrote:
> const

Done.

[tfarina, 2010-05-27 21:51:34]

Ping

[oshima, 2010-05-27 21:57:49]

lgtm