[Bf-committers] UI layout: Why 'align' property of layouts is inherited?

[Bastien Montagne]

On 23/08/2013 14:37, Brecht Van Lommel wrote:
> On Thu, Aug 22, 2013 at 11:33 PM, Bastien Montagne
> <montagne29 at wanadoo.fr>  wrote:
>> Was wondering, why 'align' layout flag is de-facto inherited for all
>> most common layout types (rows, columns, etc.)? I won't pretend to have
>> a complete understanding of the layout engine, but this is currently
>> giving me white hair...
>>
>> This very simple patch seems to fix this (afaict):
>> http://www.pasteall.org/45073/diff
>>
>> It will induce edits in UI (e.g. 3DView tools, where some group of
>> options are currently drawn 'aligned' even though there immediate layout
>> is not set so), but imho it makes much more sense this way.
>>
>> Else it’s very confusing, especially as py code never know whether the
>> layout given to it is aligned or not!
> The reason it works this way is because the align option for buttons
> is internally using the old align flags, for which there is only one
> level, it doesn't have any notion of nested layout types. Such aligned
> layouts over both column and rows aren't used much anymore though (I
> actually couldn't find one quickly) so it could be changed, it's
> always possible to manually set align flags for all levels.
Hmm… I’m fine with it, as long as all nested layouts are defined as aligned.

E.g. in:
     col = layout.column(True)
     row = col.row(False)

... items in row will still be aligned. This is bad, imho (and DingTo 
agreed here). However, if both col and row are defined as 'aligned', I 
see no problem with the recursive call (and hence aligning all items in 
both col and row into a single 'group'). That’s what my patch is doing.

So yes, I guess we should forbid that implicit 'alignment', and force to 
specify it at each level of layout, when required (I think at least the 
four gpencil buttons in 3D view's object tools use such an implicit 
column+row alignment).

>> As I was in UI layout code, I also made "labelled items" (like e.g.
>> string, non-expanded enums, etc.) sensible to current layout's direction
>> (like expanded enums), so that the label is displayed above them when in
>> vertical layout. This is what we already often do in py UI, using stupid
>> UILayout.label calls for that. This simple change allows me e.g. to get
>> rid of for label() calls in WeightEdit modifier's UI...
> Sometimes you want it vertical if the width is narrow, but if it's
> wide enough you want it horizontal. What the patch seems to do now is
> not actually work this way in all cases. For a panel the top level
> layout is a vertical layout but it still makes the items horizontal?
> Otherwise e.g. the first Render panel layout would break.
Yeah... don’t know why this happens indeed, in my two-columns layout of 
WeightEdit modifier, it works as expected... Anyway, that left/top label 
modification is more of an oportunity-change, we can let this aside for 
now. The only thing I really need here is to explicitly inherit the 
'align' parameter from parent layout, so that text fields & co still 
behave as expected when aligned... ;)

>> if (!is_vert)
>>      w = w - labelw;
> What is the reason for this change? It changes all the horizontal
> label + button width a little bit but why?
Well, when we have label above widget, we do not need all that width 
sharing, we have full width available for both! Else (label to the left 
of widget, aka horizontal sublayout), there is no change here from 
previous code.

>
> My main concern with this patch is that it breaks API compatibility,
> do we want to do this with 2.69 for a bit of code cleanup or is there
> a more important reason to do it where it's impossible to achieve some
> layout?
I had to dive into this because else I could not get my uiList filtering 
UI right (as I use an aligned column to layout the list itself, the grip 
+ filter_show buttons, and the filter options, but then do not want all 
filter options to be aligned into a single group!). Else I have to add 
separators, but these take way too much room!

Bastien