Skip to content

Commit 8a91995

Browse files
Copilotlaeubi
authored andcommitted
Implement visibility tracking for regular toolbar items
Fix #3513
1 parent 9e1b6e0 commit 8a91995

File tree

2 files changed

+169
-9
lines changed

2 files changed

+169
-9
lines changed

bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/ToolBarManagerRenderer.java

Lines changed: 111 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import java.util.List;
3636
import java.util.Map;
3737
import org.eclipse.core.expressions.ExpressionInfo;
38+
import org.eclipse.e4.core.commands.ExpressionContext;
3839
import org.eclipse.e4.core.contexts.ContextInjectionFactory;
3940
import org.eclipse.e4.core.contexts.IContextFunction;
4041
import org.eclipse.e4.core.contexts.IEclipseContext;
@@ -516,15 +517,7 @@ public boolean changed(IEclipseContext context) {
516517

517518
record.updateVisibility(parentContext.getActiveLeaf());
518519
runExternalCode(() -> {
519-
manager.update(false);
520-
getUpdater().updateContributionItems(e -> {
521-
if (e instanceof MToolBarElement) {
522-
if (((MUIElement) ((MToolBarElement) e).getParent()) == toolbarModel) {
523-
return true;
524-
}
525-
}
526-
return false;
527-
});
520+
updateToolbar(toolbarModel, manager);
528521
});
529522
return true;
530523
}
@@ -627,9 +620,118 @@ public void processContents(MElementContainer<MUIElement> container) {
627620
}
628621
}
629622

623+
// Set up visibility tracking for direct toolbar items with visibleWhen expressions
624+
final MToolBar toolbarModel = (MToolBar) obj;
625+
setupVisibilityTracking(toolbarModel, parentManager);
626+
630627
updateWidget(parentManager);
631628
}
632629

630+
/**
631+
* Set up visibility tracking for direct toolbar items (not from contributions)
632+
* that have visibleWhen expressions.
633+
*/
634+
private void setupVisibilityTracking(final MToolBar toolbarModel, final ToolBarManager manager) {
635+
// Check if any direct children have visibleWhen expressions
636+
List<MToolBarElement> itemsWithVisibility = new ArrayList<>();
637+
for (MUIElement child : toolbarModel.getChildren()) {
638+
if (child instanceof MToolBarElement element) {
639+
if (requiresVisibilityCheck(element)) {
640+
itemsWithVisibility.add(element);
641+
}
642+
}
643+
}
644+
645+
if (itemsWithVisibility.isEmpty()) {
646+
return;
647+
}
648+
649+
// Collect all variable names that need to be tracked
650+
ExpressionInfo info = new ExpressionInfo();
651+
for (MToolBarElement element : itemsWithVisibility) {
652+
ContributionsAnalyzer.collectInfo(info, element.getVisibleWhen());
653+
String visibilityId = element.getPersistedState().get(MenuManagerRenderer.VISIBILITY_IDENTIFIER);
654+
if (visibilityId != null) {
655+
info.addVariableNameAccess(visibilityId);
656+
}
657+
}
658+
updateVariables.addAll(Arrays.asList(info.getAccessedVariableNames()));
659+
660+
final IEclipseContext parentContext = getContext(toolbarModel);
661+
if (parentContext == null) {
662+
return;
663+
}
664+
665+
parentContext.runAndTrack(new RunAndTrack() {
666+
@Override
667+
public boolean changed(IEclipseContext context) {
668+
if (getManager(toolbarModel) == null) {
669+
// tool bar no longer being managed, ignore it
670+
return false;
671+
}
672+
673+
// Update visibility for all items with visibleWhen expressions
674+
ExpressionContext exprContext = new ExpressionContext(parentContext.getActiveLeaf());
675+
boolean visibilityChanged = false;
676+
for (MToolBarElement element : itemsWithVisibility) {
677+
boolean newVisibility = computeElementVisibility(element, exprContext);
678+
if (element.isVisible() != newVisibility) {
679+
element.setVisible(newVisibility);
680+
visibilityChanged = true;
681+
}
682+
}
683+
684+
if (visibilityChanged) {
685+
runExternalCode(() -> updateToolbar(toolbarModel, manager));
686+
}
687+
return true;
688+
}
689+
});
690+
}
691+
692+
/**
693+
* Check if a toolbar element requires visibility checking
694+
*/
695+
private boolean requiresVisibilityCheck(MToolBarElement element) {
696+
return element.getVisibleWhen() != null
697+
|| element.getPersistedState().get(MenuManagerRenderer.VISIBILITY_IDENTIFIER) != null;
698+
}
699+
700+
/**
701+
* Compute visibility for a single toolbar element
702+
*/
703+
private boolean computeElementVisibility(MToolBarElement element, ExpressionContext exprContext) {
704+
boolean visible = true;
705+
706+
// Check persisted state visibility identifier
707+
String identifier = element.getPersistedState().get(MenuManagerRenderer.VISIBILITY_IDENTIFIER);
708+
if (identifier != null) {
709+
Object rc = exprContext.eclipseContext.get(identifier);
710+
if (rc instanceof Boolean) {
711+
visible = ((Boolean) rc).booleanValue();
712+
}
713+
}
714+
715+
// Check visibleWhen expression
716+
if (visible && element.getVisibleWhen() != null) {
717+
visible = ContributionsAnalyzer.isVisible(element.getVisibleWhen(), exprContext);
718+
}
719+
720+
return visible;
721+
}
722+
723+
private void updateToolbar(final MToolBar toolbarModel, final ToolBarManager manager) {
724+
manager.update(false);
725+
getUpdater().updateContributionItems(e -> {
726+
if (e instanceof MToolBarElement) {
727+
if (((MUIElement) ((MToolBarElement) e).getParent()) == toolbarModel) {
728+
return true;
729+
}
730+
}
731+
return false;
732+
});
733+
}
734+
633735
private void updateWidget(ToolBarManager manager) {
634736
manager.update(true);
635737
ToolBar toolbar = manager.getControl();

tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/workbench/renderers/swt/ToolBarManagerRendererTest.java

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,17 @@
2121
import static org.junit.Assert.assertTrue;
2222

2323
import jakarta.inject.Inject;
24+
import jakarta.inject.Named;
2425
import java.util.ArrayList;
2526
import java.util.Arrays;
2627
import java.util.List;
2728
import org.eclipse.core.runtime.ILogListener;
2829
import org.eclipse.core.runtime.Platform;
30+
import org.eclipse.e4.core.di.annotations.Evaluate;
31+
import org.eclipse.e4.core.di.annotations.Optional;
2932
import org.eclipse.e4.core.services.events.IEventBroker;
3033
import org.eclipse.e4.ui.model.application.MApplication;
34+
import org.eclipse.e4.ui.model.application.ui.MImperativeExpression;
3135
import org.eclipse.e4.ui.model.application.ui.basic.MTrimBar;
3236
import org.eclipse.e4.ui.model.application.ui.basic.MTrimmedWindow;
3337
import org.eclipse.e4.ui.model.application.ui.menu.MDirectToolItem;
@@ -513,4 +517,58 @@ public void dispose() {
513517

514518
}
515519

520+
@Test
521+
public void testDirectToolItemWithVisibleWhen() {
522+
// Create toolbar items with visibleWhen expressions
523+
MToolItem toolItem1 = ems.createModelElement(MDirectToolItem.class);
524+
toolItem1.setElementId("Item1");
525+
526+
// Create an imperative expression that checks for a context variable
527+
MImperativeExpression exp1 = ems.createModelElement(MImperativeExpression.class);
528+
exp1.setTracking(true);
529+
exp1.setContributionURI("bundleclass://org.eclipse.e4.ui.tests/org.eclipse.e4.ui.workbench.renderers.swt.ToolBarManagerRendererTest$TestVisibilityExpression");
530+
toolItem1.setVisibleWhen(exp1);
531+
toolBar.getChildren().add(toolItem1);
532+
533+
MToolItem toolItem2 = ems.createModelElement(MDirectToolItem.class);
534+
toolItem2.setElementId("Item2");
535+
toolBar.getChildren().add(toolItem2);
536+
537+
contextRule.createAndRunWorkbench(window);
538+
ToolBarManager tbm = getToolBarManager();
539+
540+
// Initially, item1 should be hidden (expression returns false when showItem1 is not set)
541+
assertEquals(2, tbm.getSize());
542+
assertFalse("Item1 should be hidden initially", toolItem1.isVisible());
543+
assertTrue("Item2 should be visible", toolItem2.isVisible());
544+
545+
// Set context variable to show item1
546+
window.getContext().set("showItem1", Boolean.TRUE);
547+
548+
// Force context update by spinning the event loop
549+
contextRule.spinEventLoop();
550+
551+
// Now item1 should be visible
552+
assertTrue("Item1 should be visible after setting context variable", toolItem1.isVisible());
553+
assertTrue("Item2 should still be visible", toolItem2.isVisible());
554+
555+
// Hide item1 again
556+
window.getContext().set("showItem1", Boolean.FALSE);
557+
contextRule.spinEventLoop();
558+
559+
// Item1 should be hidden again
560+
assertFalse("Item1 should be hidden after removing context variable", toolItem1.isVisible());
561+
assertTrue("Item2 should still be visible", toolItem2.isVisible());
562+
}
563+
564+
/**
565+
* Test expression that checks for "showItem1" context variable
566+
*/
567+
public static class TestVisibilityExpression {
568+
@Evaluate
569+
public boolean evaluate(@Optional @Named("showItem1") Boolean showItem1) {
570+
return Boolean.TRUE.equals(showItem1);
571+
}
572+
}
573+
516574
}

0 commit comments

Comments
 (0)