RFR: 8301302: Platform preferences API [v18]

Michael Strauß mstrauss at openjdk.org
Wed Nov 1 03:15:20 UTC 2023


On Tue, 31 Oct 2023 21:04:15 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> 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/glass/ui/Application.java line 786:
> 
>> 784:      * @return a map of platform-specific keys to well-known keys
>> 785:      */
>> 786:     public Map<String, String> getWellKnownPlatformPreferenceKeys() {
> 
> Not really liking the name.  Aren't these keys that are mapped to FX keys?  Perhaps `getPlatformSpecificMappings`

I chose `getPlatformKeyMappings` to put more emphasis on the fact that this is a mapping of keys to keys.

> modules/javafx.graphics/src/main/java/javafx/application/Platform.java line 575:
> 
>> 573:          *
>> 574:          * @return the {@code appearance} property
>> 575:          */
> 
> Should this include `@defaultValue` ?

Yes.

> modules/javafx.graphics/src/main/java/javafx/application/Platform.java line 607:
> 
>> 605:          * The accent color.
>> 606:          * <p>
>> 607:          * If the platform does not report an accent color, this property defaults to {@code #157EFB}.
> 
> Perhaps include what that color represents (`vivid blue`)?

Done.

> modules/javafx.graphics/src/main/java/javafx/application/Platform.java line 617:
> 
>> 615: 
>> 616:         /**
>> 617:          * Returns the {@code Integer} instance to which the specified key is mapped.
> 
> I think the wording of all of these should be modified slightly.  No integer instance is returned.  When dealing with optional I usually word it as:
> 
>      Returns an optional {@code Integer} to which ...
> 
> ... and I leave out the `Optional.empty` part.
> 
> I'd also leave out all the `instance` suffixes.

I reworded all of these methods, but added a clarification that the `IllegalArgumentException` is only thrown if a mapping exists, but the value is wrong.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1378350039
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1378350135
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1378348626
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1378349137


More information about the openjfx-dev mailing list