From: Artur Date: Thu, 22 Dec 2016 09:40:21 +0000 (+0200) Subject: Use correct indexes in multiselect checkboxes after removing rows (#8072) X-Git-Tag: 7.7.7~31 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=813c53c5260486757b3ef4d6dfe33ac73c75af32;p=vaadin-framework.git Use correct indexes in multiselect checkboxes after removing rows (#8072) Fixes #8011 --- diff --git a/client/src/main/java/com/vaadin/client/widget/grid/selection/MultiSelectionRenderer.java b/client/src/main/java/com/vaadin/client/widget/grid/selection/MultiSelectionRenderer.java index 27e6108fa6..b27a4a2eed 100644 --- a/client/src/main/java/com/vaadin/client/widget/grid/selection/MultiSelectionRenderer.java +++ b/client/src/main/java/com/vaadin/client/widget/grid/selection/MultiSelectionRenderer.java @@ -26,6 +26,7 @@ import com.google.gwt.dom.client.BrowserEvents; import com.google.gwt.dom.client.Element; import com.google.gwt.dom.client.NativeEvent; import com.google.gwt.dom.client.TableElement; +import com.google.gwt.dom.client.TableRowElement; import com.google.gwt.dom.client.TableSectionElement; import com.google.gwt.event.dom.client.ClickEvent; import com.google.gwt.event.dom.client.ClickHandler; @@ -45,6 +46,7 @@ import com.vaadin.client.widget.grid.RendererCellReference; import com.vaadin.client.widget.grid.events.GridEnabledEvent; import com.vaadin.client.widget.grid.events.GridEnabledHandler; import com.vaadin.client.widget.grid.selection.SelectionModel.Multi.Batched; +import com.vaadin.client.widgets.Escalator.AbstractRowContainer; import com.vaadin.client.widgets.Grid; /** @@ -319,7 +321,7 @@ public class MultiSelectionRenderer int constrainedPageY = Math.max(bodyAbsoluteTop, Math.min(bodyAbsoluteBottom, pageY)); - int logicalRow = getLogicalRowIndex(WidgetUtil + int logicalRow = getLogicalRowIndex(grid, WidgetUtil .getElementFromPoint(initialPageX, constrainedPageY)); int incrementOrDecrement = (logicalRow > lastModifiedLogicalRow) ? 1 @@ -586,8 +588,6 @@ public class MultiSelectionRenderer } } - private static final String LOGICAL_ROW_PROPERTY_INT = "vEscalatorLogicalRow"; - private final Grid grid; private HandlerRegistration nativePreviewHandlerRegistration; @@ -633,8 +633,6 @@ public class MultiSelectionRenderer CheckBox checkBox) { checkBox.setValue(data, false); checkBox.setEnabled(grid.isEnabled() && !grid.isEditorActive()); - checkBox.getElement().setPropertyInt(LOGICAL_ROW_PROPERTY_INT, - cell.getRowIndex()); } @Override @@ -668,7 +666,7 @@ public class MultiSelectionRenderer private void startDragSelect(NativeEvent event, final Element target) { injectNativeHandler(); - int logicalRowIndex = getLogicalRowIndex(target); + int logicalRowIndex = getLogicalRowIndex(grid, target); autoScrollHandler.start(logicalRowIndex); event.preventDefault(); event.stopPropagation(); @@ -687,7 +685,7 @@ public class MultiSelectionRenderer } } - private int getLogicalRowIndex(final Element target) { + private int getLogicalRowIndex(Grid grid, final Element target) { if (target == null) { return -1; } @@ -707,7 +705,8 @@ public class MultiSelectionRenderer final Element checkbox = td.getFirstChildElement(); assert checkbox != null : "Checkbox has disappeared"; - return checkbox.getPropertyInt(LOGICAL_ROW_PROPERTY_INT); + return ((AbstractRowContainer) grid.getEscalator().getBody()) + .getLogicalRowIndex((TableRowElement) tr); } tr = tr.getNextSiblingElement(); } diff --git a/client/src/main/java/com/vaadin/client/widgets/Escalator.java b/client/src/main/java/com/vaadin/client/widgets/Escalator.java index 4bd61cb53c..f9c8d96ad4 100644 --- a/client/src/main/java/com/vaadin/client/widgets/Escalator.java +++ b/client/src/main/java/com/vaadin/client/widgets/Escalator.java @@ -53,7 +53,6 @@ import com.google.gwt.dom.client.TableSectionElement; import com.google.gwt.dom.client.Touch; import com.google.gwt.event.shared.HandlerRegistration; import com.google.gwt.logging.client.LogConfiguration; -import com.google.gwt.user.client.Command; import com.google.gwt.user.client.DOM; import com.google.gwt.user.client.Window; import com.google.gwt.user.client.ui.RequiresResize; @@ -469,9 +468,9 @@ public class Escalator extends Widget if (isCurrentBrowserIE11OrEdge()) { return vertical ? event.getNativeEvent().getClientY() - + Window.getScrollTop() + + Window.getScrollTop() : event.getNativeEvent().getClientX() - + Window.getScrollLeft(); + + Window.getScrollLeft(); } JsArray a = event.getNativeEvent().getTouches(); return vertical ? a.get(0).getPageY() : a.get(0).getPageX(); @@ -564,9 +563,11 @@ public class Escalator extends Widget // (#18737), // otherwise allow touch only if there is a single touch in the // event - private boolean allowTouch(final TouchHandlerBundle.CustomTouchEvent event) { + private boolean allowTouch( + final TouchHandlerBundle.CustomTouchEvent event) { if (isCurrentBrowserIE11OrEdge()) { - return (POINTER_EVENT_TYPE_TOUCH.equals(event.getPointerType())); + return (POINTER_EVENT_TYPE_TOUCH + .equals(event.getPointerType())); } else { return (event.getNativeEvent().getTouches().length() == 1); } @@ -1025,8 +1026,8 @@ public class Escalator extends Widget }-*/; /** - * Using pointerdown, pointermove, pointerup, and pointercancel for IE11 and Edge instead of - * touch* listeners (#18737) + * Using pointerdown, pointermove, pointerup, and pointercancel for IE11 + * and Edge instead of touch* listeners (#18737) * * @param element */ @@ -1047,8 +1048,8 @@ public class Escalator extends Widget }-*/; /** - * Using pointerdown, pointermove, pointerup, and pointercancel for IE11 and Edge instead of - * touch* listeners (#18737) + * Using pointerdown, pointermove, pointerup, and pointercancel for IE11 + * and Edge instead of touch* listeners (#18737) * * @param element */ @@ -1134,7 +1135,7 @@ public class Escalator extends Widget } } - protected abstract class AbstractRowContainer implements RowContainer { + public abstract class AbstractRowContainer implements RowContainer { private EscalatorUpdater updater = EscalatorUpdater.NULL; private int rows; @@ -2160,7 +2161,14 @@ public class Escalator extends Widget */ protected abstract double getHeightOfSection(); - protected int getLogicalRowIndex(final TableRowElement tr) { + /** + * Gets the logical row index for the given table row element. + * + * @param tr + * the table row element inside this container. + * @return the logical index of the given element + */ + public int getLogicalRowIndex(final TableRowElement tr) { return tr.getSectionRowIndex(); }; @@ -3469,7 +3477,7 @@ public class Escalator extends Widget } @Override - protected int getLogicalRowIndex(final TableRowElement tr) { + public int getLogicalRowIndex(final TableRowElement tr) { assert tr .getParentNode() == root : "The given element isn't a row element in the body"; int internalIndex = visualRowOrder.indexOf(tr); @@ -6905,6 +6913,7 @@ public class Escalator extends Widget /** * Internal method for checking whether the browser is IE11 or Edge + * * @return true only if the current browser is IE11, or Edge */ private static boolean isCurrentBrowserIE11OrEdge() { diff --git a/client/src/main/java/com/vaadin/client/widgets/Grid.java b/client/src/main/java/com/vaadin/client/widgets/Grid.java index 889216e673..8c71b3fe66 100644 --- a/client/src/main/java/com/vaadin/client/widgets/Grid.java +++ b/client/src/main/java/com/vaadin/client/widgets/Grid.java @@ -5799,11 +5799,11 @@ public class Grid extends ResizeComposite implements HasSelectionHandlers, public void onStart() { dragStarted(); dragger.getElement().appendChild(resizeElement); - resizeElement.getStyle() - .setLeft((dragger.getElement() - .getOffsetWidth() + resizeElement.getStyle().setLeft( + (dragger.getElement().getOffsetWidth() - resizeElement.getOffsetWidth()) - * .5, Unit.PX); + * .5, + Unit.PX); resizeElement.getStyle().setHeight( col.grid.getOffsetHeight(), Unit.PX); } @@ -6844,7 +6844,12 @@ public class Grid extends ResizeComposite implements HasSelectionHandlers, return editor; } - protected Escalator getEscalator() { + /** + * Gets the {@link Escalator} used by this Grid instnace. + * + * @return the escalator instance, never null + */ + public Escalator getEscalator() { return escalator; } diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/basicfeatures/server/GridSelectionTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/basicfeatures/server/GridSelectionTest.java index a1dc542250..54dd1e37ab 100644 --- a/uitest/src/test/java/com/vaadin/tests/components/grid/basicfeatures/server/GridSelectionTest.java +++ b/uitest/src/test/java/com/vaadin/tests/components/grid/basicfeatures/server/GridSelectionTest.java @@ -18,6 +18,10 @@ package com.vaadin.tests.components.grid.basicfeatures.server; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import java.util.Arrays; +import java.util.HashSet; + +import org.junit.Assert; import org.junit.Test; import org.openqa.selenium.Keys; import org.openqa.selenium.WebDriver; @@ -31,6 +35,7 @@ import com.vaadin.testbench.elements.GridElement.GridCellElement; import com.vaadin.testbench.elements.GridElement.GridRowElement; import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeatures; import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeaturesTest; +import com.vaadin.tests.components.grid.basicfeatures.element.CustomGridElement; public class GridSelectionTest extends GridBasicFeaturesTest { @@ -396,6 +401,52 @@ public class GridSelectionTest extends GridBasicFeaturesTest { "Exception occured, java.lang.NullPointerException: null")); } + @Test + public void testRemoveSelectedRowMulti() { + openTestURL(); + + setSelectionModelMulti(); + CustomGridElement grid = getGridElement(); + grid.getCell(5, 0).click(); + + selectMenuPath("Component", "Body rows", "Remove selected rows"); + assertSelected(); + grid.getCell(5, 0).click(); + assertSelected(5); + grid.getCell(6, 0).click(); + assertSelected(5, 6); + grid.getCell(5, 0).click(); + assertSelected(6); + grid.getCell(5, 0).click(); + grid.getCell(4, 0).click(); + selectMenuPath("Component", "Body rows", "Remove selected rows"); + assertSelected(); + grid.getCell(0, 0).click(); + assertSelected(0); + grid.getCell(5, 0).click(); + assertSelected(0, 5); + grid.getCell(6, 0).click(); + assertSelected(0, 5, 6); + + } + + private void assertSelected(Integer... selected) { + CustomGridElement grid = getGridElement(); + HashSet expected = new HashSet( + Arrays.asList(selected)); + for (int i = 0; i < 10; i++) { + boolean rowSelected = grid.getRow(i).isSelected(); + if (expected.contains(i)) { + Assert.assertTrue("Expected row " + i + " to be selected", + rowSelected); + } else { + Assert.assertFalse("Expected row " + i + " not to be selected", + rowSelected); + } + } + + } + private void waitUntilCheckBoxValue(final WebElement checkBoxElememnt, final boolean expectedValue) { waitUntil(new ExpectedCondition() {