Skip to content

Allow to ignore explicitly defined disabled icons #4002

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.

Sign up for GitHub

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

Open
HeikoKlare wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Allow to ignore explicitly defined disabled icons #4002

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ...renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/AbstractContributionItem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.eclipse.e4.ui.workbench.modeling.EModelService;
import org.eclipse.e4.ui.workbench.swt.util.ISWTResourceUtilities;
import org.eclipse.emf.common.util.URI;
import org.eclipse.jface.action.ActionContributionItem;
import org.eclipse.jface.action.ContributionItem;
import org.eclipse.jface.action.IContributionManager;
import org.eclipse.jface.action.IMenuCreator;
Expand Down Expand Up @@ -211,6 +212,9 @@ protected void updateIcons() {
}

private String getDisabledIconURI(MItem toolItem) {
if (!ActionContributionItem.getUseDisabledIcons()) {
return ""; //$NON-NLS-1$
}
Object obj = toolItem.getTransientData().get(IPresentationEngine.DISABLED_ICON_IMAGE_KEY);
return obj instanceof String s ? s : ""; //$NON-NLS-1$
}
Expand Down
2 changes: 1 addition & 1 deletion bundles/org.eclipse.jface/META-INF/MANIFEST.MF
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %pluginName
Bundle-SymbolicName: org.eclipse.jface;singleton:=true
Bundle-Version: 3.39.100.qualifier
Bundle-Version: 3.40.0.qualifier
Bundle-Vendor: %providerName
Bundle-Localization: plugin
Export-Package: org.eclipse.jface,
Expand Down
37 changes: 36 additions & 1 deletion bundles/org.eclipse.jface/src/org/eclipse/jface/action/ActionContributionItem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@
@NoExtend
public class ActionContributionItem extends ContributionItem {

/**
* System property for specifying whether custom disabled icons for actions
* shall be used or whether all disabled icons shall be generated on-the-fly.
*/
private static final String SYSTEM_PROPERTY_USE_DISABLED_ICONS = "org.eclipse.jface.action.useDisabledIcons"; //$NON-NLS-1$

Comment thread
HannesWell marked this conversation as resolved.
/**
* Mode bit: Show text on tool items or buttons, even if an image is
* present. If this mode bit is not set, text is only shown on tool items if
Expand Down Expand Up @@ -96,6 +102,32 @@ public static void setUseColorIconsInToolbars(boolean useColorIcons) {
USE_COLOR_ICONS = useColorIcons;
}

private static boolean USE_DISABLED_ICONS = Boolean.getBoolean(SYSTEM_PROPERTY_USE_DISABLED_ICONS);

/**
* Returns whether disabled icons assigned to actions shall be used or whether
* all such disabled icons shall be generated on-the-fly.
*
* @return <code>true</code> if disabled icons assigned to actions shall be
* used, <code>false</code> otherwise
* @since 3.40
*/
public static boolean getUseDisabledIcons() {
return USE_DISABLED_ICONS;
}

/**
* Sets whether disabled icons assigned to actions shall be used or whether all
* such disabled icons shall be generated on-the-fly.
*
* @param useDisabledIcons <code>true</code> if disabled icons set to actions
* shall be used, <code>false</code> otherwise
* @since 3.40
*/
public static void setUseDisabledIcons(boolean useDisabledIcons) {
USE_DISABLED_ICONS = useDisabledIcons;
}

Comment on lines +127 to +130
Copy link
Copy Markdown
Member

@HannesWell HannesWell May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this just for testing or should this also be used by down-stream consumers?

Copy link
Copy Markdown
Contributor Author

@HeikoKlare HeikoKlare May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently it's only used for testing, but it may also validly be used to programmatically configure the behavior instead of using the system property. I would be fine with only making it available for tests but properly doing it would not be that easy with our test setup. We could mark it as @noreference though. What do you think?

Copy link
Copy Markdown
Member

@HannesWell HannesWell May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently it's only used for testing, but it may also validly be used to programmatically configure the behavior instead of using the system property.

If we definitively want to support that, then I think it makes sense to have this public. But if we don't or are not sure, then I suggest to not make it an 'API' and only elevate it when desired/necessary. I think it just should be a clear decision, but I'm not for one or the other.

doing it would not be that easy with our test setup. We could mark it as @noreference though.

Yes, in that case it we could do that or make it (package) private and just call it reflectively or set the field reflectively. But that's of course not very elegant.

/**
* The presentation mode.
*/
Expand Down Expand Up @@ -988,7 +1020,10 @@ private boolean updateImages(boolean forceImage) {
if (widget instanceof ToolItem) {
ImageDescriptor image = action.getImageDescriptor();
ImageDescriptor hoverImage = action.getHoverImageDescriptor();
ImageDescriptor disabledImage = action.getDisabledImageDescriptor();
ImageDescriptor disabledImage = null;
if (getUseDisabledIcons()) {
disabledImage = action.getDisabledImageDescriptor();
}
// Make sure there is a valid image in case images are forced.
if (image == null && forceImage) {
image = ImageDescriptor.getMissingImageDescriptor();
Expand Down
5 changes: 4 additions & 1 deletion bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/menus/CommandContributionItem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.eclipse.core.commands.common.NotDefinedException;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.Status;
import org.eclipse.jface.action.ActionContributionItem;
import org.eclipse.jface.action.ContributionItem;
import org.eclipse.jface.action.IContributionManager;
import org.eclipse.jface.action.IMenuListener2;
Expand Down Expand Up @@ -892,7 +893,9 @@ private void updateIcons() {
localResourceManager = m;
} else if (widget instanceof ToolItem item) {
LocalResourceManager m = new LocalResourceManager(JFaceResources.getResources());
item.setDisabledImage(disabledIcon == null ? null : m.create(disabledIcon));
if (ActionContributionItem.getUseDisabledIcons()) {
item.setDisabledImage(disabledIcon == null ? null : m.create(disabledIcon));
}
item.setHotImage(hoverIcon == null ? null : m.create(hoverIcon));
item.setImage(icon == null ? null : m.create(icon));
disposeOldImages();
Expand Down
6 changes: 6 additions & 0 deletions tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/action/ToolBarManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,10 @@ protected Control createControl(Composite parent) {
@Test
public void testDefaultImageIsGray() {
boolean oldState = ActionContributionItem.getUseColorIconsInToolbars();
boolean oldStateUseDisabled = ActionContributionItem.getUseDisabledIcons();
try {
ActionContributionItem.setUseColorIconsInToolbars(false);
ActionContributionItem.setUseDisabledIcons(true);
ToolBarManager manager = new ToolBarManager();
Action action = new Action("Button with Hover") {
};
Expand All @@ -165,14 +167,17 @@ public void testDefaultImageIsGray() {
assertImageEqualsDescriptor(ImageDescriptor.createWithFlags(descriptor, SWT.IMAGE_GRAY), item.getImage());
} finally {
ActionContributionItem.setUseColorIconsInToolbars(oldState);
ActionContributionItem.setUseDisabledIcons(oldStateUseDisabled);
}
}

@Test
public void testActionImagesAreSet() {
boolean oldState = ActionContributionItem.getUseColorIconsInToolbars();
boolean oldStateUseDisabled = ActionContributionItem.getUseDisabledIcons();
try {
ActionContributionItem.setUseColorIconsInToolbars(true);
ActionContributionItem.setUseDisabledIcons(true);
ToolBarManager manager = new ToolBarManager();
Action action = new Action("Button with Hover") {
};
Expand All @@ -195,6 +200,7 @@ public void testActionImagesAreSet() {
assertImageEqualsDescriptor(disabledDescriptor, item.getDisabledImage());
} finally {
ActionContributionItem.setUseColorIconsInToolbars(oldState);
ActionContributionItem.setUseDisabledIcons(oldStateUseDisabled);
}
}

Expand Down
Loading