RFR: 8313424: JavaFX controls in the title bar [v21]
Andy Goryachev
angorya at openjdk.org
Tue Nov 5 19:59:54 UTC 2024
On Tue, 5 Nov 2024 01:32:16 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:
>> This PR is a new take on a highly requested feature: JavaFX controls in the header bar (see also #594 for an earlier iteration).
>>
>> This is a feature with many possible ways to skin the cat, and it has taken quite a bit of effort to come up with a good user model. In contrast to the previous iteration, the focus has shifted from providing an entirely undecorated window to providing a window with a user-configurable header bar.
>>
>> The customizable header bar is a new layout container: `javafx.scene.layout.HeaderBar`. It has three areas that accept child nodes: leading, center, and trailing. `HeaderBar` also automatically adjusts for the placement of the default window buttons (minimize, maximize, close) on the left or right side of the window.
>>
>> The customizable header bar is combined with a new `EXTENDED` stage style, which extends the client area into the header bar area. The new extended stage style is supported on Windows, macOS, and Linux. For platforms that don't support this stage style, it automatically downgrades to `DECORATED`.
>>
>> This is how it looks like on each of the three operating systems:
>>
>> 
>>
>> The window buttons (minimize, maximize, close) are provided by JavaFX, not by the application developer. This makes it easier to get basic window functionality without recreating the entirety of the window controls for all platforms.
>>
>> ## Usage
>> This is a minimal example that uses a custom header bar with a `TextField` in the center area. `HeaderBar` is usually placed in the top area of a `BorderPane` root container:
>>
>> public class MyApp extends Application {
>> @Override
>> public void start(Stage stage) {
>> var headerBar = new HeaderBar();
>> headerBar.setCenter(new TextField());
>>
>> var root = new BorderPane();
>> root.setTop(headerBar);
>>
>> stage.setScene(new Scene(root));
>> stage.initStyle(StageStyle.EXTENDED);
>> stage.show();
>> }
>> }
>>
>> To learn more about the details of the API, refer to the documentation of `StageStyle.EXTENDED` and `HeaderBar`.
>>
>> ## Platform integration
>> The implementation varies per platform, and ranges from pretty easy to quite involved:
>> 1. **macOS**: The window buttons are provided by macOS, we just leave an empty area where the window buttons will appear. The client area is extended to cover the entire window by setting the `NSW...
>
> Michael Strauß has updated the pull request incrementally with one additional commit since the last revision:
>
> stylistic changes
second batch of comments
modules/javafx.graphics/src/main/java/javafx/scene/layout/HeaderBar.java line 118:
> 116:
> 117: private static final String ALIGNMENT = "headerbar-alignment";
> 118: private static final String MARGIN = "headerbar-margin";
Q: should these be a String or an Object? String.equals() are more expensive.
(I guess we use Strings elsewhere, so probably ok).
modules/javafx.graphics/src/main/java/javafx/scene/layout/HeaderBar.java line 170:
> 168: // Inflate the minHeight property. This is important so that we can track whether a stylesheet or
> 169: // user code changes the property value before we set it to the height of the native title bar.
> 170: minHeightProperty();
if that's the case, why not initialize it in L1150 ?
modules/javafx.graphics/src/main/java/javafx/scene/layout/HeaderBar.java line 262:
> 260: @Override
> 261: protected double computeMinWidth(double height) {
> 262: Node leading = getLeading(), center = getCenter(), trailing = getTrailing();
I think it's a bad practice to have multiple statements on one line.
modules/javafx.graphics/src/main/java/javafx/scene/layout/HeaderBarBase.java line 51:
> 49: * can specify draggable content nodes of the {@code HeaderBarBase} with the {@link #setDraggable} method.
> 50: *
> 51: * @apiNote Most application developers should use the {@link HeaderBar} implementation instead of
just curious: what's the reason to split the HB into the two classes? Can you give an example?
modules/javafx.graphics/src/main/java/javafx/scene/layout/HeaderBarBase.java line 138:
> 136: private void updateInsets() {
> 137: if (currentFullScreen || currentMetrics == null) {
> 138: leftSystemInset.set(new Dimension2D(0, 0));
Q: would it make sense to declare a
`static final Dimension2D EMPTY = new Dimension2D(0, 0)`
and use it everywhere?
modules/javafx.graphics/src/main/java/javafx/scene/layout/HeaderBarBase.java line 190:
> 188: * independent of layout orientation.
> 189: */
> 190: private final ReadOnlyObjectWrapper<Dimension2D> rightSystemInset =
Q: would it make sense to create a class with requestLayout() ? this will save two class objects...
modules/javafx.graphics/src/main/java/javafx/stage/StageStyle.java line 83:
> 81: * <p>
> 82: * An extended window has the default window buttons (minimize, maximize, close), but no system-provided
> 83: * draggable header bar. Applications need to provide their own header bar by placing a single {@link HeaderBar}
I would like to see this expanded just a little bit, to give an example of what is (system menu, buttons) and is not rendered (rounded corners, app title), similarly to the the table in the JEP.
I would prefer a table or an `<ul>`, and I specifically want to include the window title. Window title is a property, and I think it deserves a mention (will it be shown in the tray? in the "running applications" os widget?)
I don't know whether the best place to include that is here or in the `HeaderBar`.
modules/javafx.graphics/src/main/native-glass/mac/GlassViewDelegate.m line 1336:
> 1334: }
> 1335:
> 1336: - (NSEvent*)lastEvent
could there be side effects with exposing this field?
concurrency issues?
modules/javafx.graphics/src/main/resources/com/sun/glass/ui/gtk/WindowDecorationGnome.css line 26:
> 24: */
> 25:
> 26: .window-button-container {
is it possible for the app dev to alter the styling with an external style sheet?
modules/javafx.graphics/src/main/resources/com/sun/glass/ui/gtk/WindowDecorationGnome.css line 125:
> 123:
> 124: .close-button > .glyph {
> 125: -fx-shape: "m 8.1464844,8.1464844 a 0.5,0.5 0 0 0 0,0.7070312 L 11.292969,12 8.1464844,15.146484 a 0.5,0.5 0 0 0 0,0.707032 0.5,0.5 0 0 0 0.7070312,0 L 12,12.707031 l 3.146484,3.146485 a 0.5,0.5 0 0 0 0.707032,0 0.5,0.5 0 0 0 0,-0.707032 L 12.707031,12 15.853516,8.8535156 a 0.5,0.5 0 0 0 0,-0.7070312 0.5,0.5 0 0 0 -0.707032,0 L 12,11.292969 8.8535156,8.1464844 a 0.5,0.5 0 0 0 -0.7070312,0 z";
minor: we could probably minimize the path by rounding to 2 or 3 decimal points
(same comment for all .css changes)
modules/javafx.graphics/src/main/resources/com/sun/glass/ui/win/WindowDecoration.css line 107:
> 105:
> 106: .maximize-button.restore > .glyph {
> 107: -fx-shape: "m 7.6491699,2.5192871 q 0,-0.3444824 -0.1369629,-0.6495361 Q 7.3752441,1.5646973 7.1407471,1.338501 6.90625,1.1123047 6.5970459,0.98156738 6.2878418,0.85083008 5.9475098,0.85083008 H 1.7722168 Q 1.838623,0.65991211 1.9589844,0.50219727 2.0793457,0.34448242 2.2370605,0.23242188 2.3947754,0.12036133 2.5836182,0.06018066 2.7724609,0 2.9758301,0 H 5.9475098 Q 6.4746094,0 6.9394531,0.20129395 7.4042969,0.40258789 7.7508545,0.74707031 8.0974121,1.0915527 8.2987061,1.5563965 8.5,2.0212402 8.5,2.5483398 V 5.5241699 Q 8.5,5.7275391 8.4398193,5.9163818 8.3796387,6.1052246 8.2675781,6.2629395 8.1555176,6.4206543 7.9978027,6.5410156 7.8400879,6.661377 7.6491699,6.7277832 Z M 1.253418,8.5 Q 1.0043945,8.5 0.77612305,8.3983154 0.54785156,8.2966309 0.37561035,8.1243896 0.20336914,7.9521484 0.10168457,7.723877 0,7.4956055 0,7.246582 V 2.9550781 Q 0,2.7019043 0.10168457,2.475708 0.20336914,2.2495117 0.37561035,2.0772705 0.54785156,1.9050293 0.77404785,1.8033447 1.0002441,1.70166
02 1.253418,1.7016602 h 4.2915039 q 0.2531738,0 0.4814453,0.1016845 0.2282715,0.1016846 0.3984375,0.2718506 0.170166,0.170166 0.2718506,0.3984375 0.1016845,0.2282715 0.1016845,0.4814453 V 7.246582 q 0,0.2531739 -0.1016845,0.4793701 Q 6.5949707,7.9521484 6.4227295,8.1243896 6.2504883,8.2966309 6.024292,8.3983154 5.7980957,8.5 5.5449219,8.5 Z M 5.5241699,7.6491699 q 0.087158,0 0.1639405,-0.033203 0.076782,-0.033203 0.1369628,-0.091309 0.060181,-0.058105 0.093384,-0.1348877 0.033203,-0.076782 0.033203,-0.1639404 v -4.25 q 0,-0.087158 -0.033203,-0.1660156 Q 5.8852539,2.730957 5.8271484,2.6728516 5.769043,2.6147461 5.6901855,2.581543 5.6113281,2.5483398 5.5241699,2.5483398 h -4.25 q -0.087158,0 -0.1639404,0.033203 -0.076782,0.033203 -0.1348877,0.093384 -0.0581055,0.060181 -0.0913086,0.1369628 -0.0332031,0.076782 -0.0332031,0.1639405 v 4.25 q 0,0.087158 0.0332031,0.1639404 0.0332031,0.076782 0.0913086,0.1348877 0.0581055,0.058105 0.1348877,0.091309 0.076782,0.033203 0.1639404,0.033203 z";
... especially here
modules/javafx.graphics/src/test/java/test/javafx/scene/layout/HeaderBarTest.java line 70:
> 68: "BOTTOM_CENTER, 10, 10, 100, 80",
> 69: "BOTTOM_RIGHT, 10, 10, 100, 80"
> 70: })
these tests can be simplified to avoid parsing with @CsvSource and default conversion, example:
@ParameterizedTest
@CsvSource(value = {"yo,true,1",
"bah,true,1.1",
"meh,false,2.2"})
void test(String candidate, boolean expected, double a) {
System.out.println("candidate=" + candidate + " expected=" + expected + " a=" +a );
}
-------------
PR Review: https://git.openjdk.org/jfx/pull/1605#pullrequestreview-2416113079
PR Review Comment: https://git.openjdk.org/jfx/pull/1605#discussion_r1829669611
PR Review Comment: https://git.openjdk.org/jfx/pull/1605#discussion_r1829671987
PR Review Comment: https://git.openjdk.org/jfx/pull/1605#discussion_r1829674239
PR Review Comment: https://git.openjdk.org/jfx/pull/1605#discussion_r1829687841
PR Review Comment: https://git.openjdk.org/jfx/pull/1605#discussion_r1829847752
PR Review Comment: https://git.openjdk.org/jfx/pull/1605#discussion_r1829849333
PR Review Comment: https://git.openjdk.org/jfx/pull/1605#discussion_r1829857742
PR Review Comment: https://git.openjdk.org/jfx/pull/1605#discussion_r1829868608
PR Review Comment: https://git.openjdk.org/jfx/pull/1605#discussion_r1829874022
PR Review Comment: https://git.openjdk.org/jfx/pull/1605#discussion_r1829875372
PR Review Comment: https://git.openjdk.org/jfx/pull/1605#discussion_r1829876147
PR Review Comment: https://git.openjdk.org/jfx/pull/1605#discussion_r1829926610
More information about the openjfx-dev
mailing list