[haiku-development] Re: BBox and layout
- From: Ingo Weinhold <ingo_weinhold@xxxxxx>
- To: haiku-development@xxxxxxxxxxxxx
- Date: Tue, 15 May 2012 15:21:58 +0200
On 2012-05-14 at 23:56:29 [+0200], Clemens Zeidler
<clemens.zeidler@xxxxxxxxxxxxxx> wrote:
> On Sat, 12 May 2012 03:27:13 +1200, Alex Wilson <yourpalal2@xxxxxxxxx>
> wrote:
> > Alright, this thread has sort of morphed into a discussion of three
> > issues:
> >
> > 1: BBox
> >
> > We have the idea of adding a BView::SetInsets() to deal with this.
> > That seems unclean to me, especially if layouts already support
> > insets. Nesting layouts is really easy, and it's also very powerful.
> > Why add *more* layout features to BView, further confusing things?
> >
> > I still think adding a BBoxLayout is the best solution; it won't
> > require adjusting the BBox use in our apps,
That's an interesting point. How is BBox used in our apps? Looking at the
BBox code it supports both the user setting a layout and the user adding a
single child BView. The former method would obviously break when
introducing a BBoxLayout. Though I suppose it isn't that much work to
adjust the BBox uses in our code base.
> > and doesn't require any
> > deviation from the other parts of the Interface Kit. Clean and
> > consistent :) With this idea, there is really no need for a
> > SetContentLayout() method, since box->GetLayout()->AddItem(myLayout)
> > (or box->AddChild(myLayout) makes perfect sense, just like you can add
> > layouts to a BGroupView.
I would still add BBox::SetContent(BLayoutItem*/BView*) convenience
methods, since (a) they are convenient and (b) the BBox can only have one
content item/view anyway, so a setter fits better than add/remove methods.
That also means that there isn't much speaking for having a separate layout
(BBox could lay out the two children by itself), but that's just an
implementation detail.
> > We also have the idea of having BBox manage the insets of its layout,
> > which would work fine (even without BVIew::SetInsets()). BBox could
> > call SetInsets() on the layout, using a nested layout if you wanted
> > extra insets.
If the content item is laid out to tightly fit the frame, there's no reason
for an additional insets setter, since the child layout would be
responsible for those. BBox would probably be more convenient to use to
have some default insets, though (how does it work ATM?). That would
require a BBox::SetContentInsets() to have a way to override those, though.
I tend to favor this approach, too.
> 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.
There is a clear separation of concerns: A layout gets a rectangular area
and arranges layout items in it. One layout property are the insets for the
original area. BView has two roles: As a simple layout item with size and
alignment constraints and as an entity that drives the layout process.
Neither role requires or even benefits from insets.
> I think from the user point of view its seem odd to add an extra BLayout
> just to add an inset for the inner layout. Note, the placement of an inner
> layout in a BView and the inset of the inner layout are in general managed
> by two different classes, thus the BView should not mess with the inset of
> the inner layout!
Well, it doesn't. In case that wasn't clear the BBox::SetContentInsets()
I'm proposing wouldn't affect the insets property of an inner layout at
all. In general the content is just a layout item (or view), not a layout.
BBox's content insets would only affect the *placement* of the content item.
> In the example of BBox it shows that SetInset make the implementation of
> the placement into the content frame trivial (BBox just calls SetInset in
> its constructor).
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.
> Moreover, the user does not needs any knowledge about
> layout internals and can just set the single layout using SetLayout.
>
> 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.
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.
> > 2: BView::AddChild() -> BLayout::AddItem().
> >
> > Theoretically, I do like the idea of breaking this behaviour, but the
> > potential work included isn't something that I want to do. For this
> > reason, I prefer the proposal of using a view flag to trigger this, or
> > potentially a flag could be passed to AddChild(). Encouraging people
> > to learn the API and focus on using the layout over the view is
> > important, though and breaking this behaviour would do that.
>
> I probably miss some details. What else beside fixing apps has to be done
> in the layout class?
I think that's all. But since it isn't something the compiler will help you
with, it means searching our code base for this type of use and understand,
adjust, and test each instance. Particularly in case of a dynamic GUI it
may be easy to miss some issue when testing.
CU, Ingo
Other related posts:
