Make addComponentAtIndex logic FW8 compatible (#20)

Part of the fix for #4600
vaadin · Sep 14, 2018 · b302cd9 · b302cd9
1 parent 53103a8
commit b302cd9
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 44 deletions.
diff --git a/src/main/java/com/vaadin/flow/component/contextmenu/ContextMenuBase.java b/src/main/java/com/vaadin/flow/component/contextmenu/ContextMenuBase.java
@@ -36,16 +36,15 @@
  * {@code <vaadin-context-menu>}. Classes extending this should provide API for
  * adding items and handling events related to them. For basic example, see
  * {@link ContextMenu}.
- * 
+ *
  * @author Vaadin Ltd.
  */
 @SuppressWarnings("serial")
 @HtmlImport("flow-component-renderer.html")
 @HtmlImport("frontend://bower_components/vaadin-list-box/src/vaadin-list-box.html")
 @JavaScript("frontend://contextMenuConnector.js")
 public class ContextMenuBase<C extends ContextMenuBase<C>>
-        extends GeneratedVaadinContextMenu<C>
-        implements HasComponents {
+        extends GeneratedVaadinContextMenu<C> implements HasComponents {
 
     private Component target;
 
@@ -91,7 +90,7 @@ public ContextMenuBase() {
      * <p>
      * By default, the context menu can be opened with a right click or a long
      * touch on the target component.
-     * 
+     *
      * @param target
      *            the target component for this context menu, can be
      *            {@code null} to remove the target
@@ -164,7 +163,7 @@ private UI getCurrentUI() {
     /**
      * Gets the target component of this context menu, or {@code null} if it
      * doesn't have a target.
-     * 
+     *
      * @return the target component of this context menu
      * @see #setTarget(Component)
      */
@@ -177,7 +176,7 @@ public Component getTarget() {
      * <p>
      * By default, the context menu can be opened with a right click or a long
      * touch on the target component.
-     * 
+     *
      * @param openOnClick
      *            if {@code true}, the context menu can be opened with left
      *            click only. Otherwise the context menu follows the default
@@ -193,7 +192,7 @@ public void setOpenOnClick(boolean openOnClick) {
      * <p>
      * By default, this will return {@code false} and context menu can be opened
      * with a right click or a long touch on the target component.
-     * 
+     *
      * @return {@code true} if the context menu can be opened with left click
      *         only. Otherwise the context menu follows the default behavior.
      */
@@ -267,7 +266,7 @@ public void removeAll() {
      * The added elements in the DOM will not be children of the
      * {@code <vaadin-context-menu>} element, but will be inserted into an
      * overlay that is attached into the {@code <body>}.
-     * 
+     *
      * @param index
      *            the index, where the component will be added.
      * @param component
@@ -276,15 +275,13 @@ public void removeAll() {
     @Override
     public void addComponentAtIndex(int index, Component component) {
         Objects.requireNonNull(component, "Component should not be null");
-        int indexCheck;
         if (index < 0) {
-            indexCheck = 0;
-        } else if (index > container.getChildCount()) {
-            indexCheck = container.getChildCount();
-        } else {
-            indexCheck = index;
+            throw new IllegalArgumentException(
+                    "Cannot add a component with a negative index");
         }
-        container.insertChild(indexCheck, component.getElement());
+        // The case when the index is bigger than the children count is handled
+        // inside the method below
+        container.insertChild(index, component.getElement());
     }
 
     /**
@@ -305,7 +302,7 @@ public Stream<Component> getChildren() {
     /**
      * Gets the items added to this component (the children of this component
      * that are instances of {@link MenuItem}).
-     * 
+     *
      * @return the {@link MenuItem} components in this context menu
      * @see #addItem(String, ComponentEventListener)
      */
@@ -326,7 +323,7 @@ public boolean isOpened() {
     /**
      * Creates and adds a new item component to this context menu with the given
      * text content.
-     * 
+     *
      * @param text
      *            the text content for the created menu item
      * @return the created menu item
@@ -341,7 +338,7 @@ protected MenuItem addItem(String text) {
     /**
      * Creates and adds a new item component to this context menu with the given
      * component inside.
-     * 
+     *
      * @param component
      *            the component to add to the created menu item
      * @return the created menu item

diff --git a/src/test/java/com/vaadin/flow/component/contextmenu/ContextMenuTest.java b/src/test/java/com/vaadin/flow/component/contextmenu/ContextMenuTest.java
@@ -159,6 +159,23 @@ public void addItemsAndComponents_getItemsReturnsItemsOnly() {
         Assert.assertEquals(item2, items.get(1));
     }
 
+    @Test(expected = IllegalArgumentException.class)
+    public void addComponentAtIndex_negativeIndex() {
+        addDivAtIndex(-1);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void addComponentAtIndex_indexIsBiggerThanChildrenCount() {
+        addDivAtIndex(1);
+    }
+
+    private void addDivAtIndex(int index) {
+        ContextMenu contextMenu = new ContextMenu();
+
+        Div div = new Div();
+        contextMenu.addComponentAtIndex(index, div);
+    }
+
     private void assertComponentIsMenuItem(Component component,
             String expectedText) {
         Assert.assertTrue(

diff --git a/src/test/java/com/vaadin/flow/component/contextmenu/it/ContextMenuPage.java b/src/test/java/com/vaadin/flow/component/contextmenu/it/ContextMenuPage.java
@@ -61,14 +61,7 @@ private void createContextMenuAndAddComponentAtIndex() {
         NativeButton addAtSecond = createTestButton(contextMenu, addedButton,
                 "button-to-second", 1);
 
-        NativeButton addOverIndex = createTestButton(contextMenu, addedButton,
-                "button-over-index", 10);
-
-        NativeButton addNegativeIndex = createTestButton(contextMenu,
-                addedButton,
-                "button-negative-index", -10);
-
-        add(target, addFirst, addAtSecond, addOverIndex, addNegativeIndex);
+        add(target, addFirst, addAtSecond);
     }
 
     private NativeButton createTestButton(ContextMenu menu,

diff --git a/src/test/java/com/vaadin/flow/component/contextmenu/it/ContextMenuPageIT.java b/src/test/java/com/vaadin/flow/component/contextmenu/it/ContextMenuPageIT.java
@@ -154,24 +154,6 @@ public void openMenuAddComponentAtIndex() {
         assertButtonText(1);
     }
 
-    @Test
-    public void openMenuAddComponentOverIndex() {
-        verifyInitialMenu(3);
-        findElement(By.id("button-over-index")).click();
-        rightClickOn("context-menu-add-component-target");
-        assertButtonNumberInMenu(4);
-        assertButtonText(3);
-    }
-
-    @Test
-    public void openMenuAddComponentNegativeIndex() {
-        verifyInitialMenu(3);
-        findElement(By.id("button-negative-index")).click();
-        rightClickOn("context-menu-add-component-target");
-        assertButtonNumberInMenu(4);
-        assertButtonText(0);
-    }
-
     private void assertButtonText(int index) {
         Assert.assertEquals("Button Text is not correct", "Added Button",
                 findElements(By.tagName(CONTEXT_MENU_OVERLAY_TAG)).get(0)