Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SWT Dark Theme ToolItem Toggle Buttons not showing down state #467

Closed
mikevwdriver opened this issue Nov 30, 2022 · 15 comments
Closed

SWT Dark Theme ToolItem Toggle Buttons not showing down state #467

mikevwdriver opened this issue Nov 30, 2022 · 15 comments

Comments

@mikevwdriver
Copy link

I am using the 2022-12 RC1 on Windows 10. Toggle buttons in tool bars and even regular toggle buttons are not showing their down state as expected.

image
You can't tell, but the sync with editor button is currently pressed down.

The same issue can be seen in the perspective chooser:
image

@mikevwdriver
Copy link
Author

mikevwdriver commented Dec 1, 2022

@vogella I've identified the commit that caused this issue:
c07ab53

Before this commit, ToolItem toggle buttons had a color that indicated their down state, starting with this commit, the down state is no longer apparent on Dark theme.

edit: It does appear that regular SWT toggle buttons were broken by a different commit than the ToolItem toggle buttons. For this separate issue, I've opened eclipse-platform/eclipse.platform.swt#483

@mikevwdriver mikevwdriver changed the title SWT Dark Theme Toggle Buttons not showing down state SWT Dark Theme ToolItem Toggle Buttons not showing down state Dec 1, 2022
@mikevwdriver
Copy link
Author

// ToolItem prevents itself from repaints if the same color is set
((ToolItem) widget).setBackground(newColor);

If I remove this one line from CSSPropertyBackgroundSWTHandler.java, the ToolItems are styled correctly.

@mikevwdriver
Copy link
Author

Okay, more information. This is a bug that affects the standard Eclipse IDE. Our product has it's own custom CSS, it was affected by this change as well, however, simply by removing all instances of ToolItem in our dark theme CSS files, we are no longer affected by this issue.

I took a look at the platform CSS and although it does not directly reference ToolItem, I suspect it is inheriting a rule which causes the background to be explicitly set, which will unfortunately have an effect on the toggled/checked state.

@humphreygao
Copy link

#340

@de-jcup
Copy link

de-jcup commented Jan 30, 2023

Is there any update on fixing this issue?

I am using now eclipse 2022-12 (nearly only JDT) and I am having really big problems with this:

At Console, JUnit, Project explorer and many more views it is not possible to determine if an icon inside a view toolbar is enabled or disabled.

Using the dark mode with Linux Mint (GTK) variants, there is absolutely no way to check this. Especially when it comes to the console view this is very irritating (e.g. the "Show console log when standard out changes"). There is no menu inside where I can check the states (as a workaround):

image

(remark: i toggled the states on/off and it always look the same...)

@de-jcup
Copy link

de-jcup commented Jan 30, 2023

Hmm... tried out oomp-installer + platform IDE - starting Eclipse 2023-03 (master) I had following:

image

The toolbar icons are now correct rendered with Theme "dark". Enabled ToggleIcons have now a border again, so it's clear which state they have.

If this looks same in JDT (and I think so) than this has been already fixed for 2023-03

@trancexpress
Copy link
Contributor

trancexpress commented Jan 30, 2023

Some of the screenshots here show many views opened in the same part stack, i.e. there are many tabs. When there are many tabs, the toolbar buttons will be painted not on the tab bar, but below it. You might want to check this case as well, just in case (since the last screenshot I see has only a few tabs and the toolbar buttons are painted on the tab bar).

@merks
Copy link
Contributor

merks commented Jan 30, 2023

It's true that some seem to paint their check state and others don't:

image

@de-jcup
Copy link

de-jcup commented Jan 30, 2023

Yes It is still a problem with upcoming 2023-03. It was only correct rendered, because the toolbar was "on top". When reducing the size of the window, the toolbar is no longer rendered with the light grey background, but uses the dark one: The Border has the same color like the dark background - so border for selection becomes invisible.
image

de-jcup added a commit to de-jcup/eclipse.platform.ui that referenced this issue Jan 30, 2023
…#536

- Toolbar has in dark mode now a dedicated color
  instead of css inheritance. A parent container
  change by resizing does not change the toolbar color any longer
- The color is choosen in way that the main toolbar looks as expected
  before. Also the toggle border is now visible again, which could
  address eclipse-platform#467 as well.

Signed-off-by: Albert Tregnaghi <[email protected]>
de-jcup added a commit to de-jcup/eclipse.platform.ui that referenced this issue Jan 30, 2023
…#536

- Toolbar has in dark mode now a dedicated color
  instead of css inheritance. A parent container
  change by resizing does not change the toolbar color any longer
- The color is choosen in way that the main toolbar looks as expected
  before. Also the toggle border is now visible again, which could
  address eclipse-platform#467 as well.

Signed-off-by: Albert Tregnaghi <[email protected]>
@l-uuz
Copy link

l-uuz commented Mar 23, 2023

Still present in 2023-03. Is there a workaround? This is really annoying, e.g. with the button "Skip all breakpoints".

