RFR: 8248: [Accessibility] Low Contrast in High Contrast mode on Stacktrace View [v3]

Suchita Chaturvedi schaturvedi at openjdk.org
Mon Sep 16 08:37:14 UTC 2024


On Mon, 16 Sep 2024 07:49:59 GMT, Brice Dutheil <bdutheil at openjdk.org> wrote:

>> Suchita Chaturvedi has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
>> 
>>  - Merge branch 'openjdk:master' into 8248
>>  - Restart not required to apply the DARK mode appearance preference
>>  - 8248: [Accessibility] Low Contrast in High Contrast mode on Stacktrace View
>>    
>>    Co-authored By: Marcus Hirt <hirt at openjdk.org>
>
> application/org.openjdk.jmc.ui.common/src/main/java/org/openjdk/jmc/ui/common/util/ThemeUtils.java line 13:
> 
>> 11: public class ThemeUtils {
>> 12: 
>> 13: 	private static boolean isCurrentThemeDark = isDarkMode();
> 
> **suggestion:** 
> 
>> But seems like spotbugs is not happy with updating static variable isCurrentThemeDark from the listener
> 
> I don't know if it's alright by JMC spotbug rules to write this global state like this
> 
> Suggestion:
> 
> 	private static AtomicReference<Boolean> isCurrentThemeDark = new AtomicReference<>(isDarkMode());
> 
> 
> and then access it with `AtomicReference::get` / `AtomicReference::set`.

Thanks for the suggestion. I have tried fixing it with another simple approach and updated the PR with that change. Spotbugs is also happy with that. Can you please have a look? If it doesn't look good, I will try this AtomicReference approach. Thanks in advance.

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

PR Review Comment: https://git.openjdk.org/jmc/pull/580#discussion_r1760741149


More information about the jmc-dev mailing list