From b302cd948421b8e7c733b5f76fcd326ba7abece0 Mon Sep 17 00:00:00 2001 From: Denis Date: Fri, 14 Sep 2018 11:51:20 +0300 Subject: [PATCH] Make addComponentAtIndex logic FW8 compatible (#20) Part of the fix for #4600 --- .../contextmenu/ContextMenuBase.java | 33 +++++++++---------- .../contextmenu/ContextMenuTest.java | 17 ++++++++++ .../contextmenu/it/ContextMenuPage.java | 9 +---- .../contextmenu/it/ContextMenuPageIT.java | 18 ---------- 4 files changed, 33 insertions(+), 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 index 91caccb..84f9943 100644 --- a/src/main/java/com/vaadin/flow/component/contextmenu/ContextMenuBase.java +++ b/src/main/java/com/vaadin/flow/component/contextmenu/ContextMenuBase.java @@ -36,7 +36,7 @@ * {@code }. 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") @@ -44,8 +44,7 @@ @HtmlImport("frontend://bower_components/vaadin-list-box/src/vaadin-list-box.html") @JavaScript("frontend://contextMenuConnector.js") public class ContextMenuBase> - extends GeneratedVaadinContextMenu - implements HasComponents { + extends GeneratedVaadinContextMenu implements HasComponents { private Component target; @@ -91,7 +90,7 @@ public ContextMenuBase() { *

* 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() { *

* 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) { *

* 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 } element, but will be inserted into an * overlay that is attached into the {@code }. - * + * * @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 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 index 5c293d1..b8107e1 100644 --- 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 index f815178..334e30f 100644 --- 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 index b5516ce..9ec912c 100644 --- 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)