Align Commands in Onboarding Composite

Issue: eclipse-platform#753
marcushoepfner · May 17, 2023 · 3cd7114 · 3cd7114
1 parent 1e60584
commit 3cd7114
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 18 deletions.
diff --git a/....workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRenderer.java b/....workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRenderer.java
@@ -202,8 +202,6 @@ public class StackRenderer extends LazyStackRenderer {
 	private Composite onboardingComposite;
 	private Label onboardingText;
 	private Label onboardingImage;
-	private GridDataFactory onBoardingGridDataFactory = GridDataFactory
-			.create(GridData.VERTICAL_ALIGN_CENTER | GridData.HORIZONTAL_ALIGN_CENTER).grab(true, true);
 
 	List<CTabItem> getItemsToSet(MPart part) {
 		List<CTabItem> itemsToSet = new ArrayList<>();
@@ -271,10 +269,19 @@ private void setOnboarding(MPerspective perspective) {
 
 		Color color = JFaceResources.getColorRegistry().get(JFacePreferences.QUALIFIER_COLOR);
 
+		GridDataFactory labelGridDataFactory = GridDataFactory.swtDefaults().align(SWT.RIGHT, SWT.CENTER);
+		GridDataFactory commandGridDataFactory = GridDataFactory.swtDefaults().align(SWT.LEFT, SWT.CENTER);
+
 		for (int i = 0; i < commands.length; i++) {
-			onBoardingGridDataFactory.indent(SWT.DEFAULT, i == 0 ? 10 : SWT.DEFAULT);
-			WidgetFactory.label(SWT.NONE).text(commands[i]).foreground(color)
-					.supplyLayoutData(onBoardingGridDataFactory::create).create(onboardingComposite);
+			labelGridDataFactory.indent(SWT.DEFAULT, i == 0 ? 10 : SWT.DEFAULT);
+			commandGridDataFactory.indent(SWT.DEFAULT, i == 0 ? 10 : SWT.DEFAULT);
+
+			String[] commandAndText = commands[i].split("\\$\\$\\$"); //$NON-NLS-1$
+
+			WidgetFactory.label(SWT.NONE).text(commandAndText[0]).foreground(color)
+					.supplyLayoutData(labelGridDataFactory::create).create(onboardingComposite);
+			WidgetFactory.label(SWT.NONE).text(commandAndText[1]).foreground(color)
+					.supplyLayoutData(commandGridDataFactory::create).create(onboardingComposite);
 		}
 	}
 
@@ -699,11 +706,14 @@ private void createOnboardingControls(CTabFolder tabFolder) {
 		Composite onBoarding = WidgetFactory.composite(SWT.NONE).layout(GridLayoutFactory.swtDefaults().create())
 				.background(tabFolder.getBackground()).create(tabFolder);
 
-		onboardingComposite = WidgetFactory.composite(SWT.NONE).supplyLayoutData(onBoardingGridDataFactory::create)
-				.layout(GridLayoutFactory.swtDefaults().create()).background(tabFolder.getBackground())
+		onboardingComposite = WidgetFactory.composite(SWT.NONE).background(tabFolder.getBackground())
 				.create(onBoarding);
+		GridDataFactory.create(GridData.VERTICAL_ALIGN_CENTER | GridData.HORIZONTAL_ALIGN_CENTER).grab(true, true)
+				.applyTo(onboardingComposite);
+
+		GridLayoutFactory.swtDefaults().numColumns(2).equalWidth(true).applyTo(onboardingComposite);
 
-		GridDataFactory gridDataFactory = onBoardingGridDataFactory.copy().indent(SWT.DEFAULT, 10);
+		GridDataFactory gridDataFactory = GridDataFactory.swtDefaults().indent(SWT.DEFAULT, 10).span(2, 1);
 
 		onboardingImage = WidgetFactory.label(SWT.NONE).supplyLayoutData(gridDataFactory::create)
 				.create(onboardingComposite);

diff --git a/...e.ui.workbench/Eclipse UI/org/eclipse/ui/internal/e4/compatibility/ModeledPageLayout.java b/...e.ui.workbench/Eclipse UI/org/eclipse/ui/internal/e4/compatibility/ModeledPageLayout.java
@@ -80,6 +80,7 @@ public class ModeledPageLayout implements IPageLayout {
 	public static final String EDITOR_ONBOARDING_TEXT = EDITOR_ONBOARDING + "Text:"; //$NON-NLS-1$
 	public static final String EDITOR_ONBOARDING_IMAGE = EDITOR_ONBOARDING + "ImageUri:"; //$NON-NLS-1$
 	public static final String EDITOR_ONBOARDING_COMMAND = EDITOR_ONBOARDING + "Command:"; //$NON-NLS-1$
+	public static final String EDITOR_ONBOARDING_COMMAND_SEPARATOR = "$$$"; //$NON-NLS-1$
 
 	public static List<String> getIds(MPerspective model, String tagPrefix) {
 		if (model == null) {
@@ -643,14 +644,13 @@ public void addEditorOnboardingCommandId(String commandId) {
 		if (numberOfOnboardingCommands >= 5)
 			return;
 
-		String commandText = "{0}: {1}"; //$NON-NLS-1$
 		Predicate<MKeyBinding> commandWithEqualId = b -> b.getCommand().getElementId().equals(commandId);
 		Function<MKeyBinding, String> toCommandText = b -> {
 			try {
-				return NLS.bind(commandText, b.getCommand().getCommandName(),
-						KeySequence.getInstance(b.getKeySequence()).format());
+				return b.getCommand().getCommandName() + EDITOR_ONBOARDING_COMMAND_SEPARATOR
+						+ KeySequence.getInstance(b.getKeySequence()).format();
 			} catch (ParseException e) {
-				return NLS.bind(commandText, b.getCommand().getCommandName(), b.getKeySequence());
+				return b.getCommand().getCommandName() + EDITOR_ONBOARDING_COMMAND_SEPARATOR + b.getKeySequence();
 			}
 		};
 

diff --git a/....eclipse.e4.ui.tests/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRendererTest.java b/....eclipse.e4.ui.tests/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRendererTest.java
@@ -443,7 +443,7 @@ public void testOnboardingIsFilled() {
 		MPerspective perspective = ems.createModelElement(MPerspective.class);
 		perspective.getTags().add("persp.editorOnboardingText:Onboarding text");
 		perspective.getTags().add("persp.editorOnboardingImageUri:" + PART_ICON);
-		perspective.getTags().add("persp.editorOnboardingCommand:Find Actions: STRG+3");
+		perspective.getTags().add("persp.editorOnboardingCommand:Find Actions$$$STRG+3");
 
 		partStack.getTags().add("EditorStack");
 
@@ -461,10 +461,11 @@ public void testOnboardingIsFilled() {
 
 		Composite outerOnboardingComposite = (Composite) tabFolder.getChildren()[2];
 		Composite innerOnboardingComposite = (Composite) outerOnboardingComposite.getChildren()[0];
-		assertEquals(3, innerOnboardingComposite.getChildren().length);
+		assertEquals(4, innerOnboardingComposite.getChildren().length);
 		assertNotNull(((Label) innerOnboardingComposite.getChildren()[0]).getImage());
 		assertEquals("Onboarding text", ((Label) innerOnboardingComposite.getChildren()[1]).getText());
-		assertEquals("Find Actions: STRG+3", ((Label) innerOnboardingComposite.getChildren()[2]).getText());
+		assertEquals("Find Actions", ((Label) innerOnboardingComposite.getChildren()[2]).getText());
+		assertEquals("STRG+3", ((Label) innerOnboardingComposite.getChildren()[3]).getText());
 	}
 
 	@Test

diff --git a/...org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/api/ModeledPageLayoutTest.java b/...org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/api/ModeledPageLayoutTest.java
@@ -95,9 +95,9 @@ public void addsEditorOnboardingCommandIds() throws Exception {
 
 		assertThat(perspective.getTags(),
 				hasItems(
-						ModeledPageLayout.EDITOR_ONBOARDING_COMMAND + "Find Actions: "
+						ModeledPageLayout.EDITOR_ONBOARDING_COMMAND + "Find Actions$$$"
 								+ KeySequence.getInstance("M1+3").format(),
-						ModeledPageLayout.EDITOR_ONBOARDING_COMMAND + "Show Key Assist: "
+						ModeledPageLayout.EDITOR_ONBOARDING_COMMAND + "Show Key Assist$$$"
 								+ KeySequence.getInstance("M2+M1+L").format()));
 	}
 

diff --git a/...se.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/api/PerspectiveExtensionReaderTest.java b/...se.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/api/PerspectiveExtensionReaderTest.java
@@ -80,7 +80,7 @@ public void extendsOnboardingTags() throws Exception {
 		assertThat(perspective.getTags(), hasItems(//
 				EDITOR_ONBOARDING_TEXT + "The onboarding text", //
 				EDITOR_ONBOARDING_IMAGE + "platform:/plugin/org.eclipse.ui.tests/icons/anything.gif", //
-				EDITOR_ONBOARDING_COMMAND + "Find Actions: " + KeySequence.getInstance("M1+3").format()));
+				EDITOR_ONBOARDING_COMMAND + "Find Actions$$$" + KeySequence.getInstance("M1+3").format()));
 	}
 
 	private MKeyBinding createBinding(String elementId, String name, String keys) {