[haiku-development] Re: BBox and layout
- From: Ingo Weinhold <ingo_weinhold@xxxxxx>
- To: haiku-development@xxxxxxxxxxxxx
- Date: Thu, 17 May 2012 16:31:24 +0200
On 2012-05-16 at 22:23:50 [+0200], Clemens <clemens.zeidler@xxxxxxxxxxxxxx>
wrote:
> On Wed, 16 May 2012 01:21:58 +1200, Ingo Weinhold <ingo_weinhold@xxxxxx>
> wrote:
> >> After giving it a another thought I still think SetInset should be in
> >> BView for consistency reasons. As said before BView is a special
> >> layout/layout item. It's somehow an artificial BLayoutItem or BLayout.
> >> It
> >> can be added into a layout and handled like a normal BLayoutItem. For
> >> this
> >> reason when adding a SetInset to BLayout it should also be added to
> >> BView
> >> (similar to the layout related size getter and setter in BView).
> >
> > I can't quite follow this train of thought. BView is essentially a
> > BLayoutItem (and it really would be, had that been possible to implement
> > in
> > a binary compatible way). I don't see how that implies that because
> > BLayout
> > has insets BView should have insets, too.
>
> yes BView is a hybrid of an BLayoutItem and a BLayout, BView would not
> only be a BLayoutItem, it would be a BLayout. For all important BLayout
> methods there are already equivalent methods in BView.
BView delegates all layout functionality to its layout. Admittedly it also
has a DoLayout() method, but that is just a convenient alternative for
simple cases (like BBox) where implementing a real layout class would add
unnecessary overhead. This really is an alternative -- you can't have both a
layout and still do layout in BView at the same time. It is also worth
mentioning that due to the {Add,Remove}Child() methods not being virtual,
there are serious limitation as to what layout a view class can do without
providing specialized mandatory methods for adding/removing children.
> The BView, as a BLayout, has no "Add*" layout method but just one
> SetLayout method because it can exactly hold one layout.
There is a BView::AddChild(BLayoutItem*) which can be used to add layouts.
BView acts a façade in this respect, delegating everything to its layout.
Consequently a BView::SetInsets() should also be delegated to
GetLayout()->SetInsets().
Which isn't what your proposed method would do -- hence it would have to
have a different name, clearly stating what it does (like
SetAdditional[Layout]Insets()). However, it is mostly duplicate
functionality. The layout already has insets and it would just add the
additional insets. Since the owner of the view usually also controls the
layout, setting the added insets on the layout would work just as well. As
explained in my previous mail the method wouldn't work very well for BBox
anyway. And in the rare remaining cases where one strictly wants to keep the
two sets of insets separate using nested layouts would be a very simple
alternative. In short, this is functionality that isn't needed.
> > Since border style and label of the BBox can also be set any time after
> > construction, insets would have to be updated then as well. Furthermore
> > the
> > size constraints of the label view can change, which would also have that
> > implication. That is in practice you can't easily override the insets
> > for a
> > BBox.
>
> bummer, yes SetInset cannot provide a clean solution in a label resize
> event. However, I would still hide the BBoxLayout complexity and make it
> working by overwriting BBox::SetLayout and BBox::SetInset.
And we're back at the beginning. As written in my first mail on the topic,
it's just a question of how we want to perceive BBox: 1. as a frame for some
item (e.g. a container) or as a framed container. In both cases there are
the two options of either laying out the content item/set the layout insets
to tightly fit the frame -- in which case the user would be responsible for
adding pleasant insets in the content item/layout (most easily by nesting
layouts) -- or already use some additional default insets. For the latter
option the additional insets should be overridable
(BBox::SetContentInsets()).
Apparently you prefer alternative 2, BBox as a framed container. This is
fine, although personally I favor alternative 1. However, I don't see why
you are insisting on BView::SetInsets(). There is really no need to have
that method in BView and it doesn't even work very well with BBox (I don't
see how overriding the method in BBox would help in this respect).
> >> A BBoxLayout::AddItem call to set a single layout is confusing for the
> >> user, i.e. does it means that multiple layouts can be assigned to a
> >> BBox?
> >
> > As written above, I'd add BBox::SetContent() methods (and a BLayoutItem*
> > constructor version -- there already is one for a BView* child) to
> > simplify
> > and clarify the API.
>
> But there is still the problem that the user could try to use SetLayout to
> set the content layout. (that would happened for user like me :) ) This
> confusion can be avoided by using the more general BView api.
BBox::SetLayout() can simply call the debugger, which will clear up any
confusion a developer (who did neither manage to read the documentation to
the point where it says the that BBox has a content item/view that has to be
set nor find the SetContent() method in the header) might have. I don't find
the consideration for developers who don't even try to get the most basic
idea of how to use a class a very good argument for API design.
> > Such a BBox API also doesn't require any knowledge of "layout
> > internals". I
> > think it's at least as easy to grasp that a BBox is a frame with a
> > content
> > item that one can set as how certain insets would add up and that the
> > label
> > view isn't managed by the layout.
>
> The more basic and simpler view on a BBox is that it's just a simple BView
> that does some drawing and sets a default inset for the layout.
I beg to differ. IMO considering a BBox a frame around some other item/view
is at least as simple. You see how even expressing the idea requires fewer
words. :-P
Anyway, I feel the discussion is getting repetitive. The main decision to be
made is what BBox shall be: a frame for an item/view or a framed container.
Browing a bit through the preferences, the BBox objects used in a layout
context seem to be used exclusively as the former (they don't set a layout
on the BBox and add a single child). I also favor that approach.
CU, Ingo
Other related posts:
