Skip to content

Commit

Permalink
Fix onboarding - don't keep onboarding related controls as state (ecl…
Browse files Browse the repository at this point in the history 
…ipse-platform#1400)

Issue: eclipse-platform#1180

The StackRenderer may be used several times e.g. when a second Eclipse
window is opened via Menu -> Window -> new Window.
When this second window is closed, the onboarding controls are disposed.
When these controls have been kept as state (fields), the logic does not
work anymore any strange ui behaviour can be observed. E.g. Onboarding
controls are visible even when an editor is open.

Solution: remove the state and get the controls from the model element.
In order to find them easier the controls store an ID in their data
attribute.
  • Loading branch information
@marcushoepfner
marcushoepfner authored Dec 13, 2023
1 parent a7df241 commit 9029bea
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 21 deletions.
89 changes: 70 additions & 19 deletions ....workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRenderer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.Map.Entry;
import java.util.Set;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Predicate;
import org.eclipse.core.runtime.preferences.IEclipsePreferences;
import org.eclipse.e4.core.contexts.IEclipseContext;
Expand Down Expand Up @@ -120,6 +121,17 @@
* IPresentation.STYLE_OVERRIDE_KEY key
*/
public class StackRenderer extends LazyStackRenderer {

private static final String ONBOARDING_CONTAINER = "EditorStack.OnboardingContainer"; //$NON-NLS-1$
private static final String ONBOARDING_COMPOSITE = "EditorStack.OnboardingComposite"; //$NON-NLS-1$
private static final String ONBOARDING_IMAGE = "EditorStack.OnboardingImage"; //$NON-NLS-1$
private static final String ONBOARDING_TEXT = "EditorStack.OnboardingText"; //$NON-NLS-1$

/**
* Id of a a control.
*/
private static final String ID = "id"; //$NON-NLS-1$

private static final String THE_PART_KEY = "thePart"; //$NON-NLS-1$

/**
Expand Down Expand Up @@ -201,10 +213,6 @@ public class StackRenderer extends LazyStackRenderer {

private boolean imageChanged;

private Composite onboardingComposite;
private Label onboardingText;
private Label onboardingImage;

List<CTabItem> getItemsToSet(MPart part) {
List<CTabItem> itemsToSet = new ArrayList<>();

Expand Down Expand Up @@ -249,13 +257,31 @@ void subscribePerspectiveSwitched(
}

private void setOnboarding(MPerspective perspective) {
if (onboardingImage == null || onboardingText == null || onboardingComposite == null
|| onboardingComposite.isDisposed()) {
CTabFolder tabFolder = getEditorTabFolder(perspective);
if (tabFolder == null) {
return;
}

Composite onboardingContainer = getChild(tabFolder, ONBOARDING_CONTAINER, Composite.class);
if (onboardingContainer == null || onboardingContainer.isDisposed()) {
return;
}

Composite onboardingComposite = getChild(onboardingContainer, ONBOARDING_COMPOSITE, Composite.class);
if (onboardingComposite == null || onboardingComposite.isDisposed()) {
return;
}

Arrays.stream(onboardingComposite.getChildren()).filter(c -> c != onboardingImage)
.filter(c -> c != onboardingText).forEach(Control::dispose);
Label onboardingImage = getChild(onboardingComposite, ONBOARDING_IMAGE, Label.class);
Label onboardingText = getChild(onboardingComposite, ONBOARDING_TEXT, Label.class);

if (onboardingImage == null || onboardingImage.isDisposed() || onboardingText == null
|| onboardingText.isDisposed()) {
return;
}

Arrays.stream(onboardingComposite.getChildren())
.filter(c -> c != onboardingImage).filter(c -> c != onboardingText).forEach(Control::dispose);

String textId = "persp.editorOnboardingText:"; //$NON-NLS-1$
perspective.getTags().stream().filter(tag -> tag.startsWith(textId)).map(tag -> tag.substring(textId.length()))
Expand Down Expand Up @@ -704,32 +730,38 @@ public Object createWidget(MUIElement element, Object parent) {
}

private void createOnboardingControls(CTabFolder tabFolder) {
Composite onBoarding = WidgetFactory.composite(SWT.NONE).layout(GridLayoutFactory.swtDefaults().create())
.background(tabFolder.getBackground()).create(tabFolder);
Composite onBoardingContainer = WidgetFactory.composite(SWT.NONE)
.layout(GridLayoutFactory.swtDefaults().create()).background(tabFolder.getBackground())
.create(tabFolder);
onBoardingContainer.setData(ID, ONBOARDING_CONTAINER);

onboardingComposite = WidgetFactory.composite(SWT.NONE).background(tabFolder.getBackground())
.create(onBoarding);
Composite onboardingComposite = WidgetFactory.composite(SWT.NONE).background(tabFolder.getBackground())
.create(onBoardingContainer);
GridDataFactory.create(GridData.VERTICAL_ALIGN_CENTER | GridData.HORIZONTAL_ALIGN_CENTER).grab(true, true)
.applyTo(onboardingComposite);
onboardingComposite.setData(ID, ONBOARDING_COMPOSITE);

GridLayoutFactory.swtDefaults().numColumns(2).equalWidth(true).spacing(10, SWT.DEFAULT)
.applyTo(onboardingComposite);

GridDataFactory gridDataFactory = GridDataFactory.swtDefaults().align(SWT.CENTER, SWT.CENTER)
.indent(SWT.DEFAULT, 10).span(2, 1);

onboardingImage = WidgetFactory.label(SWT.NONE).supplyLayoutData(gridDataFactory::create)
Label onboardingImage = WidgetFactory.label(SWT.NONE).supplyLayoutData(gridDataFactory::create)
.create(onboardingComposite);
onboardingImage.setData(ID, ONBOARDING_IMAGE);

Color color = JFaceResources.getColorRegistry().get(JFacePreferences.QUALIFIER_COLOR);
onboardingText = WidgetFactory.label(SWT.NONE).foreground(color).supplyLayoutData(gridDataFactory::create)
Label onboardingText = WidgetFactory.label(SWT.NONE).foreground(color).supplyLayoutData(gridDataFactory::create)
.create(onboardingComposite);
onboardingText.setData(ID, ONBOARDING_TEXT);

onBoarding.setLocation(ONBOARDING_SPACING, ONBOARDING_TOP_SPACING);
Consumer<ControlEvent> sizeUpdate = e -> setOnboardingControlSize(tabFolder, onBoarding, onboardingImage);
onBoardingContainer.setLocation(ONBOARDING_SPACING, ONBOARDING_TOP_SPACING);
Consumer<ControlEvent> sizeUpdate = e -> setOnboardingControlSize(tabFolder, onBoardingContainer,
onboardingComposite, onboardingImage);
tabFolder.addControlListener(ControlListener.controlResizedAdapter(sizeUpdate));
Consumer<CTabFolderEvent> tabCountUpdate = e -> setOnboardingControlSize(tabFolder, onBoarding,
onboardingImage);
Consumer<CTabFolderEvent> tabCountUpdate = e -> setOnboardingControlSize(tabFolder, onBoardingContainer,
onboardingComposite, onboardingImage);
tabFolder.addCTabFolder2Listener(CTabFolder2Listener.itemsCountAdapter(tabCountUpdate));

tabFolder.addDisposeListener(e -> {
Expand All @@ -740,7 +772,8 @@ private void createOnboardingControls(CTabFolder tabFolder) {
});
}

private void setOnboardingControlSize(CTabFolder tabFolder, Composite onBoarding, Label onBoardingImage) {
private void setOnboardingControlSize(CTabFolder tabFolder, Composite onBoarding, Composite onboardingComposite,
Label onBoardingImage) {
if (onBoarding == null || onBoarding.isDisposed() || tabFolder == null || tabFolder.isDisposed()
|| onBoardingImage == null || onBoardingImage.isDisposed()) {
return;
Expand Down Expand Up @@ -1881,4 +1914,22 @@ static void updateBusyStateNoCss(CTabItem cti, Object newValue, Object oldValue)
protected boolean imageChanged() {
return this.imageChanged;
}

private CTabFolder getEditorTabFolder(MPerspective perspective) {
Predicate<Object> tabFolders = CTabFolder.class::isInstance;
Function<Object, CTabFolder> toTabFolder = CTabFolder.class::cast;

List<MPartStack> elements = modelService.findElements(perspective, null, MPartStack.class,
List.of("EditorStack")); //$NON-NLS-1$
return elements.stream().map(MUIElement::getWidget).filter(tabFolders).map(toTabFolder).findFirst()
.orElse(null);
}

private <T extends Control> T getChild(Composite parent, String id, Class<T> type) {
if (parent == null || parent.isDisposed()) {
return null;
}
return Arrays.stream(parent.getChildren()).filter(child -> id.equals(child.getData(ID)))
.filter(type::isInstance).map(type::cast).findFirst().orElse(null);
}
}
13 changes: 11 additions & 2 deletions ....eclipse.e4.ui.tests/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRendererTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.eclipse.e4.ui.model.application.MApplication;
import org.eclipse.e4.ui.model.application.descriptor.basic.MPartDescriptor;
import org.eclipse.e4.ui.model.application.ui.advanced.MPerspective;
import org.eclipse.e4.ui.model.application.ui.advanced.MPerspectiveStack;
import org.eclipse.e4.ui.model.application.ui.advanced.MPlaceholder;
import org.eclipse.e4.ui.model.application.ui.basic.MPart;
import org.eclipse.e4.ui.model.application.ui.basic.MPartStack;
Expand Down Expand Up @@ -448,6 +449,15 @@ public void testOnboardingIsFilled() {
perspective.getTags().add("persp.editorOnboardingImageUri:" + PART_ICON);
perspective.getTags().add("persp.editorOnboardingCommand:Find Actions$$$STRG+3");

// "connect" the perspective with the application more or less like in
// productive environment
MPerspectiveStack perspectiveStack = ems.createModelElement(MPerspectiveStack.class);
perspectiveStack.getChildren().add(perspective);
window.getChildren().add(perspectiveStack);
MPlaceholder placeholder = ems.createModelElement(MPlaceholder.class);
placeholder.setRef(partStack);
perspective.getChildren().add(placeholder);

partStack.getTags().add("EditorStack");

contextRule.createAndRunWorkbench(window);
Expand All @@ -457,8 +467,7 @@ public void testOnboardingIsFilled() {

context.get(EventBroker.class).send(UIEvents.UILifeCycle.PERSPECTIVE_SWITCHED, params);

Composite uiContainer = (Composite) ((StackRenderer) partStack.getRenderer()).getUIContainer(partStack);
CTabFolder tabFolder = (CTabFolder) ((Composite) uiContainer.getChildren()[0]).getChildren()[0];
CTabFolder tabFolder = (CTabFolder) partStack.getWidget();
assertNotNull(tabFolder.getChildren());
assertEquals(3, tabFolder.getChildren().length);

Expand Down

0 comments on commit 9029bea

Please sign in to comment.