On Tue, 31 Oct 2023 21:04:15 GMT, John Hendrikx <[email protected]> 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
