On Sat, 4 Jul 2026 18:27:14 GMT, Michael Strauß <[email protected]> wrote:

>> JavaFX controls in the title bar were previewed with 
>> [JDK-8313424](https://bugs.openjdk.org/browse/JDK-8313424) (delivered in JFX 
>> 25) and the API was subsequently refined with 
>> [JDK-8369836](https://bugs.openjdk.org/browse/JDK-8369836) and 
>> [JDK-8370446](https://bugs.openjdk.org/browse/JDK-8370446) (delivered in JFX 
>> 26). The feedback collected in two preview rounds suggests that the API is 
>> fit for purpose, and no fundamental changes are required. This means that 
>> the feature is ready to become final and permanent in JFX 27.
>> 
>> I propose a few small changes to smooth out remaining edges:
>> 1. **Rename `HeaderBar.prefButtonHeight` to `HeaderBar.systemButtonHeight`**
>>    This more clearly expresses that it is the height of the 
>> _system-provided_ buttons.
>> 2. **Rename `HeaderBar.minSystemHeight` to `HeaderBar.systemMinHeight`**
>>    What is meant is not a _minimum system height_ (which makes no sense), 
>> but a _system-provided minimum height_.
>> 3. **Add `HeaderBar.systemColorScheme`**
>>    In the previous iteration, the color scheme of the header buttons on 
>> Windows and Linux was determined by the brightness of the `Scene.fill` 
>> background. On macOS, which uses header buttons drawn by the operating 
>> system, the color scheme of the header buttons was instead determined by the 
>> color scheme of the window itself.
>> 
>>    The newly added `HeaderBar.systemColorScheme` attached property allows 
>> applications to specify the color scheme for system-provided header buttons 
>> directly, without falling back to either the scene background or the window 
>> color scheme. The default value of the property is `null`, which means "no 
>> preference"; in this case, the color scheme for header buttons will match 
>> the color scheme for the window on all platforms. Setting this property to a 
>> non-null value is only necessary in the rare case when an application needs 
>> different color schemes for header buttons and the window (i.e. when it has 
>> a bright title bar in dark mode, or a dark title bar in light mode).
>> 
>> Since these changes are only cosmetic (in the case of renamed properties) 
>> and don't fundamentally change the API of the enhancement (in the case of 
>> `HeaderBar.systemColorScheme`), they shouldn't need to be previewed for 
>> another round.
>> 
>> The rest of the significant changes in this PR are a clean-up pass over the 
>> documentation, and having the macOS header button implementation support the 
>> new `HeaderBar.systemColorScheme` property.
>> 
>> ## Note on the use of attached properties
>> This e...
>
> Michael Strauß has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   lazy properties

Some more findings. My testing looks good. This was on my list for a long time 
but did not used it until now.

Javadoc is very well written. I used it mostly to figure out the API and it 
guided me very well.

modules/javafx.controls/src/main/java/javafx/scene/control/DialogPane.java line 
451:

> 449:         if (headerBar == null) {
> 450:             headerBar = new SimpleObjectProperty<>(this, "headerBar") {
> 451:                 WeakReference<HeaderBar> wref = new 
> WeakReference<>(null);

`oldHeaderBar` might be a bit better and follows the other properties in other 
controls

modules/javafx.graphics/src/main/java/javafx/scene/layout/HeaderBar.java line 
541:

> 539:      * @defaultValue {@code null}
> 540:      */
> 541:     private final ObjectProperty<Node> left = new NodeProperty("left");

I wonder if this properties (including `leftSystemPadding` and 
`rightSystemPadding` can be made lazy as well. The `xxxProperty` method are 
unused by default, so unless developers bind it or use the setter, the 
properties will not be initialized.

modules/javafx.graphics/src/main/java/javafx/scene/layout/HeaderDragType.java 
line 99:

> 97:      * In contrast to {@link #DRAGGABLE}, which positively identifies a 
> node as a draggable part of the
> 98:      * {@code HeaderBar}, this option excludes a node from draggable-area 
> hit testing: the header bar
> 99:      * behaves as if the node was not present and continues hit testing 
> unimpeded.

`TRANSPARENT` was the the hardest for me to understand. I think you did a good 
job already explaining it - But to fully understand, one more sentence what it 
means that the hit testing continues would help a lot to clarify IMO.

modules/javafx.graphics/src/main/java/javafx/stage/StageStyle.java line 115:

> 113:      * }</pre>
> 114:      *
> 115:      * <h4>Title text</h4>

Not just the title text but also the application icon in case of Windows and 
probably most of Linux. Probably good to mention as well? Since for both, 
clicking on the icon gives a `ContextMenu` which is also lost - so good to know 
for the developer.

modules/javafx.graphics/src/test/java/test/javafx/scene/layout/HeaderBarTest.java
 line 103:

> 101:     }
> 102: 
> 103:     <T> T getAttachedProperty(String name) {

Should this be called `getExtendedProperties` as well?

modules/javafx.graphics/src/test/java/test/javafx/scene/layout/HeaderBarTest.java
 line 167:

> 165:         @Test
> 166:         void attachedPropertyPublishesExplicitValue() {
> 167:             var colorScheme = new ColorScheme[1];

Minor: `AtomicReference` could be used. I personally like it a bit more

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

PR Review: https://git.openjdk.org/jfx/pull/2188#pullrequestreview-4630255148
PR Review Comment: https://git.openjdk.org/jfx/pull/2188#discussion_r3523851763
PR Review Comment: https://git.openjdk.org/jfx/pull/2188#discussion_r3523846917
PR Review Comment: https://git.openjdk.org/jfx/pull/2188#discussion_r3523850185
PR Review Comment: https://git.openjdk.org/jfx/pull/2188#discussion_r3523859198
PR Review Comment: https://git.openjdk.org/jfx/pull/2188#discussion_r3523880554
PR Review Comment: https://git.openjdk.org/jfx/pull/2188#discussion_r3523880136

Reply via email to