From: Guillermo Alvarez Date: Tue, 19 Aug 2014 15:38:35 +0000 (+0300) Subject: Table handles both onMouseDown and onMouseUp events (#14347) X-Git-Tag: 7.3.1~23 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=6877a4d8802f9fae2b54e390e5171a1df705253d;p=vaadin-framework.git Table handles both onMouseDown and onMouseUp events (#14347) Now it has the same behaviour that the layout, and checks that the element under the mouse matches to complete the click action Change-Id: I9c61dac24479913c1bb1094edaf8878749984342 --- diff --git a/client/src/com/vaadin/client/ui/VScrollTable.java b/client/src/com/vaadin/client/ui/VScrollTable.java index cb90823a7f..e7d760cc4e 100644 --- a/client/src/com/vaadin/client/ui/VScrollTable.java +++ b/client/src/com/vaadin/client/ui/VScrollTable.java @@ -25,6 +25,8 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.logging.Level; +import java.util.logging.Logger; import com.google.gwt.core.client.JavaScriptObject; import com.google.gwt.core.client.Scheduler; @@ -66,6 +68,8 @@ import com.google.gwt.regexp.shared.RegExp; import com.google.gwt.user.client.Command; import com.google.gwt.user.client.DOM; import com.google.gwt.user.client.Event; +import com.google.gwt.user.client.Event.NativePreviewEvent; +import com.google.gwt.user.client.Event.NativePreviewHandler; import com.google.gwt.user.client.Timer; import com.google.gwt.user.client.Window; import com.google.gwt.user.client.ui.FlowPanel; @@ -763,6 +767,51 @@ public class VScrollTable extends FlowPanel implements HasWidgets, private HandlerRegistration addCloseHandler; + /** + * Changes to manage mouseDown and mouseUp + */ + /** + * The element where the last mouse down event was registered. + */ + private Element lastMouseDownTarget; + + /** + * Set to true by {@link #mouseUpPreviewHandler} if it gets a mouseup at the + * same element as {@link #lastMouseDownTarget}. + */ + private boolean mouseUpPreviewMatched = false; + + private HandlerRegistration mouseUpEventPreviewRegistration; + + /** + * Previews events after a mousedown to detect where the following mouseup + * hits. + */ + private final NativePreviewHandler mouseUpPreviewHandler = new NativePreviewHandler() { + + @Override + public void onPreviewNativeEvent(NativePreviewEvent event) { + if (event.getTypeInt() == Event.ONMOUSEUP) { + mouseUpEventPreviewRegistration.removeHandler(); + + // Event's reported target not always correct if event + // capture is in use + Element elementUnderMouse = Util.getElementUnderMouse(event + .getNativeEvent()); + if (lastMouseDownTarget != null + && lastMouseDownTarget.isOrHasChild(elementUnderMouse)) { + mouseUpPreviewMatched = true; + } else { + getLogger().log( + Level.FINEST, + "Ignoring mouseup from " + elementUnderMouse + + " when mousedown was on " + + lastMouseDownTarget); + } + } + } + }; + public VScrollTable() { setMultiSelectMode(MULTISELECT_MODE_DEFAULT); @@ -5922,114 +5971,134 @@ public class VScrollTable extends FlowPanel implements HasWidgets, } break; case Event.ONMOUSEUP: - if (targetCellOrRowFound) { - /* - * Queue here, send at the same time as the - * corresponding value change event - see #7127 - */ - boolean clickEventSent = handleClickEvent(event, - targetTdOrTr, false); - - if (event.getButton() == Event.BUTTON_LEFT - && isSelectable()) { - - // Ctrl+Shift click - if ((event.getCtrlKey() || event.getMetaKey()) - && event.getShiftKey() - && isMultiSelectModeDefault()) { - toggleShiftSelection(false); - setRowFocus(this); - - // Ctrl click - } else if ((event.getCtrlKey() || event - .getMetaKey()) - && isMultiSelectModeDefault()) { - boolean wasSelected = isSelected(); - toggleSelection(); - setRowFocus(this); - /* - * next possible range select must start on - * this row - */ - selectionRangeStart = this; - if (wasSelected) { - removeRowFromUnsentSelectionRanges(this); - } + /* + * Only fire a click if the mouseup hits the same + * element as the corresponding mousedown. This is first + * checked in the event preview but we can't fire the + * event there as the event might get canceled before it + * gets here. + */ + if (mouseUpPreviewMatched + && lastMouseDownTarget != null + && lastMouseDownTarget == getElementTdOrTr(Util + .getElementUnderMouse(event))) { + // "Click" with left, right or middle button + + if (targetCellOrRowFound) { + /* + * Queue here, send at the same time as the + * corresponding value change event - see #7127 + */ + boolean clickEventSent = handleClickEvent( + event, targetTdOrTr, false); + + if (event.getButton() == Event.BUTTON_LEFT + && isSelectable()) { + + // Ctrl+Shift click + if ((event.getCtrlKey() || event + .getMetaKey()) + && event.getShiftKey() + && isMultiSelectModeDefault()) { + toggleShiftSelection(false); + setRowFocus(this); - } else if ((event.getCtrlKey() || event - .getMetaKey()) && isSingleSelectMode()) { - // Ctrl (or meta) click (Single selection) - if (!isSelected() - || (isSelected() && nullSelectionAllowed)) { + // Ctrl click + } else if ((event.getCtrlKey() || event + .getMetaKey()) + && isMultiSelectModeDefault()) { + boolean wasSelected = isSelected(); + toggleSelection(); + setRowFocus(this); + /* + * next possible range select must start + * on this row + */ + selectionRangeStart = this; + if (wasSelected) { + removeRowFromUnsentSelectionRanges(this); + } - if (!isSelected()) { - deselectAll(); + } else if ((event.getCtrlKey() || event + .getMetaKey()) + && isSingleSelectMode()) { + // Ctrl (or meta) click (Single + // selection) + if (!isSelected() + || (isSelected() && nullSelectionAllowed)) { + + if (!isSelected()) { + deselectAll(); + } + + toggleSelection(); + setRowFocus(this); } - toggleSelection(); + } else if (event.getShiftKey() + && isMultiSelectModeDefault()) { + // Shift click + toggleShiftSelection(true); + + } else { + // click + boolean currentlyJustThisRowSelected = selectedRowKeys + .size() == 1 + && selectedRowKeys + .contains(getKey()); + + if (!currentlyJustThisRowSelected) { + if (isSingleSelectMode() + || isMultiSelectModeDefault()) { + /* + * For default multi select mode + * (ctrl/shift) and for single + * select mode we need to clear + * the previous selection before + * selecting a new one when the + * user clicks on a row. Only in + * multiselect/simple mode the + * old selection should remain + * after a normal click. + */ + deselectAll(); + } + toggleSelection(); + } else if ((isSingleSelectMode() || isMultiSelectModeSimple()) + && nullSelectionAllowed) { + toggleSelection(); + }/* + * else NOP to avoid excessive server + * visits (selection is removed with + * CTRL/META click) + */ + + selectionRangeStart = this; setRowFocus(this); } - } else if (event.getShiftKey() - && isMultiSelectModeDefault()) { - // Shift click - toggleShiftSelection(true); - - } else { - // click - boolean currentlyJustThisRowSelected = selectedRowKeys - .size() == 1 - && selectedRowKeys - .contains(getKey()); - - if (!currentlyJustThisRowSelected) { - if (isSingleSelectMode() - || isMultiSelectModeDefault()) { - /* - * For default multi select mode - * (ctrl/shift) and for single - * select mode we need to clear the - * previous selection before - * selecting a new one when the user - * clicks on a row. Only in - * multiselect/simple mode the old - * selection should remain after a - * normal click. - */ - deselectAll(); - } - toggleSelection(); - } else if ((isSingleSelectMode() || isMultiSelectModeSimple()) - && nullSelectionAllowed) { - toggleSelection(); - }/* - * else NOP to avoid excessive server - * visits (selection is removed with - * CTRL/META click) - */ - - selectionRangeStart = this; - setRowFocus(this); + // Remove IE text selection hack + if (BrowserInfo.get().isIE()) { + ((Element) event.getEventTarget() + .cast()).setPropertyJSO( + "onselectstart", null); + } + // Queue value change + sendSelectedRows(false); } - - // Remove IE text selection hack - if (BrowserInfo.get().isIE()) { - ((Element) event.getEventTarget().cast()) - .setPropertyJSO("onselectstart", - null); + /* + * Send queued click and value change events if + * any If a click event is sent, send value + * change with it regardless of the immediate + * flag, see #7127 + */ + if (immediate || clickEventSent) { + client.sendPendingVariableChanges(); } - // Queue value change - sendSelectedRows(false); - } - /* - * Send queued click and value change events if any - * If a click event is sent, send value change with - * it regardless of the immediate flag, see #7127 - */ - if (immediate || clickEventSent) { - client.sendPendingVariableChanges(); } } + mouseUpPreviewMatched = false; + lastMouseDownTarget = null; break; case Event.ONTOUCHEND: case Event.ONTOUCHCANCEL: @@ -6135,6 +6204,17 @@ public class VScrollTable extends FlowPanel implements HasWidgets, } break; case Event.ONMOUSEDOWN: + /* + * When getting a mousedown event, we must detect where + * the corresponding mouseup event if it's on a + * different part of the page. + */ + lastMouseDownTarget = getElementTdOrTr(Util + .getElementUnderMouse(event)); + mouseUpPreviewMatched = false; + mouseUpEventPreviewRegistration = Event + .addNativePreviewHandler(mouseUpPreviewHandler); + if (targetCellOrRowFound) { setRowFocus(this); ensureFocus(); @@ -6269,7 +6349,12 @@ public class VScrollTable extends FlowPanel implements HasWidgets, */ private Element getEventTargetTdOrTr(Event event) { final Element eventTarget = event.getEventTarget().cast(); - Widget widget = Util.findWidget(eventTarget, null); + return getElementTdOrTr(eventTarget); + } + + private Element getElementTdOrTr(Element element) { + + Widget widget = Util.findWidget(element, null); if (widget != this) { /* @@ -6289,7 +6374,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets, return null; } } - return getTdOrTr(eventTarget); + return getTdOrTr(element); } @Override @@ -8133,4 +8218,8 @@ public class VScrollTable extends FlowPanel implements HasWidgets, public boolean isWorkPending() { return lazyAdjustColumnWidths.isRunning(); } + + private static Logger getLogger() { + return Logger.getLogger(VScrollTable.class.getName()); + } } diff --git a/uitest/src/com/vaadin/tests/components/table/TableMatchesMouseDownMouseUpElement.java b/uitest/src/com/vaadin/tests/components/table/TableMatchesMouseDownMouseUpElement.java new file mode 100644 index 0000000000..1515589b4c --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/table/TableMatchesMouseDownMouseUpElement.java @@ -0,0 +1,76 @@ +package com.vaadin.tests.components.table; + +import com.vaadin.data.Item; +import com.vaadin.server.VaadinRequest; +import com.vaadin.shared.ui.label.ContentMode; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.Button; +import com.vaadin.ui.Button.ClickEvent; +import com.vaadin.ui.Button.ClickListener; +import com.vaadin.ui.Component; +import com.vaadin.ui.Label; +import com.vaadin.ui.Table; +import com.vaadin.ui.VerticalLayout; + +public class TableMatchesMouseDownMouseUpElement extends AbstractTestUI { + + static final String CLEAR_BUTTON_ID = "clear-button-id"; + + @Override + protected String getTestDescription() { + return "Both mouse down and mouse up should be done on same cell to be considered as a click."; + } + + @Override + protected Integer getTicketNumber() { + return 14347; + } + + @SuppressWarnings("unchecked") + @Override + protected void setup(VaadinRequest request) { + final Table table = new Table(); + table.setHeight("500px"); + table.setSelectable(true); + table.setNullSelectionAllowed(true); + table.addContainerProperty("Column 1", String.class, ""); + table.addContainerProperty("Column 2", Component.class, ""); + table.addContainerProperty("Column 3", Component.class, ""); + table.addContainerProperty("Column 4", Component.class, ""); + + Item item = table.addItem("Item 1 (row 1)"); + item.getItemProperty("Column 1").setValue("String A"); + item.getItemProperty("Column 2").setValue(new Label("Label A")); + item.getItemProperty("Column 3").setValue( + new Label("Label A", ContentMode.HTML)); + VerticalLayout l = new VerticalLayout(); + l.setId("row-1"); + l.setHeight(100, Unit.PIXELS); + item.getItemProperty("Column 4").setValue(l); + + item = table.addItem("Item 2 (row 2)"); + item.getItemProperty("Column 1").setValue("String B"); + item.getItemProperty("Column 2").setValue(new Label("Label B")); + item.getItemProperty("Column 3") + .setValue( + new Label( + "Label B", + ContentMode.HTML)); + l = new VerticalLayout(); + l.setId("row-2"); + l.setSizeFull(); + item.getItemProperty("Column 4").setValue(l); + + Button clear = new Button("Clear"); + clear.setId(CLEAR_BUTTON_ID); + clear.addClickListener(new ClickListener() { + + @Override + public void buttonClick(ClickEvent event) { + table.setValue(null); + } + }); + addComponent(table); + addComponent(clear); + } +} diff --git a/uitest/src/com/vaadin/tests/components/table/TableMatchesMouseDownMouseUpElementTest.java b/uitest/src/com/vaadin/tests/components/table/TableMatchesMouseDownMouseUpElementTest.java new file mode 100644 index 0000000000..ea730ea30e --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/table/TableMatchesMouseDownMouseUpElementTest.java @@ -0,0 +1,147 @@ +/* + * 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.table; + +import static com.vaadin.tests.components.table.TableMatchesMouseDownMouseUpElement.CLEAR_BUTTON_ID; +import static org.junit.Assert.assertEquals; + +import java.util.List; + +import org.junit.Test; +import org.openqa.selenium.By; +import org.openqa.selenium.WebElement; +import org.openqa.selenium.interactions.Actions; + +import com.vaadin.testbench.elements.TableElement; +import com.vaadin.tests.tb3.MultiBrowserTest; + +/** + * Regular click cases already covered by @LabelEmbeddedClickThroughForTableTest + * Testing cases when mouse down and mouse up positions are different + * + * @since + * @author Vaadin Ltd + */ +public class TableMatchesMouseDownMouseUpElementTest extends MultiBrowserTest { + + TableElement table; + + @Test + public void testClick() { + openTestURL(); + table = $(TableElement.class).first(); + + testMoveOut(getBoldTag(0, 2)); + testMoveIn(getBoldTag(0, 2)); + + testMoveOut(getLabel(0, 1)); + testMoveIn(getLabel(0, 1)); + + testClickOnDifferentRows(); + } + + /** + * MouseDown on element and mouseUp outside element but on same cell + */ + private void testMoveOut(WebElement element) { + clearSelection(); + clickAndMove(element, 5, 5, 0, 50); + checkSelectedRowCount(1); + checkRowSelected(0); + } + + /** + * MouseDown outside element but on same cell and mouseUp on element + */ + private void testMoveIn(WebElement element) { + clearSelection(); + clickAndMove(element, 5, 55, 0, -50); + checkSelectedRowCount(1); + checkRowSelected(0); + } + + /** + * Mouse down in cell of row1 holds and mouse up in cell of row 2 + */ + public void testClickOnDifferentRows() { + clearSelection(); + WebElement elementFrom = getCell(0, 1); + WebElement elementTo = getCell(0, 2); + clickAndMove(elementFrom, elementTo); + checkSelectedRowCount(0); + } + + private WebElement getBoldTag(int row, int column) { + return table.getCell(row, column).findElement(By.className("v-label")) + .findElement(By.tagName("b")); + } + + private WebElement getLabel(int row, int column) { + return table.getCell(row, column).findElement(By.className("v-label")); + } + + private WebElement getCell(int row, int column) { + return table.getCell(row, column); + } + + private void clearSelection() { + WebElement clearButton = vaadinElementById(CLEAR_BUTTON_ID); + clearButton.click(); + } + + /** + * Mouse down on element + initial offset -> Moves the "move offset" -> + * Mouse up + */ + private void clickAndMove(WebElement element, int initialX, int initialY, + int moveX, int moveY) { + new Actions(driver).moveToElement(element, initialX, initialY) + .clickAndHold().perform(); + new Actions(driver).moveByOffset(moveX, moveY).perform(); + new Actions(driver).release().perform(); + } + + /** + * Mouse down on elementFrom -> Moves to elementTo -> Mouse up + */ + private void clickAndMove(WebElement elementFrom, WebElement elementTo) { + new Actions(driver).moveToElement(elementFrom, 5, 5).clickAndHold() + .perform(); + new Actions(driver).moveToElement(elementTo, 5, 5).perform(); + new Actions(driver).release().perform(); + } + + private void checkRowSelected(int rowIndex) { + assertEquals( + "contents of the selected row don't match contents of the row #" + + rowIndex, + table.getCell(rowIndex, 0).getText(), + getSelectedRows().get(0) + .findElement(By.className("v-table-cell-wrapper")) + .getText()); + } + + private void checkSelectedRowCount(int expected) { + assertEquals("unexpected table selection size", expected, + getSelectedRows().size()); + } + + private List getSelectedRows() { + return table.findElement(By.className("v-table-body")).findElements( + By.className("v-selected")); + } + +}