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/com/sun/javafx/application/PlatformImpl.java line 1098: > 1096: } else { > 1097: setAccessibilityTheme(null); > 1098: } Can this not be written more simply as String val = Boolean.TRUE.equals(preferences.get("Windows.SPI.HighContrastOn")) && preferences.get("Windows.SPI.HighContrastColorScheme") instanceof String s ? s : null; setAccessibilityTheme(val); ? modules/javafx.graphics/src/main/java/com/sun/javafx/application/preferences/PlatformPreferences.java line 61: > 59: final Map<String, Object> effectivePreferences = new HashMap<>(); > 60: final Map<String, Object> unmodifiableEffectivePreferences = > Collections.unmodifiableMap(effectivePreferences); > 61: final PreferenceProperties properties = new > PreferenceProperties(this); Why are these not `private`? I don't see them being used outside of this class. Also, if possible, add some documentation on these fields so maintainers can understand better what each is for, modules/javafx.graphics/src/main/java/com/sun/javafx/application/preferences/PlatformPreferences.java line 67: > 65: > 66: public PlatformPreferences(Map<String, String> wellKnownKeys) { > 67: this.wellKnownKeys = wellKnownKeys; Is the argument map guaranteed to not change outside of this class, or is there a need for a defensive copy? modules/javafx.graphics/src/main/java/com/sun/javafx/application/preferences/PlatformPreferences.java line 192: > 190: /** > 191: * Updates this map of preferences with a new set of platform > preferences. > 192: * The specified preferences may include all available preferences, > or only the changed preferences. I would mention here that removed preferences are specified by being mapped to `null`, but that the resulting preferences (after update) cannot map to `null`. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1378839517 PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1378817922 PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1380059912 PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1380097328
