Community
Participate
Working Groups
Created attachment 242758 [details] Screenshot showing tree with dark theme on windows 7. Currently having a dark theme on windows is almost unusable as the tree doesn't use the foreground color to show its arrows. Note: this should probably be done in SWT (i.e.: it should use the foreground color for that instead of using the system). I'm attaching a screenshot to see how it looks.
I've setup $150 bug bounty on FreedomSponsors: https://freedomsponsors.org/issue/666/dark-theme-tree-must-respect-the-foreground-color-for-items-arrows I hope to see this bug fixed.
See also the discussion in Bug 463620.
As a note, if you use the dark theme on LiClipse: http://www.liclipse.com it already works -- it's commercial, but still less than you offered as a bounty :) Regarding having it in Eclipse, I created a gist some time ago which could be used to integrate what I do in LiClipse in a theme in Eclipse itself: https://gist.github.com/fabioz/59b06c519f505d37fcfb -- the work in the Eclipse side should be trivial at this point: mostly add that code and some css to integrate that (and have a committer review and accept it)... Lars: do you think that approach could be integrated or is it missing something? (if it's not, I'll try to take a look on providing a patch to integrate it in the upcoming month).
(In reply to Fabio Zadrozny from comment #3) > Lars: do you think that approach could be integrated or is it missing > something? (if it's not, I'll try to take a look on providing a patch to > integrate it in the upcoming month). Sounds good as a solution as long as SWT does not provide this option. Please provide a Gerrit review for its integration into platform and I have a look. Please note that we are currently in M7 and after M7 we cannot do any feature development anymore.
(In reply to Fabio Zadrozny from comment #3) > As a note, if you use the dark theme on LiClipse: http://www.liclipse.com it > already works -- it's commercial, but still less than you offered as a > bounty :) I need this fix for Zend Studio - therefore I need it in the Eclipse Platform. Looks like this bounty will be easy money :-). Feel free to claim it if you have the patch accepted. Just to remind the "Definition of done" I posted on FreedomSponsors: - Create a patch in the respective open source project (Eclipse Platform, SWT, or wherever necessary) - The patch should be accepted in the master branch of the code repository of the respective open source project - The patch should be available at least in a nightly build of the Eclipse Platform for testing - The solution should work out-of-the-box: switching the Eclipse IDE to the dark theme should change the color of the expand/collapse toggle to something adequate level of the gray color (e.g. #A5A5A5) - There should be no regressions caused by the solution, e.g. negative effects on toggle on Linux and Mac OS X.
(In reply to Lars Vogel from comment #4) > (In reply to Fabio Zadrozny from comment #3) > > Lars: do you think that approach could be integrated or is it missing > > something? (if it's not, I'll try to take a look on providing a patch to > > integrate it in the upcoming month). > > Sounds good as a solution as long as SWT does not provide this option. > Please provide a Gerrit review for its integration into platform and I have > a look. Please note that we are currently in M7 and after M7 we cannot do > any feature development anymore. Ok, will keep that in mind.
I saw this mentioned on Twitter earlier today courtesy of @kaloyanraev, and as I had to do something similar within a Visual Studio plug-in a while back (see http://www.riverblade.co.uk/blog.php?archive=2012_08_01_archive.xml#2012080202) he asked me to post something. The trick is to adjust the luminance of the foreground colour to maintain sufficient contrast between the foreground and background, while keeping the saturation the same.. This is the function used (in C++ with Win32 types, but I imagine it will be easy enough for someone to port): /// \brief Adjust the given text colour to provide a reasonable contrast with the given background colour by fiddling with its luminance. /// COLORREF GetAdjustedTextColour(COLORREF const clrText, COLORREF const clrBackground) { float const fTextLuminance = CColour(clrText).GetLuminance(); float const fBackgroundLuminance = CColour(clrBackground).GetLuminance(); float const fLuminanceDiff = 0.5f; if (::fabs(fTextLuminance - fBackgroundLuminance) < fLuminanceDiff) { float const nNewLuminance = (fBackgroundLuminance > 0.5f) ? (fBackgroundLuminance - fLuminanceDiff) : (fBackgroundLuminance + fLuminanceDiff); CColour clrAdjusted(clrText); clrAdjusted.SetLuminance(nNewLuminance); return clrAdjusted; } return clrText; } The only dependency is a HSL/RGB colour class which can get/set the luminance for a given colour. Hope that helps, at least a little.
Windows specific by the looks?
That example is (it's from Win32 C++ code used in a Visual Studio plug-in), but the algorithm is generic. You just need to adjust the types/ranges etc.
(In reply to Anna-Jayne Metcalfe from comment #9) > That example is (it's from Win32 C++ code used in a Visual Studio plug-in), > but the algorithm is generic. You just need to adjust the types/ranges etc. I mean the issue seems to only occur on windows..
Ok, just pushed an initial attempt to gerrit :) https://git.eclipse.org/r/#/c/45169/ I've applied the fix only to windows as on other platforms it's usually not really a problem (I guess it could be a problem in Linux if the native tree is used and the system has a light based theme -- but usually in this case the system itself allows changing the configuration, so, it's not such a big problem as on windows where that's not changeable).
The problem is specific for Windows. I haven't noticed any similar issue on Linux (Ubuntu) or Mac OS X.
New review on gerrit: https://git.eclipse.org/r/#/c/45175/
Created attachment 252154 [details] Screenshot of proposed solution Great job, Fabio! I fetched your patched in a dev env and I can confirm that it works. See the attached screenshot. One question. As seen on the screenshot, the toggle has the old Windows XP style. Is it possible to draw it with the triangle style used on Windows 7 and later? This would keep the native look and feel of Eclipse. If you switch your Windows to the High Contrast theme you would see in Windows Explorer how the triangle toggle should look in a dark theme.
Added a new version which has the option to draw as triangles: https://git.eclipse.org/r/#/c/45287/
Created attachment 252161 [details] Screenshot of proposed solution with triangles Amazing! Screenshot attached. BTW, just a Gerrit hint. If you amend your previous commit in EGit then, when you push to Gerrit, the change will appear as a new patch set in the existing Gerrit change instead of a new Gerrit change.
New Gerrit change created: https://git.eclipse.org/r/45295
Hopefully https://git.eclipse.org/r/#/c/45295/ is now final (rebased with the current master and with proper commit message, supporting a custom foreground color and with a mode to draw with triangles or a square with +/-). In a previous review Andrey Loskutov noted that it needs PMC approval to be included, how can that be attained?
Created attachment 252165 [details] Checkbox Tree issue I did a little bit more testing and found an issue with checkbox trees (see the attachement). The overlay toggle is drawn over the checkbox instead over the native toggle. I guess some calculations need to be adjusted in the case of checkbox tree.
Just updated https://git.eclipse.org/r/#/c/45295/ to fix the issue in a tree with the check style.
Created attachment 252170 [details] Checkbox Tree issue - solved Thank you! Looks good now.
(In reply to Eclipse Genie from comment #17) > New Gerrit change created: https://git.eclipse.org/r/45295 Bug was reported on SWT but got a fix in UI code.. assigning to Platform-UI
FYI - The CSS engine is not API, so PMC approval is not required for this change. I currently do not have a windows system to test the change but Kaloyans screenshot looks great. I wait a bit if Brian has more feedback for https://git.eclipse.org/r/#/c/45295/ but IMHO this can and should be merged for M7.
Gerrit change https://git.eclipse.org/r/45295 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=e2794114b3d4b95a9fd5cf05cf5852f9e654501c
Tweaked and committed. Thanks Fabio. As an aside, I tested on OS X and it seems the erase doesn't remove the platform's tree arrows.
I think this change added a undesired message to the command line. Created Bug 465668 for that.
