On Wed, 26 Mar 2025 19:57:59 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:

>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TitledPaneSkin.java
>>  line 160:
>> 
>>> 158:         registerChangeListener(titleRegion.alignmentProperty(), e -> 
>>> pos = titleRegion.getAlignment());
>>> 159: 
>>> 160:         // NOTE: Listening to two different alignment properties, and 
>>> using the last value of either is likely a bug!
>> 
>> I agree: it is definitely a bug.  I think we should remove it along with the 
>> possibility of specifying conflicting alignment via CSS.  The control must 
>> be in control.
>
> Yeah, the reason I was hesitant to change this is that currently you can set 
> the alignment from CSS in two ways:
> 
>      .titled-pane {
>          -fx-alignment: RIGHT;
>      }
>      
> Or:
>  
>     .titled-pane > .title {
>          -fx-alignment: RIGHT;
>     }     
>     
> Both will work at the moment.  I'm not sure which one is more correct.  
> 
> A titled pane naturally has two major regions; its title area, and its 
> content area.  The top level alignment could apply to either or none in 
> theory.  The top level alignment setting is "inherited" from being a Labeled 
> control, which only has a single alignment to worry about (this also shows 
> using this kind of inheritance is just a bad idea).  It would have been much 
> more clear to have a `titleAlignment` and `contentAlignment` property.
> 
> At the "top" level there is nothing to align, since a titled pane always fill 
> its width.

the original code had some logic around `(pos == null)`, I wonder if we need to 
do the same here, that is, derive the effective alignment from both values?

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1742#discussion_r2014933886

Reply via email to