On Tue, 31 Oct 2023 17:28:35 GMT, Michael Strauß <mstra...@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
