Community
Participate
Working Groups
A common complain is that scrollbars cannot be styled in SWT. This make Eclipse looking bad if the IDE is styled by a dark theme.
At the moment we're focused on gtk3 bug-fixes, but we would be happy to review patches.
I think this issue is most important for the Windows platform, as Windows constantly shows the scrollbars -> Changing the platform field in bugzilla to Windows.
New Gerrit change created: https://git.eclipse.org/r/136091
After researching the problem, I found that overriding scrollbars is tedious. However, on newest Win10 with Dark Windows Explorer (since build 1809) it's rather simple with an undocumented theme string. I implemented new methods that allow to use it, for Win10 only. One should check if dark theme is available. It's sufficient to do this once: Display.isSystemThemeAvailable(SWT.THEME_SYSTEMUI_DARK) If yes, then it can be set for desired controls: Control.setSystemTheme(SWT.THEME_SYSTEMUI_DARK);
Note: Activating dark scrollbars also changes the color of expand buttons in Tree. To my view, it actually improves it.
Alexandr, could you upload a screenshot?
Shouldn't native dark theme support be a separate issue? I doubt it would mesh well with SWT's ad-hoc color overrides. Also, native dark theme on Windows is very new, undocumented, probably unstable and affects more than just scrollbars. And it's not just new theme classes, a couple of new functions were added to Uxtheme.
Can't make a screenshot until Monday. For our application it seems to work just well with our own color overrides. Unfortunately, I can't ask for dark scrollbars alone, and have to theme entire control - but, again, it looks well. Yes, dark theme support involves more then just new themes, yet I don't think this is important. In this just, I only use the new theme, it should be relatively safe since I'm using all the old API's with just the new theme name.
Also, my patch doesn't do anything on its own, you have to explicitly request the control to be skinned with new theme - hence, the risk should be very low.
Created attachment 277456 [details] Our application before the suggested patch
Created attachment 277457 [details] Our application with the suggested patch
I have created the screenshots now. I guess they are pretty convincing :) But yes, to reiterate, there are some downsides: 1) It's only available since Win10 build 1809 2) The scrollbar colors can't be selected, it's just fixed dark or default.
On the GTK side of things we do this internally. There is an internal function in OS.java called "setDarkThemePreferred" which is called by Platform UI somewhere. This function will trigger the OS to use the dark theme colors. Maybe this approach could be used for Windows as well? IMO adding API relating to theme isn't a good idea as theming varies wildly from platform to platform.
(In reply to Eric Williams from comment #13) > On the GTK side of things we do this internally. There is an internal > function in OS.java called "setDarkThemePreferred" which is called by > Platform UI somewhere. This function will trigger the OS to use the dark > theme colors. > > Maybe this approach could be used for Windows as well? IMO adding API > relating to theme isn't a good idea as theming varies wildly from platform > to platform. +1 for this. Similar work is currently done for Mac. See Bug 544039 or Gerrit https://git.eclipse.org/r/#/c/136217/ Alexandr, WDYT about this approach? Bye the way many thanks for this work, this will make many dark Eclipse users happy.
Fabio, if you still interested in better dark Eclipse support for Windows, you may want to get involved here.
This patch solves our problem, and making it public is not a priority for us. If you're going to actually accept the patch with 'OS.setDarkThemePreferred' approach, then _maybe_ I can change it after a couple weeks. Still, I'm not sure that 'OS.setDarkThemePreferred' approach is actually a good idea: if I understand it correctly, without uniform API users would typically have to use reflection to call the method. Also, it removes granular control: either you have everything themed, or you don't. Theming some controls could cause undesired changes.
On the other hand, theming support on different platforms is indeed quite different. Maybe it's indeed reasonable to just have platform-specific methods that are also different?
(In reply to Alexandr Miloslavskiy from comment #16) > This patch solves our problem, and making it public is not a priority for > us. If you're going to actually accept the patch with > 'OS.setDarkThemePreferred' approach, then _maybe_ I can change it after a > couple weeks. > > Still, I'm not sure that 'OS.setDarkThemePreferred' approach is actually a > good idea: if I understand it correctly, without uniform API users would > typically have to use reflection to call the method. Also, it removes > granular control: either you have everything themed, or you don't. Theming > some controls could cause undesired changes. SWT should not have theming API as there is no such concept from SWT's POV. So public API for this type of thing is a no-go IMO. The platform differences are too great to make this feasible. Another issue: SWT has color constants, like COLOR_WIDGET_BACKGROUND, for example. If we are styling each Control based on a theme, where does the "default" color come from? The theme set for that Control or the system theme? What does display.getSystemColor(COLOR_WIDGET_BACKGROUND) return? There also all sorts of checks that do control.getBackground() != COLOR_WIDGET_BACKGROUND which could break in this instance. On GTK we handle this by theming all widgets, and even scraping the system colors according to the theme. This way we provide theming support for dark themes without creating fragmented colors/styling. We support an environment variable that allows users to change the theme for SWT, but that is the theme for *every* Control/Widget.
Moving out of 4.11
(In reply to Alexandr Miloslavskiy from comment #16) > This patch solves our problem, and making it public is not a priority for > us. If you're going to actually accept the patch with > 'OS.setDarkThemePreferred' approach, then _maybe_ I can change it after a > couple weeks. SWT GTK uses OS.setDarkThemePreferred and I think also SWT Mac, so I think this approach is fine for SWT Win.
Mac uses the same approach: https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=d9cbf6812bd7c17517ae96f6d834539c211fd8c2 Alexandr, could you also support this approach in SWT Win? This way we could easily trigger the setting for the IDE.
It's somewhat more complicated on Windows at the moment: 1) Instead of setting a global flag, theme is applied to individual controls. 2) Dark scrollbars is a non-documented parameter for a documented API SetWindowTheme() 3) Enabling dark scrollbars also applies other bits of the theme to the affected control, such as changing Tree's expander button (the change improves it, though). This said, yes, it's possible to convert the patch to 'OS.setDarkThemePreferred' approach, but it will require more changes (adding code to every widget that can benefit from it). If you're good with it, I can alter the patch.
There definitely should public API be used. Is OS considered public API? But this would be beyond this ticket. Alex, maybe one could set the preferred dark theme for the display and then if a scrollbar is created it will use this value?
On Windows, many controls have embedded scrollbars that are not true controls. This is why I will have to identify and support every specific Widget that could have scrollbars in it.
(In reply to Alexandr Miloslavskiy from comment #24) > On Windows, many controls have embedded scrollbars that are not true > controls. This is why I will have to identify and support every specific > Widget that could have scrollbars in it. Would it not be sufficient to put a method in Scrollable and then override it on widget-by-widget basis if needed? (In reply to Thomas Singer from comment #23) > There definitely should public API be used. Is OS considered public API? But > this would be beyond this ticket. > > Alex, maybe one could set the preferred dark theme for the display and then > if a scrollbar is created it will use this value? OS is internal, and should stay that way. As for the other idea...does Windows have some sort of environment variable like GTK_THEME on Linux?
> Would it not be sufficient to put a method in Scrollable and then override > it on widget-by-widget basis if needed? I'm not sure what is the question here. Once API and drawbacks are agreed upon, I will be able to implement that.
The other option is to wait. For the last two major Win10 updates Microsoft has been working on dark theme, also making my patches possible. Maybe they will create a public API in the next major update (the one that comes after may-2019 update which doesn't have it yet)
The SWT API discussion seems to happen in Bug 546859. Alexandr and Thomas, please have a look.
Moving out to 4.13
Hi Alexandr, Tested dark scrollbar with Tree/Table class on Win10, looks neat. Suggest we should rework this patch to use the new Display#isSystemDarkTheme() API. Also we can get rid of the Cocoa/GTK side of changes. Thanks!
Created attachment 279621 [details] DarkTheme_Win10_Controls_Bugs (In reply to Niraj Modi from comment #30) > Hi Alexandr, > Tested dark scrollbar with Tree/Table class on Win10, looks neat. Suggest we > should rework this patch to use the new Display#isSystemDarkTheme() API. > > Also we can get rid of the Cocoa/GTK side of changes. Thanks! Tested the current patch further. Below are my observations: 1. Scrollbars in all widget has turned dark, so that's the good part. 2. Dark theme PUSH Button as also seen. 3. CCombo seen with dark background.[It has buggy behavior] 4. CoolBar also having dark background at one place.[looks buggy] Attaching a screen shot for of problem seen.
Moving out of 4.13
*** Bug 541359 has been marked as a duplicate of this bug. ***
I do not see any problems with Combo in SWT ControlExample. Maybe it's a problem in your testing snippet? I confirm that CoolBar's embedded Text doesn't look good. As a workaround, my patch can leave CoolBar untouched for now. In fact, I'd even skip any controls except Button and those with scrollbars. If I rework the patch to depend on a system property, would you support that patch to be merged?
(In reply to Alexandr Miloslavskiy from comment #34) > I do not see any problems with Combo in SWT ControlExample. Maybe it's a > problem in your testing snippet? I think the screenshot is not showing the default Combo, but the CCombo and that one contains an Text control, so it's most likely the same issue as the Collbar's Text control. > I confirm that CoolBar's embedded Text doesn't look good. > As a workaround, my patch can leave CoolBar untouched for now. > In fact, I'd even skip any controls except Button and those with scrollbars. > > If I rework the patch to depend on a system property, would you support that > patch to be merged? I also had a look at your patch today and investigated the Text issue. Windows applications behave similar for Text controls: They are drawn white on black if they don't have the focus, and black on white if they have the focus. Unfortunately without any styling the SWT Text control only changes its background upon focus change, but not the forground color, so without focus you cannot see the black text on black bachground. I tried to avoid this behavior by hooking into various WM_* messages, but this ends up in a lot of custom drawing again. So unless you find the magic constant to also toggle the foreground color, I would prefer to just ignore the "isSystemDarkTheme" property for Text controls. They are styled in the IDE anyways.
I hope to find time and rework the patch in the next couple weeks.
(In reply to Alexandr Miloslavskiy from comment #34) > I do not see any problems with Combo in SWT ControlExample. Maybe it's a > problem in your testing snippet? It's CCombo not Combo, as per attachment 279621 [details]: On opening it the dark background goes away. > I confirm that CoolBar's embedded Text doesn't look good. > As a workaround, my patch can leave CoolBar untouched for now. Makes sense, not to touch any widget other than Scrollbar. > In fact, I'd even skip any controls except Button and those with scrollbars. Please check if we can/should also skip the Button part as well(we already have some custom themeing support for buttons currently) and IMO this bug should target Scrollbar as of now. > If I rework the patch to depend on a system property, would you support that > patch to be merged? +1 for system property based approach. Thanks!
(In reply to Alexandr Miloslavskiy from comment #36) > I hope to find time and rework the patch in the next couple weeks. Any update here, Alexander? A themeable scrollbar would be highly appreciated by our Eclipse IDE Dark theme users.
New tasks appeared (as it usually happens) and this is now somewhere in the middle of my TODO list. Not a high priority issue for us, because we simply use the first version of the patch (seen in gerrit). If someone if willing to rework the patch into system property approach, please do it.
*** Bug 497310 has been marked as a duplicate of this bug. ***
(In reply to Alexandr Miloslavskiy from comment #39) > New tasks appeared (as it usually happens) and this is now somewhere in the > middle of my TODO list. Do you think you can have a look for 4.15.
Conrad is currently working on a patch, let's see if he finishes it.
(In reply to Alexandr Miloslavskiy from comment #42) > Conrad is currently working on a patch, let's see if he finishes it. Great to hear!
Hello, I was test this solution on my application, using SWT 4.14 on Windows 10. But I notice that when using the "DARKMODE_EXPLORER" on the Table class some other things happend: * The visual styles of the Explorer Theme were not enable like the soft color of item selection. * There is no "hover" effect on the table itens. So, doing some digging, I found a post and they notice the same side effect as mine. On the post, they mention an alternative to "SetWindowTheme". It is: "AllowDarkModeForWindow" But, I did not test it because I do not know how to call this from Java. The post is: https://stackoverflow.com/questions/53501268/win10-dark-theme-how-to-use-in-winapi
Created attachment 281570 [details] Table with Explorer Theme Se the selected item with a soft color.
Created attachment 281571 [details] Table with DarkTheme Now se the difference on the selection color.
Moving to 4.16
(In reply to Jose Renato from comment #44) > On the post, they mention an alternative to "SetWindowTheme". It is: > "AllowDarkModeForWindow" > > But, I did not test it because I do not know how to call this from Java. > > The post is: > https://stackoverflow.com/questions/53501268/win10-dark-theme-how-to-use-in- > winapi Hi Alexandr, Did you got a chance to try the new alternative API "AllowDarkModeForWindow" or will you be interested in exploring this ?
`AllowDarkModeForWindow()` is currently not exported directly. To get access to it, API needs to be loaded by ordinal (internal identifier) which is super unreliable. The approach that I used (`SetWindowTheme(hwnd, "DarkMode_Explorer")`) leads to same effect, but with a more documented invocation.
(In reply to Alexandr Miloslavskiy from comment #49) > `AllowDarkModeForWindow()` is currently not exported directly. To get access > to it, API needs to be loaded by ordinal (internal identifier) which is > super unreliable. > > The approach that I used (`SetWindowTheme(hwnd, "DarkMode_Explorer")`) leads > to same effect, but with a more documented invocation. Hi Alexandr. Did you notice this side effects when using DARKMODE_EXPLORER? * The visual styles of the Explorer Theme were not enable like the soft color of item selection. * There is no "hover" effect on the table itens.
As mentioned by Comment 5, Comment 8 - yes, the theme of control changes instead of only changing scrollbars, which involves multiple changes. Yet they look well for us, so I don't mind it.
(In reply to Alexandr Miloslavskiy from comment #49) > `AllowDarkModeForWindow()` is currently not exported directly. To get access > to it, API needs to be loaded by ordinal (internal identifier) which is > super unreliable. Do you have that piece of code handy & share as a gerrit for us to try/experiment ? Thanks!
I'm no longer interested in discussions, unless there's a very specific plan on merging the patch, which have been lying there over a *year*, during which I've had more then enough fruitless discussions. The patch works well for us in our fork. If you don't want it, it's your choice.
Gerrit change https://git.eclipse.org/r/136091 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=235c551e987a8901900fccf6cc362cf3f0529bde
In master now. Thanks Conrad for polishing the patch.
(In reply to Alexandr Miloslavskiy from comment #55) > In master now. > Thanks Conrad for polishing the patch. Thanks Alexandr/Conrad for the coordinated efforts.
(In reply to Niraj Modi from comment #56) > (In reply to Alexandr Miloslavskiy from comment #55) > > In master now. > > Thanks Conrad for polishing the patch. > > Thanks Alexandr/Conrad for the coordinated efforts. Many, many thanks Alexandr/Conrad for finishing this development. Can you please add to the N&N. Also please help me in Bug 561588 to activate this for the Windows Dark Theme.
(In reply to Niraj Modi from comment #37) > (In reply to Alexandr Miloslavskiy from comment #34) > > If I rework the patch to depend on a system property, would you support that > > patch to be merged? > +1 for system property based approach. Thanks! In the various iterations of the patches, we lost track of the System Property approach cannot be bypassed. Thanks to Lakshmi for pointing this out. See with System Property based approach user/client can turn 'ON' & 'OFF' this experimental feature without having to modify the code. I suggest we switch back to System Property approach instead of Display.setData() which is not inline with the current SWT approach as well. I will share a patch shortly for discussion. Thanks!
New Gerrit change created: https://git.eclipse.org/r/160236
Gerrit change https://git.eclipse.org/r/160236 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=80282852cebf1c14cc243fad3959f98711982fa3
(In reply to Eclipse Genie from comment #60) > Gerrit change https://git.eclipse.org/r/160236 was merged to [master]. > Commit: > http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/ > ?id=80282852cebf1c14cc243fad3959f98711982fa3 With above patch the enabling of this feature is controlled via below System Property(when done from the Eclipse VM args options): -Dorg.eclipse.swt.internal.win32.enableDarkScrollbars=true Using Code: System.setProperty("org.eclipse.swt.internal.win32.enableDarkScrollbars", Boolean.TRUE.toString());
I assume this is fixed. Please reopen if that is a wrong assumption
(In reply to Lars Vogel from comment #57) > (In reply to Niraj Modi from comment #56) > > (In reply to Alexandr Miloslavskiy from comment #55) > > > In master now. > > > Thanks Conrad for polishing the patch. > > > > Thanks Alexandr/Conrad for the coordinated efforts. > > Many, many thanks Alexandr/Conrad for finishing this development. Can you > please add to the N&N. N&N still pending.
(In reply to Niraj Modi from comment #63) > (In reply to Lars Vogel from comment #57) > > (In reply to Niraj Modi from comment #56) > > > (In reply to Alexandr Miloslavskiy from comment #55) > > > > In master now. > > > > Thanks Conrad for polishing the patch. > > > > > > Thanks Alexandr/Conrad for the coordinated efforts. > > > > Many, many thanks Alexandr/Conrad for finishing this development. Can you > > please add to the N&N. > > N&N still pending. N&N should cover a brief of this feature and how user can activate it SWT using System property. Thanks!
Maybe someone who regularly writes N&N entries can do that. I don't want to mess up the N&N with my Paint skills.
Verified using Build id: I20200407-1800 on Win10. Will add the N&N shortly.
New Gerrit change created: https://git.eclipse.org/r/160659
Gerrit change https://git.eclipse.org/r/160659 was merged to [master]. Commit: http://git.eclipse.org/c/www.eclipse.org/eclipse/news.git/commit/?id=22819a6705a046312be3214281974bbcefa864b2
Not sure if this was already discussed above, shouldn't we get the dark scrollbars only if Eclipse is using the "Dark" theme in the Preferences? If my Windows 10 is using the dark theme, Eclipse is using the default "Light" theme in Preferences, and if this new property is set, I get dark scrollbars in light Eclipse.
(In reply to Noopur Gupta from comment #69) > Not sure if this was already discussed above, shouldn't we get the dark > scrollbars only if Eclipse is using the "Dark" theme in the Preferences? > > If my Windows 10 is using the dark theme, Eclipse is using the default > "Light" theme in Preferences, and if this new property is set, I get dark > scrollbars in light Eclipse. Styling in SWT is independent of the CSS theme in platform. Bug 561588 will handle the CSS theme.
New Gerrit change created: https://git.eclipse.org/r/161141
Gerrit change https://git.eclipse.org/r/161141 was merged to [master]. Commit: http://git.eclipse.org/c/www.eclipse.org/eclipse/news.git/commit/?id=65c8cfff8bf6d29ebf0811b81a9f573ec65faa0a
New Gerrit change created: https://git.eclipse.org/r/162977
(In reply to Eclipse Genie from comment #73) > New Gerrit change created: https://git.eclipse.org/r/162977 Above patch reworks the approach for scrollbar implementation that went in 4.16 M1, with below: ------------------------------------------------------------------------- New design goals are: 1) Feature can be enabled without reflection in cross-platform 2) Feature can be enabled in run time This is the most typical scenario. For example, Eclipse and the product I'm working on both select the theme after reading application config, which happens after creating `Display`. 3) Feature can be configured individually Different SWT-based products need different settings to match their dark themes best. 4) There's an emergency switch to turn it off without recompiling code This can be done using system property: org.eclipse.swt.internal.win32.disableCustomThemeTweaks 5) It's more performance-friendly Previous approach used system property, which due to (2) needed to be re-read every time SWT wanted to know if it's active. (4) would have caused two properties to be read. (6) also fits nicely. Additional changes: 6) Added code that tests whether theme is present. See comment in code. 7) Adjusted names to better indicate that setting has additional side effects and is not just about scrollbars.
(In reply to Niraj Modi from comment #74) > (In reply to Eclipse Genie from comment #73) > > New Gerrit change created: https://git.eclipse.org/r/162977 Anything we need to do in platform UI to keep it enabled for the EclipseDark theme?
> Anything we need to do in platform UI to keep it enabled for the EclipseDark > theme? Nothing needs to be changed in Eclipse at the moment. Old plugin from Bug 561588 will continue to work as is.
(In reply to Alexandr Miloslavskiy from comment #76) > > Anything we need to do in platform UI to keep it enabled for the EclipseDark > > theme? > > Nothing needs to be changed in Eclipse at the moment. > Old plugin from Bug 561588 will continue to work as is. Thanks.
New Gerrit change created: https://git.eclipse.org/r/163201
New Gerrit change created: https://git.eclipse.org/r/163225
Gerrit change https://git.eclipse.org/r/163225 was merged to [master]. Commit: http://git.eclipse.org/c/www.eclipse.org/eclipse/news.git/commit/?id=f4d6d6d390e16c7d6f8189616b3a76c4c44c9ab9
The N&N for this change has the wrong screenshot. It's a screenshot of the custom StyledText scrollbars.
Thanks Mike, I merged your update which you posted to another bug.
Details on how to disable this feature: On Windows 10 the dark theme tweaks, including the dark scrollbars can be disabled using the org.eclipse.swt.internal.win32.disableCustomThemeTweaks Java property. For Example. add this VM argument in eclipse.ini or on the command line after -vmargs: -Dorg.eclipse.swt.internal.win32.disableCustomThemeTweaks=true
Verified using Build id: I20200518-2220 on Win10.
The N&N is wrong for this enhancement, as I mentioned yesterday.
New Gerrit change created: https://git.eclipse.org/r/163316
Gerrit change https://git.eclipse.org/r/163316 was merged to [master]. Commit: http://git.eclipse.org/c/www.eclipse.org/eclipse/news.git/commit/?id=4aa8dbc77b13f7b0c5f180a7831c02d38aaed92b
how should this exactly work? in our ui plugin i do quickly: Display.getDefault().setData("org.eclipse.swt.internal.win32.useDarkModeExplorerTheme", true); but that has no effect (or also the system property that is talked about here) Also in "plain" Eclipse, how can we enable dark scrollbars there? (if you can only set i through code)
Refer to OS.setTheme() for current dark theme settings: https://github.com/eclipse-platform/eclipse.platform.swt/blob/master/bundles/org.eclipse.swt/Eclipse%20SWT%20PI/win32/org/eclipse/swt/internal/win32/OS.java#L2249 If you're on Win11 22H2, then you also need a recent patch: https://github.com/eclipse-platform/eclipse.platform.swt/pull/424 In Eclipse, you can see it in action by choosing Dark theme in settings.
ah ok, then i guess i need to have that patch Because if in my Eclipse Version: 2022-09 (4.25) if i turn on dark mode the scrollbars are not dark (but the rest is as far as i can see)