[haiku-development] Re: BBox and layout

  • From: Alex Wilson <yourpalal2@xxxxxxxxx>
  • To: haiku-development@xxxxxxxxxxxxx
  • Date: Tue, 8 May 2012 08:08:00 -0600

On Mon, May 7, 2012 at 9:42 PM, <clemens.zeidler@xxxxxxxxxxxxxx> wrote:
>
> > > cool thank! tried it but it did not work very well. I called the
> > > BView::{Disable|Enable}**LayoutInvalidation() but when adding a
> > > item to
> > > the layout the BView::_**InvalidateParentLayout() is called (from
> > > BLayout::AddItem) which does not check if the layout invalidation
> > > is
> > > disabled. Adding the check there helps though. However,
> > > additionally
> > > calling BLayout::{Disable|Enable}**LayoutInvalidation() solves the
> > > problem. What is the cleanest solution? and why are there flags for
> > > layout
> > > invalidation in BView and BLayout shouldn't BView just use the one
> > > from
> > > BLayout?
> >
> >
> > BView::_InvalidateParentLayout() resides in BView mainly because it
> > needs
> > access to BView stuff, and is useful even in the absence of a
> > BLayout. That
> > said, it's just a convenient way to find the nearest layout and
> > invalidate
> > it, regardless of where the invalidation is coming from, so it's
> > right that
> > it doesn't check for invalidation being disabled.
> >
> > BLayout maybe could share invalidation disabled flags with the BView,
> > when
> > it is attached to one, I can't remember any specific reason why it
> > doesn't.
>
> Ok I removed fInvalidationDisabled and BLayout::
> {Disable|Enable}LayoutInvalidation() from BLayout. Think BLayout is not
> the place to decide if it triggers a relayout. This functionality is
> only in BView now. However BLayout::InvalidateLayout checks if the
> invalidation is disabled in BView. (please take a look at the
> attachment)

I don't think it's good to remove this functionality across the board,
because in a complex layout, you might have many nested layouts, and
maybe you wouldn't want changing some of the nested ones to invalidate
your main layout. In the case of a layout with an fOwner (see next
comment), it is probably reasonable to defer to the BView on
invalidation disabledness.

> By the way what is the difference between fOwner and fTarget in
> BLayout?

fOwner is the view which owns the layout, fTarget is the view that
holds children of the layout. I.e.

BView* a  = _MyView();
a->SetLayout(_MyMainLayout());
_MyMainLayout()->AddItem(_MySubLayout());

_MyView() is the target and owner of _MyMainLayout(), _MySubLayout()
has no owner, but targets _MyView().


> > I don't think there's any need for any new methods. In the scenario
> > of
> > using a default custom BBox layout, if a user wants full manual
> > control
> > over everything in the BBox, they just use SetLayout(), if they want
> > to
> > embed a layout into the BBox content area, they add it with
> > AddChild().
> > This would also have the benefit of being backwards compatible with
> > current
> > behaviour, meaning there's no need to rework all the layout'd BBoxes
> > in the
> > repo.
>
> Hey no backwards compatibility for beta apis! ;-)

Haha, I think it's sometimes allowed, as long as you can find a way to
break at least a few apps.

> So you propose to
> leave everything as it is or do I miss something? AddChild is even more
> unusable than the current SetLayout, not only you have to calculate the
> correct inset but also the frame size and add a dummy layout container
> view!

No, in layout mode, AddChild() would nicely layout the child you add
into the BBox in the right place, with the right insets. The custom
layout would just lay out 2 things, the label, and the child (BBox
only allows one child sort of). If you want multiple items in the
BBox, you would add a nested layout using
BBox::AddChild(BLayoutItem*). Because BLayout is also a BLayoutItem,
you no longer need the dummy layout containers.

> So again, I would change the default BBox semantic of SetLayout to add
> a special BBoxLayout that has exactly one child which is the added
> layout. This layout is placed exactly in the inner BBox frame. Guess in
> 99% thats what you want.

That's almost what I want, but I think playing with SetLayout like
that would be wrong. BView::SetLayout() should basically do the same
thing as BBox::SetLayout(). It's better, in my opinion, to default to
using a BBoxLayout when the view is built with the layout
constructors, or with the B_SUPPORTS_LAYOUT flag, and let the user
replace it if they want.

> In the docs for BBox we can add: for the (unlikely) case that the
> layout should span the complete BBox frame the BView::SetLayout method
> should be called.

I think this doesn't follow the principle of least astonishment, which
IMO is very important in API design. For an api user, embedding
layouts using AddItem/AddChild should be the obvious way to do things.

--Alex

Other related posts: