On Mon, 15 Apr 2024 12:38:15 GMT, eduardsdv <d...@openjdk.org> wrote:

>> This change fixes the calculation of which nodes go to the toolbar and which 
>> go to the overflow menu.
>> It is now determined before the nodes are removed from the scene graph.
>> This is important because the values returned by 
>> ``Node.prefWidth(..)``/``Node.prefHeight(..)`` may depend on whether the 
>> node is added to the scene graph.
>> 
>> Furthermore I corrected the ``hasOveflow`` value passed to the 
>> ``organizeOverflow(double, boolean)`` in ``correctOverflow(double)``.
>
> eduardsdv has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - JDK-8328577: Update comment
>  - JDK-8328577: Bind style related properties

modules/javafx.controls/src/main/java/javafx/scene/control/skin/ToolBarSkin.java
 line 573:

> 571:         overflowBox.idProperty().bind(box.idProperty());
> 572:         overflowBox.getStyleClass().setAll(box.getStyleClass());
> 573:         box.getStyleClass().addListener((ListChangeListener<? super 
> String>) change -> overflowBox.getStyleClass().setAll(change.getList()));

I think what you need here (and below) is `Bindings.bindContent(List, 
ObservableList)`

modules/javafx.controls/src/main/java/javafx/scene/control/skin/ToolBarSkin.java
 line 575:

> 573:         box.getStyleClass().addListener((ListChangeListener<? super 
> String>) change -> overflowBox.getStyleClass().setAll(change.getList()));
> 574:         overflowBox.getStylesheets().setAll(box.getStylesheets());
> 575:         box.getStylesheets().addListener((ListChangeListener<? super 
> String>) change -> overflowBox.getStylesheets().setAll(change.getList()));

this is interesting.  

isn't `overflowBox` a sibling of `box`, having the same parent, and therefore 
inheriting the same set of stylesheets from the parent `Scene`?

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1434#discussion_r1565958505
PR Review Comment: https://git.openjdk.org/jfx/pull/1434#discussion_r1565977562

Reply via email to