From: Anna Miroshnik Date: Mon, 8 Sep 2014 07:32:01 +0000 (+0400) Subject: Tree item fails with ItemClickListener (#14388) X-Git-Tag: 7.3.1~25 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=84038225ac71d56d21887a338e5daa8994a76eed;p=vaadin-framework.git Tree item fails with ItemClickListener (#14388) Patch for #14388: added modifications to remove the regression on #6845 (RIGHT, MIDDLE mouse buttons listeners) Change-Id: I3ef95df68efa0a70bbe0d566ceea187505e2999a --- diff --git a/client/src/com/vaadin/client/ui/VTree.java b/client/src/com/vaadin/client/ui/VTree.java index b12053ea04..02ad7cfb3e 100644 --- a/client/src/com/vaadin/client/ui/VTree.java +++ b/client/src/com/vaadin/client/ui/VTree.java @@ -160,6 +160,13 @@ public class VTree extends FocusElementPanel implements VHasDropHandler, private boolean selectionHasChanged = false; + /* + * to fix #14388. The cause of defect #14388: event 'clickEvent' is sent to + * server before updating of "selected" variable, but should be send after + * it + */ + private boolean sendClickEventNow = false; + /** For internal use only. May be removed or replaced in the future. */ public String[] bodyActionKeys; @@ -471,9 +478,15 @@ public class VTree extends FocusElementPanel implements VHasDropHandler, Command command = new Command() { @Override public void execute() { + /* + * we should send selection to server immediately in 2 cases: 1) + * 'immediate' property of Tree is true 2) sendClickEventNow is + * true + */ client.updateVariable(paintableId, "selected", selectedIds.toArray(new String[selectedIds.size()]), - immediate); + sendClickEventNow || immediate); + sendClickEventNow = false; selectionHasChanged = false; } }; @@ -831,26 +844,41 @@ public class VTree extends FocusElementPanel implements VHasDropHandler, // server. We do not want to send the event if there is a // selection event happening after this. In all other cases // we want to send it immediately. - boolean sendClickEventNow = true; - - if (details.getButton() == MouseButton.LEFT && immediate - && selectable) { - // Probably a selection that will cause a value change - // event to be sent - sendClickEventNow = false; - - // The exception is that user clicked on the - // currently selected row and null selection is not - // allowed == no selection event - if (isSelected() && selectedIds.size() == 1 - && !isNullSelectionAllowed) { - sendClickEventNow = true; + sendClickEventNow = true; + + if (details.getButton() == MouseButton.LEFT && selectable) { + if (immediate) { + // event to be sent + sendClickEventNow = false; + + // The exception is that user clicked on the + // currently selected row and null selection is not + // allowed == no selection event + if (isSelected() && selectedIds.size() == 1 + && !isNullSelectionAllowed) { + sendClickEventNow = true; + } } - } - client.updateVariable(paintableId, "clickedKey", key, false); - client.updateVariable(paintableId, "clickEvent", - details.toString(), sendClickEventNow); + client.updateVariable(paintableId, "clickedKey", key, + false); + + /* + * in any case event should not be send immediately here + * - send after updating of "selected" variable + */ + client.updateVariable(paintableId, "clickEvent", + details.toString(), false); + + } else { // for all another mouse buttons (RIGHT, MIDDLE) or + // if not selectable + client.updateVariable(paintableId, "clickedKey", key, + false); + + client.updateVariable(paintableId, "clickEvent", + details.toString(), sendClickEventNow); + sendClickEventNow = false; // reset it + } } }); } diff --git a/uitest/src/com/vaadin/tests/components/tree/TreeItemSelectionWithoutImmediate.java b/uitest/src/com/vaadin/tests/components/tree/TreeItemSelectionWithoutImmediate.java new file mode 100644 index 0000000000..8e0f39be99 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/tree/TreeItemSelectionWithoutImmediate.java @@ -0,0 +1,46 @@ +package com.vaadin.tests.components.tree; + +import com.vaadin.event.ItemClickEvent; +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUIWithLog; +import com.vaadin.ui.Tree; + +@SuppressWarnings("serial") +public class TreeItemSelectionWithoutImmediate extends AbstractTestUIWithLog { + + protected static final String TREE_ID = "TreeId"; + + protected static final String MENU_ITEM_TEMPLATE = "Menu Item %d"; + + @Override + protected void setup(VaadinRequest request) { + Tree tree = new Tree("With ItemClickListener not Immediate"); + tree.setId(TREE_ID); + tree.setImmediate(false); + + for (int i = 1; i <= 4; i++) { + tree.addItem(String.format(MENU_ITEM_TEMPLATE, i)); + } + + tree.addItemClickListener(new ItemClickEvent.ItemClickListener() { + + @Override + public void itemClick(ItemClickEvent event) { + log("ItemClickEvent = " + event.getItemId()); + } + }); + + addComponent(tree); + } + + @Override + protected String getTestDescription() { + return "Test for ensuring that selection of tree items works correctly if immediate == false " + + "and ItemClickListener is added to Tree"; + } + + @Override + protected Integer getTicketNumber() { + return 14388; + } +} diff --git a/uitest/src/com/vaadin/tests/components/tree/TreeItemSelectionWithoutImmediateTest.java b/uitest/src/com/vaadin/tests/components/tree/TreeItemSelectionWithoutImmediateTest.java new file mode 100644 index 0000000000..2fb35b1680 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/tree/TreeItemSelectionWithoutImmediateTest.java @@ -0,0 +1,61 @@ +/* + * Copyright 2000-2014 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.components.tree; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + +import org.junit.Test; +import org.openqa.selenium.WebElement; +import org.openqa.selenium.interactions.Actions; + +import com.vaadin.testbench.By; +import com.vaadin.tests.tb3.MultiBrowserTest; + +public class TreeItemSelectionWithoutImmediateTest extends MultiBrowserTest { + + private static final long serialVersionUID = 1L; + + @Test + public void testSelectTreeWithItemClickListenerNotImmediate() + throws InterruptedException { + openTestURL(); + + // click on item i (in circle we select next item and check if it is + // selected in tree) + for (int i = 1; i <= 4; i++) { + WebElement treeItem = getTreeNode(String.format( + TreeItemSelectionWithoutImmediate.MENU_ITEM_TEMPLATE, i)); + + new Actions(getDriver()).moveToElement(treeItem).click().perform(); + Thread.sleep(100); + + WebElement selectedElement = driver.findElement(By + .className("v-tree-node-selected")); + + treeItem = getTreeNode(String.format( + TreeItemSelectionWithoutImmediate.MENU_ITEM_TEMPLATE, i)); + + assertThat("Clicked element should be selected", selectedElement + .getText().equals(treeItem.getText()), is(true)); + } + } + + private WebElement getTreeNode(String caption) { + return getDriver().findElement( + By.xpath("//span[text() = '" + caption + "']")); + } +}