On Thu, 2 Feb 2023 19:54:33 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 with a new target base due to a > merge or a rebase. In general, platform preferences correspond to OS-level settings and are updated dynamically. Third-party themes might integrate platform preferences into their look and feel, which is often what users expect to see. But consider a scenario where an application uses a third-party theme that adapts to the OS appearance, but the application author only wants to support a dark appearance (independent from the OS appearance). For this scenario, platform preferences should be overridable from application code. I've considered several potential approaches: #### 1. Provide two sets of preferences: class Platform { Preferences getUserPreferences(); Preferences getPlatformPreferences(); } In this idea, `getUserPreferences` returns a mutable version of the immutable `getPlatformPreferences`. However, this requires theme authors to always use user preferences instead of platform preferences (or combine both); if they fail to do so, application authors are out of luck. #### 2. Add a separate, overridable property for each of the convenience API properties: interface Preferences { ... ReadOnlyObjectProperty<Appearance> appearanceProperty(); ObjectProperty<Appearance> appearanceOverrideProperty(); ... } The value of the read-only `appearanceProperty()` then corresponds to the value of `appearanceOverrideProperty()` if the property is set to a non-null value. Otherwise, it corresponds to the platform-provided value. #### 3. Add a special setter for each of the convenience API properties: interface Preferences { ... ReadOnlyObjectProperty<Appearance> appearanceProperty(); void setAppearance(Appearance appearance); ... } The `setAppearance` method can be used to _override_ the value of `appearanceProperty()`. When `null` is passed into this method, the platform default value is restored (or, put differently, the override is cleared). There is some precedence in JavaFX for a "special setter" pattern: for example, while the `Stage.widthProperty()` is read-only, `Stage.setWidth` is a special setter that attempts to set the stage width to the specified value (and may fail to do so). I prefer the third option (the special setter), as it seems to be the cleanest approach that doesn't unnecessarily expand the API. I think I've come around to the idea of dropping the `override` method in favor of `put`. However, I don't like the idea of using `put("key", null)` to reset an overridden mapping to its default value. Having "reset" be its own operation seems to work quite well, though: interface Preferences extends ObservableMap<String, Object> { ... /** * Overrides a preference mapping. * <p> * If a platform-provided mapping for the key already exists, calling this method overrides * the value that is mapped to the key. If a platform-provided mapping for the key doesn't * exist, this method creates a new mapping. * * @param key the key * @param value the new value * @throws NullPointerException if {@code key} or {@code value} is null * @throws IllegalArgumentException if a platform-provided mapping for the key exists, and * the specified value is an instance of a different class * than the platform-provided value * @return the previous value associated with {@code key} */ Object put(String key, Object value); /** * Resets an overridden preference mapping to its platform-provided value. * <p> * If the preference is overridden, but the platform does not provide a mapping for the * specified key, the mapping will be removed. If no mapping exists for the specified * key, calling this method has no effect. * * @param key the key * @throws NullPointerException if {@code key} is null */ void reset(String key); /** * Resets all overridden preference mappings to their platform-provided values and removes * all mappings for which the platform does not provide a default value. */ void reset(); } ------------- PR Comment: https://git.openjdk.org/jfx/pull/1014#issuecomment-1501177237 PR Comment: https://git.openjdk.org/jfx/pull/1014#issuecomment-1531375810
