Community
Participate
Working Groups
+++ This bug was initially created as a clone of Bug #23837 +++ swt 20020918 1. Run the ControlExample 2. In the Button Tab 3. Change the Foreground color and the Background Color Notice that the button does not change. This works on Motif and GTK. ------------------------------------------------------------- With fix for bug 23837, this also works on Windows now via custom paint operation. Purpose of this bug is to investigate possibility of similar implementation on MAC..
Adding Till, as he is working on the styling for table and tree headers on Mac. @Till, interested in fixing this one too?
New Gerrit change created: https://git.eclipse.org/r/95214
New Gerrit change created: https://git.eclipse.org/r/95215
(In reply to Eclipse Genie from comment #2) > New Gerrit change created: https://git.eclipse.org/r/95214 Patch. I think the result is quite useable. Probably still needs some cleanups (e.g. I copied and adjusted the getDifferentColor-code in Control.java from the Windows patch in bug 23837, but I think the comments should probably be updated.) I'm not 100% convinced of my solution for the default button (I just changed to border size to 2), but didn't have any idea that looked better and works for any background color. (In reply to Eclipse Genie from comment #3) > New Gerrit change created: https://git.eclipse.org/r/95215 For testing: Patch for the mac dark theme. I've also tested with ControlExample.
Patch set 2 contains cleanups, simplifications (e.g. using bezierPathWithRoundedRect) and refinements (e.g. handle SWT.FLAT, make sure gradient for non-flat button is with darker color at bottom, adjust rect size by width of drawn border)
Thanks for contributing the patch. I currently have a few other bugs on my plate for M7. I'll try to review this for M7, but not sure if I can look at this immediately.
(In reply to Lakshmi Shanmugam from comment #6) > Thanks for contributing the patch. > I currently have a few other bugs on my plate for M7. I'll try to review > this for M7, but not sure if I can look at this immediately. Sorry, I was unable to look at it for M7 due to other items. Moving it to 4.8.
(In reply to Lakshmi Shanmugam from comment #7) > Sorry, I was unable to look at it for M7 due to other items. Moving it to > 4.8. Could you target 4.7.1? This is really an import requirement for us in Platform UI.
I came across this limitation myself, and I would be happy to test the patches on my system, however I don't know how to build the native code. The "build.sh" script doesn't seem to do this, neither does "build.xml". The documentation I could find seems out outdated. @Till: could you give me some hints? Thanks!
(In reply to Olivier Croquette from comment #9) > I came across this limitation myself, and I would be happy to test the > patches on my system, however I don't know how to build the native code. The > "build.sh" script doesn't seem to do this, neither does "build.xml". The > documentation I could find seems out outdated. @Till: could you give me some > hints? Thanks! The build system is based on maven now. You may first want to remove the xulrunner dependency by also applying Gunnar Wagenknecht's patch from https://git.eclipse.org/r/#/c/83418/ for bug 506092. Then, do: cd eclipse.platform.swt.binaries/bundles/org.eclipse.swt.cocoa.macosx.x86_64/ mvn clean package -Dnative=cocoa.macosx.x86_64 -Pbuild-individual-bundles
Hi Till, thanks to your help, I can now build the native widgets (I found it quite surprising that one needs to start the build from the "binaries" repository :) ). The buttons look good. I am not sure how important this is, but I noticed the following differences compared to the current buttons: - they are a bit narrower - they have hard borders, whereas the buttons before the change seems to have some anti-aliasing See also the screenshot (scaled up 8 times) @Lars: FYI, some widgets still don't look nice with the dark theme, for instance the comboboxes and some text inputs. See the other screenshot.
Created attachment 268743 [details] Comparison
Created attachment 268744 [details] Combobox and co.
(In reply to Lars Vogel from comment #8) > Could you target 4.7.1? This is really an import requirement for us in > Platform UI. Can this the patch be reviewed and merged early for M1 so that can have good testing before the downport?
Hi Till, I tested the patch and it sets the background color for Button as expected. I noticed a few problems when running the ControlExample :- 1. When bg color is set, the focus ring is not visible anymore for PUSH & TOGGLE buttons. For RADIO & CHECK styles it's drawn but not as visible as before. 2. TOGGLE style buttons can't be differentiated when toggled, as before. 3. The BORDER is drawn as 2 pixels wide, earlier was 1 pixel. Also, I'm not sure about the border color. 4. As noted in comment#11, border anti-aliasing is missing.
Created attachment 269369 [details] Focus ring
(In reply to Lakshmi Shanmugam from comment #15) > 1. When bg color is set, the focus ring is not visible anymore for PUSH & > TOGGLE buttons. For RADIO & CHECK styles it's drawn but not as visible as > before. Hmm, do you mean the focus ring that you can get by using "tab" to select the button (if "all controls" is selected in system preferences > keyboard > shortcuts )? I *can* see it for PUSH & TOGGLE (on mac os 10.12.5), see screen shot > 2. TOGGLE style buttons can't be differentiated when toggled, as before. Fixed. (As before? Without this patch active, when toggled, it is blue for me, like when it is being clicked) > 3. The BORDER is drawn as 2 pixels wide, earlier was 1 pixel. Also, I'm not > sure about the border color. I've looked again: For flat buttons the border is originally 1pt (=2pixels on a retina screen), like implemented. For round buttons it seems to be 0.75 pt (=1 pixel plus some anti-aliased pixels on a retina screen). I've adjusted the patch. I'm not sure about the border colors (or the gradients) either. If you have a better idea, just go head :-) BTW., Apple seems to use bitmaps to achieve the look of the rounded buttons, but they know the possible color value (blue or gray theme) - we have to draw something reasonable for an arbitrary color choice. > 4. As noted in comment#11, border anti-aliasing is missing. See 3.
(In reply to Till Brychcy from comment #17) > (In reply to Lakshmi Shanmugam from comment #15) > > 1. When bg color is set, the focus ring is not visible anymore for PUSH & > > TOGGLE buttons. For RADIO & CHECK styles it's drawn but not as visible as > > before. > > Hmm, do you mean the focus ring that you can get by using "tab" to select > the button (if "all controls" is selected in system preferences > keyboard > > shortcuts )? > I *can* see it for PUSH & TOGGLE (on mac os 10.12.5), see screen shot > It's strange that I can see the focus ring without the patch, but can't see it with the patch applied. Trying to figure out the reason...
I'm changing the target to 4.8M1, we can think about setting it to 4.7.1 *after* it is on the master
I'm unable to resolve 2 issues with the patch: 1. The focus ring is not shown for push & toggle buttons anymore, with the patch. I tried on a different machine which has a fresh install of 10.12 and it worked there. But, unable to get it to work on my machine (10.12.6). 2. For the toggled button, only the Text color changes to white, but no change in background color is visible. So, toggled state for buttons with only images is not visible anymore. Also, I noticed that the gradient colors on the button are not close to color selected by the user. Maybe we can change it?
(In reply to Lakshmi Shanmugam from comment #20) > I'm unable to resolve 2 issues with the patch: > > 1. The focus ring is not shown for push & toggle buttons anymore, with the > patch. > I tried on a different machine which has a fresh install of 10.12 and it > worked there. But, unable to get it to work on my machine (10.12.6). Strange. Always, or only if the background is set? Hmm maybe something is configured differently in the system settings? Does ist appear/disappear if you toggle the tabbing-behaviour with ctrl-f7 and then use "tab" to switch focus? > > 2. For the toggled button, only the Text color changes to white, but no > change in background color is visible. So, toggled state for buttons with > only images is not visible anymore. That is strange, too, because as written in comment 17, i specifically fixed that (see the change to isHighlighted). > > Also, I noticed that the gradient colors on the button are not close to > color selected by the user. Maybe we can change it? Of course we can change it. But this is also strange, it should be based on an interpolation of foreground and background. What colors do you use for testing? Can you please upload a screenshot?
Created attachment 269621 [details] button color vs selected color
Created attachment 269622 [details] default button color
(In reply to Till Brychcy from comment #21) > (In reply to Lakshmi Shanmugam from comment #20) > > I'm unable to resolve 2 issues with the patch: > > > > 1. The focus ring is not shown for push & toggle buttons anymore, with the > > patch. > > I tried on a different machine which has a fresh install of 10.12 and it > > worked there. But, unable to get it to work on my machine (10.12.6). > > Strange. Always, or only if the background is set? > > Hmm maybe something is configured differently in the system settings? > Does ist appear/disappear if you toggle the tabbing-behaviour with ctrl-f7 > and then use "tab" to switch focus? > This happens only when background is set. I tried toggling the setting in the preferences and then used tab, but focus ring for button disappears as soon as background color is set and appears as soon as background is unset. I can create a screencast to show this. > > > > > Also, I noticed that the gradient colors on the button are not close to > > color selected by the user. Maybe we can change it? > > Of course we can change it. But this is also strange, it should be based on > an interpolation of foreground and background. What colors do you use for > testing? > Can you please upload a screenshot? Attached screenshot. The button color looks much darker than the selected color. Though there is a gradient, can it be made lighter so that it looks closer to the selected color. Attached the default button color for comparison.
Thank you for the screenshots. Is it just me or are the texts not really centered vertically? They look like slightly moved to the top.
(In reply to Lakshmi Shanmugam from comment #24) > This happens only when background is set. I tried toggling the setting in > the preferences and then used tab, but focus ring for button disappears as > soon as background color is set and appears as soon as background is unset. Really strange. > I can create a screencast to show this. I don't know if that will help, but I currently have no other idea. > Attached screenshot. The button color looks much darker than the selected > color. Though there is a gradient, can it be made lighter so that it looks > closer to the selected color. Attached the default button color for > comparison. Now I see what you mean (if I choose a saturated blue or red background.) I'll try to come up with a better algorithm for the other color in the gradient. Moving target to 4.8 M2.
Created attachment 269662 [details] screenshot with patchset-4 This is the screenshot with the latest patchset-4. It has thinner borders than the buttons in the previous screenshot.
(In reply to Thomas Singer from comment #25) > Thank you for the screenshots. Is it just me or are the texts not really > centered vertically? They look like slightly moved to the top. I've attached the screenshot with the latest patch-set. The buttons have thinner borders now. Thanks for pointing out. Looking closely with Pixie, the button with bg color is bigger than the button without bg color by 2 pixels. And the Text is moved to top by 2pixels.
(In reply to Till Brychcy from comment #26) > (In reply to Lakshmi Shanmugam from comment #24) > > This happens only when background is set. I tried toggling the setting in > > the preferences and then used tab, but focus ring for button disappears as > > soon as background color is set and appears as soon as background is unset. > Really strange. > I agree it's strange, as this problem doesn't happen on another machine with a fresh install of 10.12. I'll try to dig more and see I find why this is happening. Also, it'll be helpful if someone else can try the patch and verify if the focus ring appears correctly on their machine.
(In reply to Lakshmi Shanmugam from comment #28) > (In reply to Thomas Singer from comment #25) > > Thank you for the screenshots. Is it just me or are the texts not really > > centered vertically? They look like slightly moved to the top. > > I've attached the screenshot with the latest patch-set. The buttons have > thinner borders now. > Thanks for pointing out. Looking closely with Pixie, the button with bg > color is bigger than the button without bg color by 2 pixels. And the Text > is moved to top by 2pixels. I assume you mean 2 retina pixels here, right? I'll have to look again. IIRC the text drawing code is not touched, so it would be that the whole button is shifted vertically. I can't remember the details, but the i think the current state was the closest I could get to the unstyled design. Maybe this depends on the macos-version?
(In reply to Till Brychcy from comment #30) > (In reply to Lakshmi Shanmugam from comment #28) > > (In reply to Thomas Singer from comment #25) > > > Thank you for the screenshots. Is it just me or are the texts not really > > > centered vertically? They look like slightly moved to the top. > > > > I've attached the screenshot with the latest patch-set. The buttons have > > thinner borders now. > > Thanks for pointing out. Looking closely with Pixie, the button with bg > > color is bigger than the button without bg color by 2 pixels. And the Text > > is moved to top by 2pixels. > > I assume you mean 2 retina pixels here, right? Right > > I'll have to look again. IIRC the text drawing code is not touched, so it > would be that the whole button is shifted vertically. > I can't remember the details, but the i think the current state was the > closest I could get to the unstyled design. Maybe this depends on the > macos-version? I'm using 10.12.6, not sure if it depends on mac version. I can verify this on my other setup which has a different os version. You don't see this on your machine?
(In reply to Lakshmi Shanmugam from comment #31) > > > > I'll have to look again. IIRC the text drawing code is not touched, so it > > would be that the whole button is shifted vertically. > > I can't remember the details, but the i think the current state was the > > closest I could get to the unstyled design. Maybe this depends on the > > macos-version? > I'm using 10.12.6, not sure if it depends on mac version. I can verify this > on my other setup which has a different os version. You don't see this on > your machine? OK, I've checked again. I intentionally made the button a bit bigger towards the button because it looks better that way in the dark design. To get the same size, you have to change the call from NSRect rect2 = smallerRect(cellFrame, 6.5f, 4.5f, 6.5f, lineWidth); to NSRect rect2 = smallerRect(cellFrame, 6.5f, 4.5f, 7.5f, lineWidth); I'll upload screenshots.
Created attachment 269664 [details] dark design with larger buttons like in patch set 4
Created attachment 269665 [details] dark design with same size as in unstyled
(In reply to Till Brychcy from comment #32) > OK, I've checked again. I intentionally made the button a bit bigger towards > the button because it looks better that way in the dark design. > We shouldn't make the button bigger, as it also moves the text up. May be it's not visible in the dark style screenshot, because the text seems thicker (bold look). We'll have to verify with Pixie. I think, Button background color will be used in general as well and is not only specific to the dark theme.
(In reply to Lakshmi Shanmugam from comment #35) > We shouldn't make the button bigger, as it also moves the text up. May be To be precise, the text drawing is not changed at all, it is just the bottom of the button that is drawn further down, so w.r.t. that, the text looks moved up (and to me it looks better this way, which is why I did it.) > it's not visible in the dark style screenshot, because the text seems > thicker (bold look). We'll have to verify with Pixie. > I think, Button background color will be used in general as well and is not > only specific to the dark theme. If you think so, let's do it this way.
In patch set 5, I've reduced the height so the button have the same size as unstyled. In patch set 6, I've changed the algorithm for determining auxillary colors (I've replaced the code copied from the windows implementation by a method that selects lighter or darker colors without changing the hue). Also, I've made the gradient on the button more detailed, so the central area is precisely the selected color and I've also made the selected state always a darker version of the set background color (previously it was lighter if the color was already dark)
Created attachment 269697 [details] Dark design with patch set 6
(In reply to Till Brychcy from comment #37) > In patch set 5, I've reduced the height so the button have the same size as > unstyled. > In patch set 6, I've changed the algorithm for determining auxillary colors > (I've replaced the code copied from the windows implementation by a method > that selects lighter or darker colors without changing the hue). Also, I've > made the gradient on the button more detailed, so the central area is > precisely the selected color and I've also made the selected state always a > darker version of the set background color (previously it was lighter if > the color was already dark) Thanks Till, the background color on the button looks much better now! The focus ring disappearing is the remaining issue, I'll spend some time tomorrow to figure out what's happening.
In the dark design, disabled button were hardly readable. Patch 7 contains code to fix this my using a lighter/darker variation of the configured foreground color. Also, I've cleaned up some code formatting issues.
Another refinement: In the dark design, the highlighted state was hardly detectable for flat buttons (e.g. in the dependencies tab of the ManifestEditor or the MavenPomEditor) Fixed in change set 8 by also making the border color a bit darker in the highlighted state (only for flat buttons, as this is not necessary for others because of their gradient in non-highlighted state).
(In reply to Lakshmi Shanmugam from comment #39) > Thanks Till, the background color on the button looks much better now! > The focus ring disappearing is the remaining issue, I'll spend some time > tomorrow to figure out what's happening. I was unable to figure out the problem with the focus ring missing on my machine. The problem doesn't happen on 2 other machines on which I tested. So, I think we can track this as separate bug and go ahead with the patch for M2.
Created attachment 270078 [details] disabled wrap style button There are 2 issues when background color is set on SWT.WRAP style buttons: 1. Disable the button, the button doesn't get disabled look. This happens only with default foreground color.
Created attachment 270079 [details] bigger wrap style button 2. When background color is set on the SWT.WRAP style button, the rounded button becomes flat style and size of the button increases by 16px.
(In reply to Lakshmi Shanmugam from comment #44) > Created attachment 270079 [details] > bigger wrap style button > > 2. When background color is set on the SWT.WRAP style button, the rounded > button becomes flat style and size of the button increases by 16px. sorry, button size increase by 12px, not 16.
(In reply to Lakshmi Shanmugam from comment #43) > Created attachment 270078 [details] > disabled wrap style button > > There are 2 issues when background color is set on SWT.WRAP style buttons: > 1. Disable the button, the button doesn't get disabled look. This happens > only with default foreground color. (You mean the foreground color. This was already an inconsistency without this patch.) Fixed in patch set #10 using a hardcoded color (picked from the screen from a disabled non-WRAP button without foreground color set) (In reply to Lakshmi Shanmugam from comment #44) > Created attachment 270079 [details] > bigger wrap style button > > 2. When background color is set on the SWT.WRAP style button, the rounded > button becomes flat style and size of the button increases by 16px. Also fixed in patch set #10 (I tried to make the size the same as without background color set, which is also a bit bigger than non-WRAP)
Gerrit change https://git.eclipse.org/r/95214 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=0650943303854f471afb27787b61aa951ffc6d5e
Thanks for all the hard work Till! Fixed in master.
(In reply to Eclipse Genie from comment #47) > Gerrit change https://git.eclipse.org/r/95214 was merged to [master]. > Commit: > http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/ > ?id=0650943303854f471afb27787b61aa951ffc6d5e Kudos to Till!!!
I've created Bug 521985 for the css-change to is this in the dark them, I'll update the gerrit for it in a moment.
(In reply to Lakshmi Shanmugam from comment #42) > (In reply to Lakshmi Shanmugam from comment #39) > I was unable to figure out the problem with the focus ring missing on my > machine. > The problem doesn't happen on 2 other machines on which I tested. So, I > think we can track this as separate bug and go ahead with the patch for M2. I verified with Build I20170908-0035 in dark theme mode, the focus ring for buttons appear fine in Eclipse. But, the problem still happens with SWT Examples & snippets. Opened Bug 522040 to track this.
Verified with I20170911-2000.
