Community
Participate
Working Groups
To see the problem, launch the Control example and go to the Text tab. Toggle the Enabled flag to off. Notice that the Text widget looks disabled but not the StyledText one. (it's labeled as RichText -- has been fixed in Eclipse View copy of Controls Example) For consistency, I think we should supply a Disabled look for StyledText. NOTES:
PRODUCT VERSION: Build 125
StyledText is part of the Eclipse branding. It should draw in a way which is consistant with the u/i designer's vision for the platform. No action should be taken at this time.
*** Bug 33594 has been marked as a duplicate of this bug. ***
Reassigning to SWT Inbox since OTT taking over StyledText.
I think it's OK that read-only text/source editors don't get a "disabled" background, but if StyledText is used inside a dialog or form editor, it's rather strange that all other controls look disabled, but StyledText doesn't. +1 for a user-settable flag to render as disabled.
To implement this feature StyeldText would need API to get the right gray color from the OS. In win32 it would be COLOR_GRAYTEXT=17. Steve ?
Hmm, sorry for my confusion of disabled vs. read-only. What bug 158277 actually needs is a read-only look, which is different from disabled for the Text widget: - disabled: gray text, light gray bg - read-only: black text, light gray bg Bug 22782 requests API to draw 'disabled' gray text.
This bug asks for 'disable look'. Markus, I didn't find API in the OS to get the background color for read-only text widget.
> - read-only: black text, light gray bg On Windows only. The other platforms don't do this.
*** Bug 183972 has been marked as a duplicate of this bug. ***
*** Bug 216666 has been marked as a duplicate of this bug. ***
*** Bug 250658 has been marked as a duplicate of this bug. ***
Your bug has been moved to triage, visit http://www.eclipse.org/swt/triage.php for more info.
Can this be reconsidered? We'd like to use a styled label but we also need to disable it which is currently looking bad due to this bug here.
I will be adding functionality to the License Information pane in the Feature.xml editor in Eclipse that will look better on Windows if this defect is fixed.
Specifically, UI element has a side by side Text and StyledText. The new feature causes the enabled state of these two widgets to be toggled in unison. Unfortunately, the Text gets the disabled look but the StyledText does not. The proximity of the two widgets really calls attention to the differing behaviours.
Please see Bug 22782.
There are still reports about this bug.
Maybe add a listener that changes the background color of sort...
New Gerrit change created: https://git.eclipse.org/r/100286
(putting a target milestone since there is already a patch pending for review, so 4.8.M1 seems to be a reachable target)
Not ready for M1.
Re-targeting for M3.
Doesn't look like Ian will be working on this atm. Removing target milestone. If someone want's to take over, please do.
Created attachment 278558 [details] Snippet to compare Text and StyledText Snippet to compare the disabled look for Text and Styledtext
Created attachment 278561 [details] Windows7/10: Difference_in_disabling_behavior_of_Text_and_StyledText (In reply to Lakshmi Shanmugam from comment #25) > Created attachment 278558 [details] > Snippet to compare Text and StyledText > > Snippet to compare the disabled look for Text and Styledtext Attached Screen-shot of above Snippet as tested on Win7 & Win10 showing the difference in disabled behavior.
Created attachment 278562 [details] Mac screenshot:Snippet with patch applied Screenshot of disabled look on Mac for Light and Dark themes with the patch applied.
Can you post a picture of the ControlExample color tab running on Mac and Windows? I want to see how the system colors look there. Please make sure you scroll to the bottom so all the system colors are visible.
Created attachment 278584 [details] Mac - SWT color constants in light theme
Hmm, looks like the Mac colors are a bit different. Maybe we can use COLOR_WIDGET_NORMAL_SHADOW for the disabled foreground color on Mac?
Created attachment 278597 [details] Win7 - SWT color constants in light theme
Thanks Niraj. I'm busy with some internal tasks this week, so I plan to work on this bug for 4.13.
(In reply to Eric Williams from comment #30) > Hmm, looks like the Mac colors are a bit different. Maybe we can use > COLOR_WIDGET_NORMAL_SHADOW for the disabled foreground color on Mac? I think, we should match the foreground and background colors of the disabled StyledText with the disabled native Text widget on a platform as much as possible. We should also note that the colors would be different for dark and light themes. So, instead of using the fixed color constant like COLOR_WIDGET_NORMAL_SHADOW, may be we could have new constants for the disabled foreground and background color, and it'll be initialized correctly for the platform and theme. WDYT?
(In reply to Lakshmi Shanmugam from comment #33) > I think, we should match the foreground and background colors of the > disabled StyledText with the disabled native Text widget on a platform as > much as possible. +1
(In reply to Lakshmi Shanmugam from comment #33) > (In reply to Eric Williams from comment #30) > > Hmm, looks like the Mac colors are a bit different. Maybe we can use > > COLOR_WIDGET_NORMAL_SHADOW for the disabled foreground color on Mac? > > I think, we should match the foreground and background colors of the > disabled StyledText with the disabled native Text widget on a platform as > much as possible. We should also note that the colors would be different for > dark and light themes. > So, instead of using the fixed color constant like > COLOR_WIDGET_NORMAL_SHADOW, may be we could have new constants for the > disabled foreground and background color, and it'll be initialized correctly > for the platform and theme. WDYT? +1 from me, sounds much better.
New Gerrit change created: https://git.eclipse.org/r/146273
Should a widget change background/foreground color if a client calls setBackground/setForeground on it while it's disabled? I.e. consider the following: Control.setEnabled(false) Control.setBackground(red) should the control have a red background?
(In reply to Eric Williams from comment #37) > Should a widget change background/foreground color if a client calls > setBackground/setForeground on it while it's disabled? I.e. consider the > following: > > Control.setEnabled(false) > Control.setBackground(red) > > should the control have a red background? Not sure what's the behavior on all platforms. On Mac StyledText and other controls change the background/foreground color even when disabled.
(In reply to Lakshmi Shanmugam from comment #38) > (In reply to Eric Williams from comment #37) > > Should a widget change background/foreground color if a client calls > > setBackground/setForeground on it while it's disabled? I.e. consider the > > following: > > > > Control.setEnabled(false) > > Control.setBackground(red) > > > > should the control have a red background? > > Not sure what's the behavior on all platforms. > On Mac StyledText and other controls change the background/foreground color > even when disabled. Same on GTK, Niraj can you check the behaviour on Windows?
(In reply to Eric Williams from comment #39) > (In reply to Lakshmi Shanmugam from comment #38) > > (In reply to Eric Williams from comment #37) > > > Should a widget change background/foreground color if a client calls > > > setBackground/setForeground on it while it's disabled? I.e. consider the > > > following: > > > > > > Control.setEnabled(false) > > > Control.setBackground(red) > > > > > > should the control have a red background? > > > > Not sure what's the behavior on all platforms. > > On Mac StyledText and other controls change the background/foreground color > > even when disabled. > > Same on GTK, Niraj can you check the behaviour on Windows? On Win7: - For disabled Native widgets: We could only set the background and not the foreground color. - For disabled Custom widgets: We could set both background/foreground colors [Note: CCombo is an exception here, where we could only set background]
Hmm alright, I'll make it so that setting the bg/fg color while disabled still works.
Gerrit change https://git.eclipse.org/r/146273 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=6f7a89a9aab424b92bc846d66939ab93a4ea38d7
(In reply to Eclipse Genie from comment #42) > Gerrit change https://git.eclipse.org/r/146273 was merged to [master]. > Commit: > http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/ > ?id=6f7a89a9aab424b92bc846d66939ab93a4ea38d7 In master now. I'll prepare a N&N entry shortly.
New Gerrit change created: https://git.eclipse.org/r/147114
New Gerrit change created: https://git.eclipse.org/r/147382
Gerrit change https://git.eclipse.org/r/147114 was merged to [master]. Commit: http://git.eclipse.org/c/www.eclipse.org/eclipse/news.git/commit/?id=61e463b4759dfbb5bcf1ce470e737304cfa0906c
(In reply to Eclipse Genie from comment #46) > Gerrit change https://git.eclipse.org/r/147114 was merged to [master]. > Commit: > http://git.eclipse.org/c/www.eclipse.org/eclipse/news.git/commit/ > ?id=61e463b4759dfbb5bcf1ce470e737304cfa0906c N&N entry in master now.
Verified on Mac with I20190819-2355.
Verified in I20190820-0600.
Eric, some suggestions for the changes in StyledText.setEnabled in 6f7a89a9a: - I suggest to put the code between the 2 assignments to insideSetEnableCall inside try-finally, so this flag is reset correctly if an exception happens in setBackground/setForeground - the line public void setEnabled (boolean enabled) { shouldn't have a space before the brace
(In reply to Thomas Singer from comment #50) > Eric, some suggestions for the changes in StyledText.setEnabled in 6f7a89a9a: > > - I suggest to put the code between the 2 assignments to insideSetEnableCall > inside try-finally, so this flag is reset correctly if an exception happens > in setBackground/setForeground > Please prepare a Gerrit and I'd be happy to review it. > - the line > > public void setEnabled (boolean enabled) { > > shouldn't have a space before the brace AFAIK this is the SWT style convention, if you look at the other methods in StyledText, they have it too.
(In reply to Eric Williams from comment #51) > (In reply to Thomas Singer from comment #50) > > Eric, some suggestions for the changes in StyledText.setEnabled in 6f7a89a9a: > > > > - I suggest to put the code between the 2 assignments to insideSetEnableCall > > inside try-finally, so this flag is reset correctly if an exception happens > > in setBackground/setForeground > > > > Please prepare a Gerrit and I'd be happy to review it. Wouldn't this be overkill? (In reply to Eric Williams from comment #51) > > - the line > > > > public void setEnabled (boolean enabled) { > > > > shouldn't have a space before the brace > > AFAIK this is the SWT style convention, if you look at the other methods in > StyledText, they have it too. I have verified and found no other method declaration (in the first half of the class) that had a space between method name and (.
(In reply to Thomas Singer from comment #52) > (In reply to Eric Williams from comment #51) > > (In reply to Thomas Singer from comment #50) > > > Eric, some suggestions for the changes in StyledText.setEnabled in 6f7a89a9a: > > > > > > - I suggest to put the code between the 2 assignments to insideSetEnableCall > > > inside try-finally, so this flag is reset correctly if an exception happens > > > in setBackground/setForeground > > > > > > > Please prepare a Gerrit and I'd be happy to review it. > > Wouldn't this be overkill? Overkill how? I don't have time to look at this right now, but if you want to make a Gerrit I'd happily push it. > (In reply to Eric Williams from comment #51) > > > - the line > > > > > > public void setEnabled (boolean enabled) { > > > > > > shouldn't have a space before the brace > > > > AFAIK this is the SWT style convention, if you look at the other methods in > > StyledText, they have it too. > > I have verified and found no other method declaration (in the first half of > the class) that had a space between method name and (. Is the issue the space between the name and the "("? Or the space between the ")" and the "{"? I see both in many other SWT classes. If you want to change it I won't oppose it.
New Gerrit change created: https://git.eclipse.org/r/149895
Gerrit change https://git.eclipse.org/r/149895 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=b684f41801161dabfff61748cdda116578a4931f
This fix seems to have caused bug 553099.
