From 27ffac6d17dfeef177f247e468fc5d9b6e9b72bc Mon Sep 17 00:00:00 2001 From: Guillermo Alvarez Date: Mon, 29 Sep 2014 10:50:18 +0300 Subject: [PATCH] TreeItem double click fixed (#14745) The event wasn't sent immediately and was sometimes overwritten by following click event. Change-Id: I7d52030ee8aac2be11b3b3db207d1c7f187d4778 --- client/src/com/vaadin/client/ui/VTree.java | 60 +++++++---------- .../components/tree/TreeItemDoubleClick.java | 61 +++++++++++++++++ .../tree/TreeItemDoubleClickTest.java | 65 +++++++++++++++++++ 3 files changed, 148 insertions(+), 38 deletions(-) create mode 100644 uitest/src/com/vaadin/tests/components/tree/TreeItemDoubleClick.java create mode 100644 uitest/src/com/vaadin/tests/components/tree/TreeItemDoubleClickTest.java diff --git a/client/src/com/vaadin/client/ui/VTree.java b/client/src/com/vaadin/client/ui/VTree.java index 02ad7cfb3e..82ffaced1f 100644 --- a/client/src/com/vaadin/client/ui/VTree.java +++ b/client/src/com/vaadin/client/ui/VTree.java @@ -162,10 +162,10 @@ public class VTree extends FocusElementPanel implements VHasDropHandler, /* * to fix #14388. The cause of defect #14388: event 'clickEvent' is sent to - * server before updating of "selected" variable, but should be send after + * server before updating of "selected" variable, but should be sent after * it */ - private boolean sendClickEventNow = false; + private boolean clickEventPending = false; /** For internal use only. May be removed or replaced in the future. */ public String[] bodyActionKeys; @@ -480,13 +480,13 @@ public class VTree extends FocusElementPanel implements VHasDropHandler, public void execute() { /* * we should send selection to server immediately in 2 cases: 1) - * 'immediate' property of Tree is true 2) sendClickEventNow is + * 'immediate' property of Tree is true 2) clickEventPending is * true */ client.updateVariable(paintableId, "selected", selectedIds.toArray(new String[selectedIds.size()]), - sendClickEventNow || immediate); - sendClickEventNow = false; + clickEventPending || immediate); + clickEventPending = false; selectionHasChanged = false; } }; @@ -844,41 +844,25 @@ 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. - 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; - } + clickEventPending = false; + if ((details.getButton() == MouseButton.LEFT || details + .getButton() == MouseButton.MIDDLE) + && !details.isDoubleClick() && selectable) { + // Probably a selection that will cause a value change + // event to be sent + clickEventPending = true; + + // 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) { + clickEventPending = false; } - - 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 } + client.updateVariable(paintableId, "clickedKey", key, false); + client.updateVariable(paintableId, "clickEvent", + details.toString(), !clickEventPending); } }); } diff --git a/uitest/src/com/vaadin/tests/components/tree/TreeItemDoubleClick.java b/uitest/src/com/vaadin/tests/components/tree/TreeItemDoubleClick.java new file mode 100644 index 0000000000..8b7890e63c --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/tree/TreeItemDoubleClick.java @@ -0,0 +1,61 @@ +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.Button; +import com.vaadin.ui.Button.ClickEvent; +import com.vaadin.ui.Tree; + +public class TreeItemDoubleClick extends AbstractTestUIWithLog { + + @Override + protected void setup(VaadinRequest request) { + final Tree tree = new Tree("Immediate With ItemClickListener"); + tree.setImmediate(true); + tree.setNullSelectionAllowed(false); + + for (int i = 1; i < 6; i++) { + tree.addItem("Tree Item " + i); + } + + ItemClickEvent.ItemClickListener listener = new ItemClickEvent.ItemClickListener() { + @Override + public void itemClick(ItemClickEvent event) { + if (event.isDoubleClick()) { + log.log("Double Click " + event.getItemId()); + } + } + }; + + tree.addItemClickListener(listener); + + addComponent(tree); + + Button button = new Button("Change immediate flag"); + button.addClickListener(new Button.ClickListener() { + + @Override + public void buttonClick(ClickEvent event) { + // this wouldn't work if tree had a value change listener + tree.setImmediate(!tree.isImmediate()); + log.log("tree.isImmediate() is now " + tree.isImmediate()); + } + + }); + + addComponent(button); + + } + + @Override + protected String getTestDescription() { + return "Tests that double click is fired"; + } + + @Override + protected Integer getTicketNumber() { + return 14745; + } + +} diff --git a/uitest/src/com/vaadin/tests/components/tree/TreeItemDoubleClickTest.java b/uitest/src/com/vaadin/tests/components/tree/TreeItemDoubleClickTest.java new file mode 100644 index 0000000000..95a3f02d60 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/tree/TreeItemDoubleClickTest.java @@ -0,0 +1,65 @@ +/* + * 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 org.junit.Test; +import org.openqa.selenium.WebElement; +import org.openqa.selenium.interactions.Actions; + +import com.vaadin.testbench.By; +import com.vaadin.testbench.elements.ButtonElement; +import com.vaadin.tests.tb3.MultiBrowserTest; + +public class TreeItemDoubleClickTest extends MultiBrowserTest { + + @Test + public void test() throws InterruptedException { + openTestURL(); + String caption = "Tree Item 2"; + doubleClick(getTreeNodeByCaption(caption)); + assertLogText("Double Click " + caption); + + changeImmediate(); + + caption = "Tree Item 3"; + doubleClick(getTreeNodeByCaption(caption)); + assertLogText("Double Click " + caption); + } + + private void changeImmediate() { + $(ButtonElement.class).caption("Change immediate flag").first().click(); + assertLogText("tree.isImmediate() is now"); + } + + private WebElement getTreeNodeByCaption(String caption) { + return getDriver().findElement( + By.xpath("//span[text() = '" + caption + "']")); + } + + private void doubleClick(WebElement element) { + new Actions(getDriver()).doubleClick(element).build().perform(); + + } + + private void assertLogText(String text) { + assertThat( + String.format("Couldn't find text '%s' from the log.", text), + logContainsText(text)); + } + +} -- 2.39.5