@trancexpress
Copy link
Contributor

Using GTK+ Inspector and comparing toolbar buttons with Breeze Dark on KDE:

  1. The main toolbar buttons have background color (252,252,252,0.125). There is a theme .css line associated with the color.
  2. View toolbar buttons have background color (81,86,88). There is no theme .css line associated with the color.

In an SWT example for toolbar button toggles, (252,252,252,0.125) seems to be the expected background color. Makes sense since the for the main toolbar the toggle state is visible. For the view toolbars, only the border is visible for me.

public class ToolbarToggleExample {

	public static void main(String[] args) {
		Display display = new Display();
		Shell shell = new Shell(display);
		shell.setSize(400, 300);
		shell.setText("Toolbar with Images");
		shell.setLayout(new RowLayout(SWT.VERTICAL));
		ToolBar toolBar = new ToolBar(shell, SWT.HORIZONTAL);
		createToolItem(toolBar, SWT.CHECK, "Check One", "This is check one");
		createToolItem(toolBar, SWT.CHECK, "Check Two", "This is check two");
		shell.open();
		while (!shell.isDisposed()) {
			if (!display.readAndDispatch()) {
				display.sleep();
			}
		}
		display.dispose();
	}

	private static ToolItem createToolItem(ToolBar parent, int type, String text, String toolTipText) {
		ToolItem item = new ToolItem(parent, type);
		item.setText(text);
		item.setToolTipText(toolTipText);
		return item;
	}
}

The unexpected color Color {81, 86, 88, 255} is set here:

"main" #1 prio=6 os_prio=0 cpu=1458.75ms elapsed=34.43s tid=0x00007f58ac015460 nid=0x8afa at breakpoint [0x00007f58b0ffb000]
   java.lang.Thread.State: RUNNABLE
        at org.eclipse.swt.widgets.ToolItem.setBackground(ToolItem.java:1111)
        at org.eclipse.e4.ui.css.swt.properties.css2.CSSPropertyBackgroundSWTHandler.applyCSSPropertyBackgroundColor(CSSPropertyBackgroundSWTHandler.java:76)
        at org.eclipse.e4.ui.css.core.dom.properties.css2.AbstractCSSPropertyBackgroundHandler.applyCSSProperty(AbstractCSSPropertyBackgroundHandler.java:43)
        at org.eclipse.e4.ui.css.swt.properties.css2.CSSPropertyBackgroundSWTHandler.applyCSSProperty(CSSPropertyBackgroundSWTHandler.java:43)
        at org.eclipse.e4.ui.css.core.impl.engine.AbstractCSSEngine.applyCSSProperty(AbstractCSSEngine.java:746)
        at org.eclipse.e4.ui.css.core.impl.engine.AbstractCSSEngine.applyStyleDeclaration(AbstractCSSEngine.java:552)
        at org.eclipse.e4.ui.css.core.impl.engine.AbstractCSSEngine.applyStyles(AbstractCSSEngine.java:426)
        at org.eclipse.e4.ui.css.core.impl.engine.AbstractCSSEngine.applyStyles(AbstractCSSEngine.java:374)
        at org.eclipse.e4.ui.css.swt.engine.CSSSWTApplyStylesListener.lambda$0(CSSSWTApplyStylesListener.java:31)
        at org.eclipse.e4.ui.css.swt.engine.CSSSWTApplyStylesListener$$Lambda$317/0x00000008004ff660.handleEvent(Unknown Source)
        at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:89)
        at org.eclipse.swt.widgets.Display.sendEvent(Display.java:5855)
        at org.eclipse.swt.widgets.Display.sendEvent(Display.java:5847)
        at org.eclipse.swt.widgets.Display.runSkin(Display.java:5146)
        at org.eclipse.swt.widgets.Composite.computeSizeInPixels(Composite.java:262)
        at org.eclipse.swt.widgets.Control.computeSize(Control.java:838)
        at org.eclipse.swt.widgets.Control.pack(Control.java:1610)
        at org.eclipse.swt.widgets.Control.pack(Control.java:1585)
        at org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer.addTopRight(StackRenderer.java:786)
        at org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer.createWidget(StackRenderer.java:673)
        ...

The used color is defined here:

bundles/org.eclipse.ui.themes/css/dark/e4-dark_ide_colorextensions.css:ColorDefinition#org-eclipse-ui-workbench-DARK_BACKGROUND {
bundles/org.eclipse.ui.themes/css/dark/e4-dark_ide_colorextensions.css- color: #515658;

In org.eclipse.ui.themes/css/dark/e4-dark_partstyle.css, the color is used for buttons (toolbar buttons have nested buttons in GTK3):

