[550436] Fix tabs order algorithm
The algorithm processing 'afterTab' elements was bugged and leaded to
have wrong tabs order. Also add test cases.
Bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=550436
Change-Id: Idf802fdc0a0812486d279395751d138e0b3ccf50
Signed-off-by: Axel RICHARD <axel.richard@obeo.fr>
diff --git a/plugins/org.eclipse.eef.properties.ui.legacy/src/org/eclipse/eef/properties/ui/legacy/internal/extension/impl/LegacyPropertyTabRegistry.java b/plugins/org.eclipse.eef.properties.ui.legacy/src/org/eclipse/eef/properties/ui/legacy/internal/extension/impl/LegacyPropertyTabRegistry.java
index ef9a6d5..a3e9839 100644
--- a/plugins/org.eclipse.eef.properties.ui.legacy/src/org/eclipse/eef/properties/ui/legacy/internal/extension/impl/LegacyPropertyTabRegistry.java
+++ b/plugins/org.eclipse.eef.properties.ui.legacy/src/org/eclipse/eef/properties/ui/legacy/internal/extension/impl/LegacyPropertyTabRegistry.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2015, 2018 Obeo.
+ * Copyright (c) 2015, 2019 Obeo.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v2.0
* which accompanies this distribution, and is available at
@@ -16,7 +16,6 @@
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
-import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@@ -36,11 +35,6 @@
public class LegacyPropertyTabRegistry implements IItemRegistry {
/**
- * The after tab top value.
- */
- private static final String TOP = "top"; //$NON-NLS-1$
-
- /**
* The map of the identifier of the description to the {@link LegacyPropertyTabItemDescriptor}.
*/
private Map<String, List<IItemDescriptor>> id2descriptors = new HashMap<>();
@@ -112,31 +106,21 @@
return tabs;
}
List<IEEFTabDescriptor> sorted = new ArrayList<IEEFTabDescriptor>();
- int categoryIndex = 0;
for (int i = 0; i < propertyCategories.size(); i++) {
// Get all the tabs of a category
final List<IEEFTabDescriptor> categoryList = new ArrayList<IEEFTabDescriptor>();
String category = propertyCategories.get(i);
- int topOfCategory = categoryIndex;
- int endOfCategory = categoryIndex;
- while (endOfCategory < tabs.size() && tabs.get(endOfCategory).getCategory().equals(category)) {
- endOfCategory++;
- }
- for (int j = topOfCategory; j < endOfCategory; j++) {
- IEEFTabDescriptor tab = tabs.get(j);
- if (tab.getAfterTab().equals(TOP)) {
- categoryList.add(0, tabs.get(j));
- } else {
- categoryList.add(tabs.get(j));
+
+ for (IEEFTabDescriptor tab : tabs) {
+ if (category.equals(tab.getCategory())) {
+ categoryList.add(tab);
}
}
-
List<IEEFTabDescriptor> sortedTabs = sortCategoryTabsByAfterTab(categoryList);
for (int j = 0; j < sortedTabs.size(); j++) {
sorted.add(sortedTabs.get(j));
}
- categoryIndex = endOfCategory;
}
return sorted;
}
@@ -149,37 +133,7 @@
* @return List of tabs sorted by after tab
*/
private List<IEEFTabDescriptor> sortCategoryTabsByAfterTab(List<IEEFTabDescriptor> categoryList) {
- List<IEEFTabDescriptor> sorted = new LinkedList<IEEFTabDescriptor>();
- for (IEEFTabDescriptor tab : categoryList) {
- String afterTabId = tab.getAfterTab();
- int indexOfAfterTab = -1;
- if (afterTabId != null && !afterTabId.isEmpty()) {
- IEEFTabDescriptor afterTab = getTab(categoryList, afterTabId);
- if (afterTab != null) {
- indexOfAfterTab = sorted.indexOf(afterTab);
- }
- }
- sorted.add(indexOfAfterTab + 1, tab);
- }
- return sorted;
- }
-
- /**
- * Get a tab in a list of tabs according to its id.
- *
- * @param tabId
- * The tab id
- * @param tabs
- * The tabs list
- * @return the Tab
- */
- private IEEFTabDescriptor getTab(List<IEEFTabDescriptor> tabs, String tabId) {
- for (IEEFTabDescriptor tab : tabs) {
- if (tabId.equals(tab.getId())) {
- return tab;
- }
- }
- return null;
+ return new LegacyPropertyTabSorter().sortTabsByAfterTab(categoryList);
}
/**
diff --git a/plugins/org.eclipse.eef.properties.ui.legacy/src/org/eclipse/eef/properties/ui/legacy/internal/extension/impl/LegacyPropertyTabSorter.java b/plugins/org.eclipse.eef.properties.ui.legacy/src/org/eclipse/eef/properties/ui/legacy/internal/extension/impl/LegacyPropertyTabSorter.java
new file mode 100644
index 0000000..738ef66
--- /dev/null
+++ b/plugins/org.eclipse.eef.properties.ui.legacy/src/org/eclipse/eef/properties/ui/legacy/internal/extension/impl/LegacyPropertyTabSorter.java
@@ -0,0 +1,109 @@
+/*******************************************************************************
+ * Copyright (c) 2019 Obeo.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors: Obeo - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.eef.properties.ui.legacy.internal.extension.impl;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+import org.eclipse.eef.properties.ui.api.IEEFTabDescriptor;
+
+/**
+ * Utility class used to sort the property tabs.
+ *
+ * @author arichard
+ */
+public class LegacyPropertyTabSorter {
+
+ /**
+ * Sort tabs by after tab. Top tabs and tabs without after tab are put at the beginning of the list as they appear.
+ *
+ * @param tabs
+ * List of tabs
+ * @return List of tabs sorted by after tab
+ */
+ public List<IEEFTabDescriptor> sortTabsByAfterTab(Collection<IEEFTabDescriptor> tabs) {
+ List<IEEFTabDescriptor> sorted = new ArrayList<IEEFTabDescriptor>(tabs);
+ List<IEEFTabDescriptor> processed = new ArrayList<IEEFTabDescriptor>();
+ int lastIndex = sorted.size() - 1;
+
+ for (int i = 0; i <= lastIndex; i++) {
+ IEEFTabDescriptor lastUnprocessedTab = getLastUnprocessedTab(sorted, processed);
+ if (lastUnprocessedTab != null) {
+ String lastUnprocessedTabId = lastUnprocessedTab.getId();
+ if (!isNullEmpty(lastUnprocessedTabId)) {
+ for (IEEFTabDescriptor tab : tabs) {
+ if (lastUnprocessedTabId.equals(tab.getAfterTab())) {
+ sorted.remove(tab);
+ sorted.add(sorted.indexOf(lastUnprocessedTab) + 1, tab);
+ }
+ }
+ }
+ processed.add(lastUnprocessedTab);
+ }
+ }
+
+ // Move top tabs and tabs without afterTab at the beginning of the list
+ int index = 0;
+ for (IEEFTabDescriptor tab : tabs) {
+ String afterTab = tab.getAfterTab();
+ if (isNullEmpty(afterTab) || isTop(afterTab)) {
+ sorted.remove(tab);
+ sorted.add(index, tab);
+ index++;
+ }
+ }
+
+ return sorted;
+ }
+
+ /**
+ * Get the last tab that has not been processed yet by {@link #sortTabsByAfterTab(Collection)}.
+ *
+ * @param tabs
+ * The list of tabs.
+ * @param processed
+ * The list of already processed tabs.
+ * @return The last tab that has not been processed yet by {@link #sortTabsByAfterTab(Collection)}
+ */
+ private IEEFTabDescriptor getLastUnprocessedTab(List<IEEFTabDescriptor> tabs, List<IEEFTabDescriptor> processed) {
+ for (int i = tabs.size() - 1; i >= 0; i--) {
+ IEEFTabDescriptor lastUnprocessedTab = tabs.get(i);
+ if (!processed.contains(lastUnprocessedTab)) {
+ return lastUnprocessedTab;
+ }
+ }
+ return null;
+ }
+
+ /**
+ * Indicates if the given id is null, or an empty string.
+ *
+ * @param id
+ * The id
+ * @return <code>true</code> if the id is null, or an empty string
+ */
+ private boolean isNullEmpty(String id) {
+ return id == null || id.isEmpty();
+ }
+
+ /**
+ * Indicates if the given after tab is top.
+ *
+ * @param afterTab
+ * The after tab
+ * @return <code>true</code> if the after tab is top
+ */
+ private boolean isTop(String afterTab) {
+ return "top".equals(afterTab); //$NON-NLS-1$
+ }
+}
diff --git a/tests/org.eclipse.eef.ui.tests/src/org/eclipse/eef/ui/tests/internal/AllUiTests.java b/tests/org.eclipse.eef.ui.tests/src/org/eclipse/eef/ui/tests/internal/AllUiTests.java
index e6e61c5..bfa1bee 100644
--- a/tests/org.eclipse.eef.ui.tests/src/org/eclipse/eef/ui/tests/internal/AllUiTests.java
+++ b/tests/org.eclipse.eef.ui.tests/src/org/eclipse/eef/ui/tests/internal/AllUiTests.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2018 Obeo.
+ * Copyright (c) 2018, 2019 Obeo.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v2.0
* which accompanies this distribution, and is available at
@@ -21,7 +21,7 @@
* @author sbegaudeau
*/
@RunWith(Suite.class)
-@SuiteClasses({ LegacyPropertySectionSorterTests.class })
+@SuiteClasses({ LegacyPropertySectionSorterTests.class, LegacyPropertyTabSorterTests.class })
public final class AllUiTests {
/**
* The constructor.
diff --git a/tests/org.eclipse.eef.ui.tests/src/org/eclipse/eef/ui/tests/internal/LegacyPropertyTabSorterTests.java b/tests/org.eclipse.eef.ui.tests/src/org/eclipse/eef/ui/tests/internal/LegacyPropertyTabSorterTests.java
new file mode 100644
index 0000000..1c99271
--- /dev/null
+++ b/tests/org.eclipse.eef.ui.tests/src/org/eclipse/eef/ui/tests/internal/LegacyPropertyTabSorterTests.java
@@ -0,0 +1,226 @@
+/*******************************************************************************
+ * Copyright (c) 2019 Obeo.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors: Obeo - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.eef.ui.tests.internal;
+
+import static org.junit.Assert.assertEquals;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.eclipse.eef.properties.ui.api.EEFTabContents;
+import org.eclipse.eef.properties.ui.api.IEEFSectionDescriptor;
+import org.eclipse.eef.properties.ui.api.IEEFTabDescriptor;
+import org.eclipse.eef.properties.ui.legacy.internal.extension.impl.LegacyPropertyTabSorter;
+import org.eclipse.swt.graphics.Image;
+import org.junit.Test;
+
+/**
+ * Test class for the LegacyPropertyTabSorter.
+ *
+ * @author arichard
+ */
+@SuppressWarnings({ "checkstyle:javadocmethod" })
+public class LegacyPropertyTabSorterTests {
+
+ /**
+ * The special after tab for the TOP tab.
+ */
+ private static final String TOP = "top"; //$NON-NLS-1$
+
+ /**
+ * Utility class to test the tab descriptor.
+ *
+ * @author arichard
+ */
+ private static class TestTabDescriptor implements IEEFTabDescriptor {
+
+ /**
+ * The id.
+ */
+ private String id;
+
+ /**
+ * The after tab.
+ */
+ private String afterTab;
+
+ /**
+ * The constructor.
+ *
+ * @param id
+ * The id
+ * @param afterTab
+ * The after tab
+ */
+ TestTabDescriptor(String id, String afterTab) {
+ this.id = id;
+ this.afterTab = afterTab;
+ }
+
+ @Override
+ public String getId() {
+ return this.id;
+ }
+
+ @Override
+ public String getAfterTab() {
+ return this.afterTab;
+ }
+
+ @Override
+ public String toString() {
+ return this.id;
+ }
+
+ @Override
+ public Image getImage() {
+ return null;
+ }
+
+ @Override
+ public String getText() {
+ return null;
+ }
+
+ @Override
+ public boolean isSelected() {
+ return false;
+ }
+
+ @Override
+ public boolean isIndented() {
+ return false;
+ }
+
+ @Override
+ public EEFTabContents createTab() {
+ return null;
+ }
+
+ @Override
+ public String getCategory() {
+ return null;
+ }
+
+ @Override
+ public String getLabel() {
+ return null;
+ }
+
+ @Override
+ public List<IEEFSectionDescriptor> getSectionDescriptors() {
+ return null;
+ }
+
+ }
+
+ @Test
+ public void testOrderedTabs() {
+ List<IEEFTabDescriptor> tabs = new ArrayList<>();
+ tabs.add(new TestTabDescriptor("1", null)); //$NON-NLS-1$
+ tabs.add(new TestTabDescriptor("2", null)); //$NON-NLS-1$
+ tabs.add(new TestTabDescriptor("3", null)); //$NON-NLS-1$
+
+ List<IEEFTabDescriptor> sortedTabs = new LegacyPropertyTabSorter().sortTabsByAfterTab(tabs);
+
+ assertEquals("[1, 2, 3]", sortedTabs.toString()); //$NON-NLS-1$
+ }
+
+ @Test
+ public void testOrderedTabsWithAfterTab() {
+ List<IEEFTabDescriptor> tabs = new ArrayList<>();
+ tabs.add(new TestTabDescriptor("4", "6")); //$NON-NLS-1$ //$NON-NLS-2$
+ tabs.add(new TestTabDescriptor("5", TOP)); //$NON-NLS-1$
+ tabs.add(new TestTabDescriptor("6", "5")); //$NON-NLS-1$ //$NON-NLS-2$
+
+ List<IEEFTabDescriptor> sortedTabs = new LegacyPropertyTabSorter().sortTabsByAfterTab(tabs);
+
+ assertEquals("[5, 6, 4]", sortedTabs.toString()); //$NON-NLS-1$
+ }
+
+ @Test
+ public void testOrderedTabsWithAfterTab2() {
+ List<IEEFTabDescriptor> tabs = new ArrayList<>();
+ tabs.add(new TestTabDescriptor("9", "10")); //$NON-NLS-1$ //$NON-NLS-2$
+ tabs.add(new TestTabDescriptor("7", "8")); //$NON-NLS-1$ //$NON-NLS-2$
+ tabs.add(new TestTabDescriptor("8", "9")); //$NON-NLS-1$ //$NON-NLS-2$
+ tabs.add(new TestTabDescriptor("10", TOP)); //$NON-NLS-1$
+
+ List<IEEFTabDescriptor> sortedTabs = new LegacyPropertyTabSorter().sortTabsByAfterTab(tabs);
+
+ assertEquals("[10, 9, 8, 7]", sortedTabs.toString()); //$NON-NLS-1$
+ }
+
+ @Test
+ public void testOrderedTabsWithAfterTab3() {
+ List<IEEFTabDescriptor> tabs = new ArrayList<>();
+ tabs.add(new TestTabDescriptor("17", TOP)); //$NON-NLS-1$
+ tabs.add(new TestTabDescriptor("16", "17")); //$NON-NLS-1$ //$NON-NLS-2$
+ tabs.add(new TestTabDescriptor("13", "14")); //$NON-NLS-1$ //$NON-NLS-2$
+ tabs.add(new TestTabDescriptor("14", "15")); //$NON-NLS-1$ //$NON-NLS-2$
+ tabs.add(new TestTabDescriptor("15", "16")); //$NON-NLS-1$ //$NON-NLS-2$
+ tabs.add(new TestTabDescriptor("12", "13")); //$NON-NLS-1$ //$NON-NLS-2$
+ tabs.add(new TestTabDescriptor("11", "12")); //$NON-NLS-1$ //$NON-NLS-2$
+
+ List<IEEFTabDescriptor> sortedTabs = new LegacyPropertyTabSorter().sortTabsByAfterTab(tabs);
+
+ assertEquals("[17, 16, 15, 14, 13, 12, 11]", sortedTabs.toString()); //$NON-NLS-1$
+ }
+
+ @Test
+ public void testOrderedTabsWithAfterTab4() {
+ List<IEEFTabDescriptor> tabs = new ArrayList<>();
+ tabs.add(new TestTabDescriptor("1", null)); //$NON-NLS-1$
+ tabs.add(new TestTabDescriptor("2", "1")); //$NON-NLS-1$ //$NON-NLS-2$
+ tabs.add(new TestTabDescriptor("3", null)); //$NON-NLS-1$
+ tabs.add(new TestTabDescriptor("5", null)); //$NON-NLS-1$
+ tabs.add(new TestTabDescriptor("4", "5")); //$NON-NLS-1$ //$NON-NLS-2$
+ tabs.add(new TestTabDescriptor("6", "5")); //$NON-NLS-1$ //$NON-NLS-2$
+
+ List<IEEFTabDescriptor> sortedTabs = new LegacyPropertyTabSorter().sortTabsByAfterTab(tabs);
+
+ assertEquals("[1, 3, 5, 2, 6, 4]", sortedTabs.toString()); //$NON-NLS-1$
+ }
+
+ @Test
+ public void testOrderedTabsWithAfterTab5() {
+ List<IEEFTabDescriptor> tabs = new ArrayList<>();
+ tabs.add(new TestTabDescriptor("5b", "4")); //$NON-NLS-1$ //$NON-NLS-2$
+ tabs.add(new TestTabDescriptor("1", null)); //$NON-NLS-1$
+ tabs.add(new TestTabDescriptor("2", "1")); //$NON-NLS-1$ //$NON-NLS-2$
+ tabs.add(new TestTabDescriptor("3", "2")); //$NON-NLS-1$ //$NON-NLS-2$
+ tabs.add(new TestTabDescriptor("4", "3")); //$NON-NLS-1$ //$NON-NLS-2$
+ tabs.add(new TestTabDescriptor("5", "4")); //$NON-NLS-1$ //$NON-NLS-2$
+ tabs.add(new TestTabDescriptor("6", "5")); //$NON-NLS-1$ //$NON-NLS-2$
+
+ List<IEEFTabDescriptor> sortedTabs = new LegacyPropertyTabSorter().sortTabsByAfterTab(tabs);
+
+ assertEquals("[1, 2, 3, 4, 5, 5b, 6]", sortedTabs.toString()); //$NON-NLS-1$
+ }
+
+ @Test
+ public void testOrderedTabsWithAfterTab6() {
+ List<IEEFTabDescriptor> tabs = new ArrayList<>();
+ tabs.add(new TestTabDescriptor("A", "C")); //$NON-NLS-1$ //$NON-NLS-2$
+ tabs.add(new TestTabDescriptor("B", null)); //$NON-NLS-1$
+ tabs.add(new TestTabDescriptor("C", "H")); //$NON-NLS-1$ //$NON-NLS-2$
+ tabs.add(new TestTabDescriptor("D", "E")); //$NON-NLS-1$ //$NON-NLS-2$
+ tabs.add(new TestTabDescriptor("E", "H")); //$NON-NLS-1$ //$NON-NLS-2$
+ tabs.add(new TestTabDescriptor("F", null)); //$NON-NLS-1$
+ tabs.add(new TestTabDescriptor("G", "B")); //$NON-NLS-1$ //$NON-NLS-2$
+ tabs.add(new TestTabDescriptor("H", "G")); //$NON-NLS-1$ //$NON-NLS-2$
+
+ List<IEEFTabDescriptor> sortedTabs = new LegacyPropertyTabSorter().sortTabsByAfterTab(tabs);
+
+ assertEquals("[B, F, G, H, E, D, C, A]", sortedTabs.toString()); //$NON-NLS-1$
+ }
+}
