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

[Brice Dutheil]

On Mon, 26 Aug 2024 10:20:20 GMT, Suchita Chaturvedi <schaturvedi at openjdk.org> wrote:

> This PR is to fix the accessibility issue for High Contrast mode. Stracktrace View was not readable at all in dark mode.
> 
> Before the fix:
> <img width="959" alt="image" src="https://github.com/user-attachments/assets/f12f1b7d-aa8a-4cce-8e0c-7731d5f9cc96">
> 
> After the fix:
> <img width="959" alt="image" src="https://github.com/user-attachments/assets/8f7be98a-206b-4758-a1c2-8971d7ab1c55">

application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/views/stacktrace/StacktraceView.java line 212:

> 210: 
> 211: 	private static final String HELP_CONTEXT_ID = FlightRecorderUI.PLUGIN_ID + ".StacktraceView"; //$NON-NLS-1$
> 212: 	private static final Boolean IS_DARK_MODE = ThemeUtils.isDarkMode();

**question:** I would avoid the static final there, I think the themes can be dynamic, i.e. change without restarting ? If that's not the case maybe it can be in the future.

See the the comment on `ThemeUtils` to see a different approach.

application/org.openjdk.jmc.ui.common/src/main/java/org/openjdk/jmc/ui/common/util/ThemeUtils.java line 8:

> 6: import org.eclipse.ui.PlatformUI;
> 7: 
> 8: public class ThemeUtils {

**suggestion:** I believe the themes can change at runtime. `isDarkMode` will compute this value always. But to avoid the computation on each call, you could store a variable here and update this value each time the theme changes.

Something like:


public class ThemeUtils {
  private static boolean isCurrentThemeDark = isDarkMode();

  static {
    PlatformUI.getWorkbench().getThemeManager().addPropertyChangeListener(() -> isCurrentThemeDark = isDarkMode())
  }

  public static boolean isDarkThemeTheme() { return isCurrentThemeDark; }
}


This way the value is updated anytime the theme changes. I'm not quite sure when to register the listener, I put it in static block as an example, but maybe this is not recommended in the Eclipse RCP programing idioms.

Note I kept the original name of `isDarkMode`, but feel free to rename to anything meaningful in your implementation.

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

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