From 4e6af9bf649b7476a93c0dcb5e6d14f287cde549 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 7 Nov 2025 14:44:02 +0000 Subject: [PATCH] Implement visibility tracking for regular toolbar items Fix https://github.com/eclipse-platform/eclipse.platform.ui/issues/3513 --- .../.settings/.api_filters | 193 ++++++++++++++++++ .../renderers/swt/ToolBarManagerRenderer.java | 120 ++++++++++- .../swt/ToolBarManagerRendererTest.java | 58 ++++++ 3 files changed, 362 insertions(+), 9 deletions(-) create mode 100644 bundles/org.eclipse.e4.ui.workbench.renderers.swt/.settings/.api_filters diff --git a/bundles/org.eclipse.e4.ui.workbench.renderers.swt/.settings/.api_filters b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/.settings/.api_filters new file mode 100644 index 00000000000..befc623ee67 --- /dev/null +++ b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/.settings/.api_filters @@ -0,0 +1,193 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/ToolBarManagerRenderer.java b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/ToolBarManagerRenderer.java index 3137704d139..a6b71cabbb7 100644 --- a/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/ToolBarManagerRenderer.java +++ b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/ToolBarManagerRenderer.java @@ -35,6 +35,7 @@ import java.util.List; import java.util.Map; import org.eclipse.core.expressions.ExpressionInfo; +import org.eclipse.e4.core.commands.ExpressionContext; import org.eclipse.e4.core.contexts.ContextInjectionFactory; import org.eclipse.e4.core.contexts.IContextFunction; import org.eclipse.e4.core.contexts.IEclipseContext; @@ -516,15 +517,7 @@ public boolean changed(IEclipseContext context) { record.updateVisibility(parentContext.getActiveLeaf()); runExternalCode(() -> { - manager.update(false); - getUpdater().updateContributionItems(e -> { - if (e instanceof MToolBarElement) { - if (((MUIElement) ((MToolBarElement) e).getParent()) == toolbarModel) { - return true; - } - } - return false; - }); + updateToolbar(toolbarModel, manager); }); return true; } @@ -627,9 +620,118 @@ public void processContents(MElementContainer container) { } } + // Set up visibility tracking for direct toolbar items with visibleWhen expressions + final MToolBar toolbarModel = (MToolBar) obj; + setupVisibilityTracking(toolbarModel, parentManager); + updateWidget(parentManager); } + /** + * Set up visibility tracking for direct toolbar items (not from contributions) + * that have visibleWhen expressions. + */ + private void setupVisibilityTracking(final MToolBar toolbarModel, final ToolBarManager manager) { + // Check if any direct children have visibleWhen expressions + List itemsWithVisibility = new ArrayList<>(); + for (MUIElement child : toolbarModel.getChildren()) { + if (child instanceof MToolBarElement element) { + if (requiresVisibilityCheck(element)) { + itemsWithVisibility.add(element); + } + } + } + + if (itemsWithVisibility.isEmpty()) { + return; + } + + // Collect all variable names that need to be tracked + ExpressionInfo info = new ExpressionInfo(); + for (MToolBarElement element : itemsWithVisibility) { + ContributionsAnalyzer.collectInfo(info, element.getVisibleWhen()); + String visibilityId = element.getPersistedState().get(MenuManagerRenderer.VISIBILITY_IDENTIFIER); + if (visibilityId != null) { + info.addVariableNameAccess(visibilityId); + } + } + updateVariables.addAll(Arrays.asList(info.getAccessedVariableNames())); + + final IEclipseContext parentContext = getContext(toolbarModel); + if (parentContext == null) { + return; + } + + parentContext.runAndTrack(new RunAndTrack() { + @Override + public boolean changed(IEclipseContext context) { + if (getManager(toolbarModel) == null) { + // tool bar no longer being managed, ignore it + return false; + } + + // Update visibility for all items with visibleWhen expressions + ExpressionContext exprContext = new ExpressionContext(parentContext.getActiveLeaf()); + boolean visibilityChanged = false; + for (MToolBarElement element : itemsWithVisibility) { + boolean newVisibility = computeElementVisibility(element, exprContext); + if (element.isVisible() != newVisibility) { + element.setVisible(newVisibility); + visibilityChanged = true; + } + } + + if (visibilityChanged) { + runExternalCode(() -> updateToolbar(toolbarModel, manager)); + } + return true; + } + }); + } + + /** + * Check if a toolbar element requires visibility checking + */ + private boolean requiresVisibilityCheck(MToolBarElement element) { + return element.getVisibleWhen() != null + || element.getPersistedState().get(MenuManagerRenderer.VISIBILITY_IDENTIFIER) != null; + } + + /** + * Compute visibility for a single toolbar element + */ + private boolean computeElementVisibility(MToolBarElement element, ExpressionContext exprContext) { + boolean visible = true; + + // Check persisted state visibility identifier + String identifier = element.getPersistedState().get(MenuManagerRenderer.VISIBILITY_IDENTIFIER); + if (identifier != null) { + Object rc = exprContext.eclipseContext.get(identifier); + if (rc instanceof Boolean) { + visible = ((Boolean) rc).booleanValue(); + } + } + + // Check visibleWhen expression + if (visible && element.getVisibleWhen() != null) { + visible = ContributionsAnalyzer.isVisible(element.getVisibleWhen(), exprContext); + } + + return visible; + } + + private void updateToolbar(final MToolBar toolbarModel, final ToolBarManager manager) { + manager.update(false); + getUpdater().updateContributionItems(e -> { + if (e instanceof MToolBarElement) { + if (((MUIElement) ((MToolBarElement) e).getParent()) == toolbarModel) { + return true; + } + } + return false; + }); + } + private void updateWidget(ToolBarManager manager) { manager.update(true); ToolBar toolbar = manager.getControl(); diff --git a/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/workbench/renderers/swt/ToolBarManagerRendererTest.java b/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/workbench/renderers/swt/ToolBarManagerRendererTest.java index 364146896b5..6f1b6724d96 100644 --- a/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/workbench/renderers/swt/ToolBarManagerRendererTest.java +++ b/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/workbench/renderers/swt/ToolBarManagerRendererTest.java @@ -21,13 +21,17 @@ import static org.junit.Assert.assertTrue; import jakarta.inject.Inject; +import jakarta.inject.Named; import java.util.ArrayList; import java.util.Arrays; import java.util.List; import org.eclipse.core.runtime.ILogListener; import org.eclipse.core.runtime.Platform; +import org.eclipse.e4.core.di.annotations.Evaluate; +import org.eclipse.e4.core.di.annotations.Optional; import org.eclipse.e4.core.services.events.IEventBroker; import org.eclipse.e4.ui.model.application.MApplication; +import org.eclipse.e4.ui.model.application.ui.MImperativeExpression; import org.eclipse.e4.ui.model.application.ui.basic.MTrimBar; import org.eclipse.e4.ui.model.application.ui.basic.MTrimmedWindow; import org.eclipse.e4.ui.model.application.ui.menu.MDirectToolItem; @@ -513,4 +517,58 @@ public void dispose() { } + @Test + public void testDirectToolItemWithVisibleWhen() { + // Create toolbar items with visibleWhen expressions + MToolItem toolItem1 = ems.createModelElement(MDirectToolItem.class); + toolItem1.setElementId("Item1"); + + // Create an imperative expression that checks for a context variable + MImperativeExpression exp1 = ems.createModelElement(MImperativeExpression.class); + exp1.setTracking(true); + exp1.setContributionURI("bundleclass://org.eclipse.e4.ui.tests/org.eclipse.e4.ui.workbench.renderers.swt.ToolBarManagerRendererTest$TestVisibilityExpression"); + toolItem1.setVisibleWhen(exp1); + toolBar.getChildren().add(toolItem1); + + MToolItem toolItem2 = ems.createModelElement(MDirectToolItem.class); + toolItem2.setElementId("Item2"); + toolBar.getChildren().add(toolItem2); + + contextRule.createAndRunWorkbench(window); + ToolBarManager tbm = getToolBarManager(); + + // Initially, item1 should be hidden (expression returns false when showItem1 is not set) + assertEquals(2, tbm.getSize()); + assertFalse("Item1 should be hidden initially", toolItem1.isVisible()); + assertTrue("Item2 should be visible", toolItem2.isVisible()); + + // Set context variable to show item1 + window.getContext().set("showItem1", Boolean.TRUE); + + // Force context update by spinning the event loop + contextRule.spinEventLoop(); + + // Now item1 should be visible + assertTrue("Item1 should be visible after setting context variable", toolItem1.isVisible()); + assertTrue("Item2 should still be visible", toolItem2.isVisible()); + + // Hide item1 again + window.getContext().set("showItem1", Boolean.FALSE); + contextRule.spinEventLoop(); + + // Item1 should be hidden again + assertFalse("Item1 should be hidden after removing context variable", toolItem1.isVisible()); + assertTrue("Item2 should still be visible", toolItem2.isVisible()); + } + + /** + * Test expression that checks for "showItem1" context variable + */ + public static class TestVisibilityExpression { + @Evaluate + public boolean evaluate(@Optional @Named("showItem1") Boolean showItem1) { + return Boolean.TRUE.equals(showItem1); + } + } + }