Improvement: switch theme automatically#5530
Improvement: switch theme automatically#5530georgemac-labs wants to merge 1 commit intoportfolio-performance:masterfrom
Conversation
georgemac-labs
commented
Feb 27, 2026
- Add automatic theme switching while the app is running when the theme preference is set to System
- Use polling on macOS and SWT.Settings handling for non-macOS platforms
|
Hi maintainers, I use auto light/dark mode on macOS and noticed PP did not switch when other apps did, so this is my attempt to address that. Unfortunately, the cross-platform approach didn't work for me – I could only get it to work with a macOS specific implementation. However, I left the other approach in. If you think the PR makes sense in principle, maybe somebody could check it on Linux/Windows? |
- Add automatic theme switching while the app is running when the theme preference is set to System - Use polling on macOS and SWT.Settings handling for non-macOS platforms
ed62ffc to
8f2bb38
Compare
|
Just a couple notes: eclipse-chemclipse/chemclipse#1972 detects it based on background color - might be better than having the AppleInferfaceStyle hard coded (the text might change). dbeaver/dbeaver#21600 it is make sure that the right theme is picked at the start (not sure if that works for PP at the moment). And I found the feature request to ask SWT to support theme change on macOS: eclipse-platform/eclipse.platform.swt#1027 |
| return; | ||
|
|
||
| this.display = display; | ||
| Boolean initialSystemDarkTheme = detectMacOSDarkMode(); |
There was a problem hiding this comment.
This is on the UI thread. The other parts it is on the background thread.
| this.display = display; | ||
| Boolean initialSystemDarkTheme = detectMacOSDarkMode(); | ||
| this.lastSystemDarkTheme = initialSystemDarkTheme != null ? initialSystemDarkTheme.booleanValue() | ||
| : UIConstants.Theme.DARK.equals(themeEngineManager.getEngineForDisplay(display).getActiveTheme().getId()); |
There was a problem hiding this comment.
not sure if getActiveTheme is always non-null. Let's be defensive
| } | ||
| }; | ||
|
|
||
| this.display.timerExec(THEME_POLL_INTERVAL_MS, themePoller); |
There was a problem hiding this comment.
Why have to mechanisms? Could we ScheduledExecutorService.scheduleWithFixedDelay() instead?
|
To be honest, I am still not sure to run a check every 2 seconds only because some users have configured macOS to dynamically change. Therefore, I think we should guard this behind an experimental flag (see Experiments class). Then I think it is not helpful for future changes, that the code is not split into different classes - ThemeAddon for Linux and Windows, OSXStartupAddon for macOS. Can we refactor this have one class - maybe even a separate TrackSystemThemeChangeAddon or something that has the logic in one place. |
|
No objection to the feedback, but then I'd probably deprioritise this for now. It's only a very minor irritation and I thought it would be quick fix, but evidently it's not :) Maybe somebody else will feel like picking it up. |