7056: Fix dark mode

[bric3]

This change allows a user of the JMC RCP application to switch the appearance with the default Eclipse themes.

Those are shipped via the org.eclipse.ui.themes and org.eclipse.e4.ui.css.swt.theme plugin. In order to activate them all that is is required is to add the above in the application feature file, and to declare them in the application manifest file.

After

Before There's no theme, as such the combo box doesn't open either.

---

Progress

• [x] Commit message must refer to an issue
• [ ] Change must be properly reviewed

Issue

• JMC-7056: Fix dark mode

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jmc pull/362/head:pull/362
$ git checkout pull/362

Update a local copy of the PR:
$ git checkout pull/362
$ git pull https://git.openjdk.java.net/jmc pull/362/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 362

View PR using the GUI difftool:
$ git pr show -t 362

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jmc/pull/362.diff

[bridgekeeper[bot]]

:wave: Welcome back bric3! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[bric3]

Note this PR doesn't fix the graph issues mentioned in JMC-7056.

For JMC-7056, I'm not quite sure how to fix that yet, but using the following APIs might prove useful:

• PlatformUI.getWorkbench().getThemeManager().getCurrentTheme() gets an ITheme
• theme.getColorRegistry().get(String) gets the Color or a symbolic name

[thegreystone]

I think we may have skipped having dark mode available due to the problems mentioned in JMC-7056. I think we need to fix JMC-7065 to re-enable.

[bric3]

It makes sense, let's fix those.

[mlbridge[bot]]

Webrevs

• 03: Full - Incremental (91043de2)
• 02: Full - Incremental (fa23f216)
• 01: Full - Incremental (1c6eb2ba)
• 00: Full (73051261)

[ascopes]

Looks like a temporary workaround for this is to (at least for GTK) set the GTK_THEME environment variable before starting jmc.

GTK_THEME="Adwaita:light" jmc &
GTK_THEME="Adwaita:dark" jmc &

[bric3]

@aptmac I can see the side of this argument. Would you like me to re-name this PR and the associated ticket ? And then make a census of the "broken" items ?

Also maybe you have some insights to the SWT / Eclipse APIs to look for, in order to fix the broken components as in this https://github.com/openjdk/jmc/pull/362#issuecomment-1013900500

[aptmac]

@aptmac I can see the side of this argument. Would you like me to re-name this PR and the associated ticket ? And then make a census of the "broken" items ?

Yeah I think that would be the right way to go.

Also maybe you have some insights to the SWT / Eclipse APIs to look for, in order to fix the broken components as in this #362 (comment)

I don't have anything off the top of my head. Thinking back I recall a lot of the backgrounds and colours being set manually within components in JMC, that's where my first search would go. Maybe for charts we could have some class/toolkit that can return standardized background/foreground/component colours, and these colour values change based on whether JMC is in light or dark mode?

[bric3]

standardized background/foreground/component colours

I also think defining palette of colors might be what to look for.

However there are some challenges for "embedded app" like the JMX browser. Maybe applying a LaF is enough.

[thegreystone]

Hi @bric3! We're getting closer to shipping JMC 9. Is this something that you still want to get into JMC 9?

[bric3]

@thegreystone Don't wait on this one I think it can go in a later version

[thegreystone]

Hi @bric3! Let's just take it and open issues on anything that doesn't work, otherwise this will stay in draft forever.

[bric3]

OK I'll update the PR tomorrow morning.

[openjdk[bot]]

@bric3 This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

7056: Fix dark mode

Reviewed-by: aptmac

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 1 new commit pushed to the master branch:

• 793e8263a338a0251854510c8eb1a4bae3c3acc6: 8054: Support the new JPLIS agent events

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch. As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@aptmac) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

[aptmac]

This will need a copyright bump to 2024 and can be integrated afterwards.

[bric3]

@aptmac It think there's an issue in the script, or is is required to update copyrights on master?

[aptmac]

@aptmac It think there's an issue in the script, or is is required to update copyrights on master?

Ah that's no good, I'll put this on my todo list of things to take a look. I'm wondering if the script didn't like the merge commit. At first glance it looks like it's checking the files that were updated in the rjmx & flightrecorder configuration shuffle. Maybe because the original commit for this PR was on an older master, when the merge commit occured to update this PR the license check script might have compared the new master to what master looked like when this PR was opened.

It's pretty easy to verify the two files on this PR, I think this should be okay and we ignore the results of the copyright script.

[bric3]

/integrate

[openjdk[bot]]

@bric3 Your change (at version 91043de2cf57efeac5202a228134c94d5202070a) is now ready to be sponsored by a Committer.

[aptmac]

/sponsor

[openjdk[bot]]

Going to push as commit b61f6a798d63f2ba4616fa734440d170e2ed06ad. Since your change was applied there has been 1 commit pushed to the master branch:

• 793e8263a338a0251854510c8eb1a4bae3c3acc6: 8054: Support the new JPLIS agent events

Your commit was automatically rebased without conflicts.

[openjdk[bot]]

@aptmac @bric3 Pushed as commit b61f6a798d63f2ba4616fa734440d170e2ed06ad.

:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.