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