Shell,
Composite, ScrolledComposite, ExpandableComposite, Canvas, TabFolder, CLabel, Label,
CoolBar, Sash, Group, RefactoringLocationControl, ChangeParametersControl, Link, FilteredTree,
ProxyEntriesComposite, NonProxyHostsComposite, DelayedFilterCheckboxTree,
Splitter, ScrolledPageContent, ViewForm, LaunchConfigurationFilteredTree,
ContainerSelectionGroup, BrowseCatalogItem, EncodingSettings,
ProgressMonitorPart, DocCommentOwnerComposite, NewServerComposite,
NewManualServerComposite, ServerTypeComposite, FigureCanvas,
DependenciesComposite, ListEditorComposite, WrappedPageBook,
CompareStructureViewerSwitchingPane, CompareContentViewerSwitchingPane,
QualifiedNameComponent, RefactoringStatusViewer,
MessageLine,
Button /* SWT-BUG: checkbox inner label font color is not accessible */,
Composite > *,
Composite > * > *,
Group > StyledText {
    background-color:'#org-eclipse-ui-workbench-DARK_BACKGROUND'; 
    color:'#org-eclipse-ui-workbench-DARK_FOREGROUND'; 
}

The code in ToolItem.updateStyle() that sets the background color:

	if (background != null) {
		css +=
			"button { background-image: none; background-color: " + display.gtk_rgba_to_css_string (background.handle) + "; }";
	}

Seems odd that this would overwrite the other states (disabled, checked, etc.) from the theme... Maybe all of the .css for toolbar buttons is overriden with this.

The change was added for: https://bugs.eclipse.org/bugs/show_bug.cgi?id=579470

I don't understand why this styling doesn't apply to the main toolbar. Maybe because the .css specifies CoolBar but not ToolBar? I tried adding ToolBar though and the main toolbar still has no problem.

@trancexpress
Copy link
Contributor

trancexpress commented Mar 24, 2023

@nnemkin can you take this over? I'm not sure what the best approach here is. For Breeze Dark, this behavior emulates what the main toolbar does:

diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/ToolItem.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/ToolItem.java
index cd970973ab..5673e8676e 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/ToolItem.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/ToolItem.java
@@ -1602,7 +1602,9 @@ void updateStyle () {
                css += parent.cssForeground;
        }
        if (background != null) {
-               css += "button { background-image: none; background-color: " + display.gtk_rgba_to_css_string (background.handle) + "; }";
+               css +=
+                       "button { border: none; background-image: none; background-color: " + display.gtk_rgba_to_css_string (background.handle) + "; }\n" +
+                       "button:checked { background-color: transparent; }\n";
        }
 
        if (GTK.GTK4) {

But if the toolbar is drawn on the same line as tabs in a part stack, the color difference is not enough to see the toggled-off button. Since the part stack line has the same color as the toolbar button background.

It doesn't look like the platform UI .css can provide a toggled-off background color. Hard-coding one doesn't seem like a good approach either. E.g. this still doesn't yield good results and is not consistent with the main toolbar:

diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/ToolItem.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/ToolItem.java
index cd970973ab..819b9c4a26 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/ToolItem.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/ToolItem.java
@@ -1602,7 +1602,12 @@ void updateStyle () {
                css += parent.cssForeground;
        }
        if (background != null) {
-               css += "button { background-image: none; background-color: " + display.gtk_rgba_to_css_string (background.handle) + "; }";
+               Color disabledBackgroundColor = new Color(background.getRGB(), 127);
+               String disabledBackgroundColorCss = display.gtk_rgba_to_css_string (disabledBackgroundColor.handle);
+               disabledBackgroundColor.dispose();
+               css +=
+                       "button { background-image: none; background-color: " + display.gtk_rgba_to_css_string (background.handle) + "; }\n" +
+                       "button:checked { background-color: " + disabledBackgroundColorCss + "; }\n";
        }
 
        if (GTK.GTK4) {

IMO the first step here is to make the main toolbar behave the same as the view toolbar. As well as, to ensure the view toolbar buttons are visible both when drawn in the view client area and when drawn on the tab line of the view part stack.

Then a color can be chosen with which using background-color: transparent; for toggled-off buttons ensures visibility both for toggled-on and toggled-off state.

@de-jcup
Copy link

de-jcup commented Mar 27, 2023

It has been some time when I tried to fix the problem and maybe it got lost, so I bring up my thoughts here:

In my draft PR #538 I fixed this problem, but I was forced to remove c07ab53 to get it working. (done in second commit of my draft PR, you can see the new (and good looking )UI behavior inside a picture at #538 (comment)).

I am not very good in SWT or in SWT CSS styling - I was/am stuck at this point. And I am running out of time. But maybe my draft PR is an idea for somebody else?

@akurtakov , @vogella : Maybe the remove or an improvement of c07ab53 could be an option? The current situation is extreme annoying (at least for myself) .

@akurtakov
Copy link
Member

I'm sorry but I can not dedicate time to look into this one.

@vogella
Copy link
Contributor

vogella commented Dec 9, 2024

With the update dark theme for 2025-03 this should be gone, now on Windows the selected state is a bit to visible see #2570

To test please use latest I-Build https://download.eclipse.org/eclipse/downloads/

@vogella vogella closed this as completed Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants