RFR: 8301302: Platform preferences API [v18]

Nir Lisker nlisker at openjdk.org
Tue Oct 31 23:03:40 UTC 2023


On Tue, 31 Oct 2023 17:28:35 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

>> Please read [this document](https://gist.github.com/mstr2/9f46f92c98d3c86aa6a0b4224a9a6548) for an introduction to the Platform Preferences API, and how it interacts with the proposed style theme and stage appearance features.
>
> Michael Strauß has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - formatting
>  - Javadoc change

modules/javafx.graphics/src/main/java/javafx/application/Appearance.java line 31:

> 29:  * Defines the appearance of the user interface.
> 30:  *
> 31:  * @since 22

I would add an `@see javafx.application.Platform.Preferences#appearanceProperty()` tag (if I got the syntax right) because it's not clear how and where to use this class from the description.

Can there be other uses for this enum outside of the current one in the above property? If so, it should be documented.

modules/javafx.graphics/src/main/java/javafx/application/Appearance.java line 44:

> 42:      */
> 43:     DARK
> 44: 

Minor:

I was told once that JavaFX uses a `;` after the last enum element.

Also no need for the extra empty line.

modules/javafx.graphics/src/main/java/javafx/application/Platform.java line 459:

> 457:      * Contains UI preferences of the current platform.
> 458:      * <p>
> 459:      * {@code Preferences} extends {@link ObservableMap} to expose platform preferences as key-value pairs.

I would mentioned here (like in `getPreferences`) that this map is unmodifiable to the user, and that changes by the underlying platform can be tracked by listening on this map.

modules/javafx.graphics/src/main/java/javafx/application/Platform.java line 567:

> 565:      */
> 566:     public interface Preferences extends ObservableMap<String, Object> {
> 567:         /**

Missing empty line.

modules/javafx.graphics/src/main/java/javafx/application/Platform.java line 636:

> 634:          *         if no mapping exists for the specified key
> 635:          */
> 636:         Optional<Double> getDouble(String key);

I'm a bit confused about this and similar methods. Several points:

1. There is no value that is a `Double`, and also no `Paint`, so I'm not sure what these are for considering that list gives all possible valid entries.

2. If the list of keys in the table is fully known, wouldn't an enum make more sense and be more safe than of a `String`?

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1378165295
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1378158983
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1378222691
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1378235234
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1378223632


More information about the openjfx-dev mailing list