On Thu, 7 Dec 2023 15:24:11 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> Michael Strauß has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> renamed Windows.SPI.HighContrastOn to Windows.SPI.HighContrast
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/application/PlatformImpl.java
> line 779:
>
>> 777: if (highContrastScheme == null) {
>> 778: return;
>> 779: }
>
> Minor: You could eliminate the null check if you defined a new "NONE" or
> "UNKNOWN" enum and restored the (no-op) `default:` on line 837. It's fine the
> way you have it, if you prefer.
I've added a `NONE` constant.
> modules/javafx.graphics/src/main/java/com/sun/javafx/application/WindowsHighContrastScheme.java
> line 80:
>
>> 78: // since we might be running on a JVM with a locale that is
>> different from the OS.
>> 79: for (WindowsHighContrastScheme item : values()) {
>> 80: for (ResourceBundle resourceBundle : resourceBundles) {
>
> Depending on how often this is called, might it be worth caching a
> `Map<String,WindowsHighContrastScheme>` whose keys are the OS theme names and
> values are the corresponding enum values? This could be a follow-up
> enhancement if it were deemed important enough, but maybe it doesn't matter.
This class will probably change a bit and be moved around if and when we add
style themes, so I think we can revisit it then.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1419313343
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1419316074
