From 91f14489833ccad2f3c5aeaabfd4d5c4a5a60728 Mon Sep 17 00:00:00 2001 From: Teemu Suo-Anttila Date: Mon, 28 Jul 2014 12:25:32 +0300 Subject: [PATCH] Fix SelectionModelSingle when updating from server (#13334) Change-Id: I4ae1411b3833387e83c8951f9b08528cf1ba97b9 --- .../grid/selection/SelectionModelSingle.java | 36 ++++------ .../grid/basicfeatures/GridSelectionTest.java | 65 ++++++++++++++----- 2 files changed, 62 insertions(+), 39 deletions(-) diff --git a/client/src/com/vaadin/client/ui/grid/selection/SelectionModelSingle.java b/client/src/com/vaadin/client/ui/grid/selection/SelectionModelSingle.java index 2647ae9c56..2942538d81 100644 --- a/client/src/com/vaadin/client/ui/grid/selection/SelectionModelSingle.java +++ b/client/src/com/vaadin/client/ui/grid/selection/SelectionModelSingle.java @@ -68,20 +68,13 @@ public class SelectionModelSingle extends AbstractRowHandleSelectionModel throw new IllegalArgumentException("Row cannot be null"); } - if (isSelected(row)) { - return false; - } - T removed = getSelectedRow(); - if (selectedRow != null) { - selectedRow.unpin(); - } - selectedRow = grid.getDataSource().getHandle(row); - selectedRow.pin(); + if (selectByHandle(grid.getDataSource().getHandle(row))) { + grid.fireEvent(new SelectionChangeEvent(grid, row, removed)); - grid.fireEvent(new SelectionChangeEvent(grid, row, removed)); - - return true; + return true; + } + return false; } @Override @@ -92,10 +85,8 @@ public class SelectionModelSingle extends AbstractRowHandleSelectionModel } if (isSelected(row)) { - T removed = selectedRow.getRow(); - selectedRow.unpin(); - selectedRow = null; - grid.fireEvent(new SelectionChangeEvent(grid, null, removed)); + deselectByHandle(selectedRow); + grid.fireEvent(new SelectionChangeEvent(grid, null, row)); return true; } @@ -109,10 +100,8 @@ public class SelectionModelSingle extends AbstractRowHandleSelectionModel @Override public void reset() { - T removed = getSelectedRow(); - - if (removed != null) { - deselect(removed); + if (selectedRow != null) { + deselect(getSelectedRow()); } } @@ -126,8 +115,10 @@ public class SelectionModelSingle extends AbstractRowHandleSelectionModel @Override protected boolean selectByHandle(RowHandle handle) { - if (!handle.equals(selectedRow)) { + if (handle != null && !handle.equals(selectedRow)) { + deselectByHandle(selectedRow); selectedRow = handle; + selectedRow.pin(); return true; } else { return false; @@ -136,7 +127,8 @@ public class SelectionModelSingle extends AbstractRowHandleSelectionModel @Override protected boolean deselectByHandle(RowHandle handle) { - if (handle.equals(selectedRow)) { + if (handle != null && handle.equals(selectedRow)) { + selectedRow.unpin(); selectedRow = null; return true; } else { diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridSelectionTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridSelectionTest.java index e18dc1faa4..873c222f80 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridSelectionTest.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridSelectionTest.java @@ -21,7 +21,8 @@ import static org.junit.Assert.assertTrue; import org.junit.Test; import com.vaadin.testbench.TestBenchElement; -import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeaturesTest; +import com.vaadin.tests.components.grid.GridElement; +import com.vaadin.tests.components.grid.GridElement.GridRowElement; public class GridSelectionTest extends GridBasicFeaturesTest { @@ -31,12 +32,12 @@ public class GridSelectionTest extends GridBasicFeaturesTest { setSelectionModelMulti(); - assertFalse("row shouldn't start out as selected", - isSelected(getRow(0))); + assertFalse("row shouldn't start out as selected", getRow(0) + .isSelected()); toggleFirstRowSelection(); - assertTrue("row should become selected", isSelected(getRow(0))); + assertTrue("row should become selected", getRow(0).isSelected()); toggleFirstRowSelection(); - assertFalse("row shouldn't remain selected", isSelected(getRow(0))); + assertFalse("row shouldn't remain selected", getRow(0).isSelected()); } @Test @@ -45,16 +46,16 @@ public class GridSelectionTest extends GridBasicFeaturesTest { setSelectionModelMulti(); - assertFalse("row shouldn't start out as selected", - isSelected(getRow(0))); + assertFalse("row shouldn't start out as selected", getRow(0) + .isSelected()); toggleFirstRowSelection(); - assertTrue("row should become selected", isSelected(getRow(0))); + assertTrue("row should become selected", getRow(0).isSelected()); scrollGridVerticallyTo(10000); // make sure the row is out of cache scrollGridVerticallyTo(0); // scroll it back into view assertTrue("row should still be selected when scrolling " - + "back into view", isSelected(getRow(0))); + + "back into view", getRow(0).isSelected()); } @Test @@ -63,18 +64,18 @@ public class GridSelectionTest extends GridBasicFeaturesTest { setSelectionModelMulti(); - assertFalse("row shouldn't start out as selected", - isSelected(getRow(0))); + assertFalse("row shouldn't start out as selected", getRow(0) + .isSelected()); scrollGridVerticallyTo(10000); // make sure the row is out of cache toggleFirstRowSelection(); scrollGridVerticallyTo(0); // scroll it back into view assertTrue("row should still be selected when scrolling " - + "back into view", isSelected(getRow(0))); + + "back into view", getRow(0).isSelected()); toggleFirstRowSelection(); - assertFalse("row shouldn't remain selected", isSelected(getRow(0))); + assertFalse("row shouldn't remain selected", getRow(0).isSelected()); } @Test @@ -83,8 +84,8 @@ public class GridSelectionTest extends GridBasicFeaturesTest { setSelectionModelMulti(); - assertFalse("row shouldn't start out as selected", - isSelected(getRow(0))); + assertFalse("row shouldn't start out as selected", getRow(0) + .isSelected()); scrollGridVerticallyTo(10000); // make sure the row is out of cache toggleFirstRowSelection(); @@ -92,13 +93,43 @@ public class GridSelectionTest extends GridBasicFeaturesTest { scrollGridVerticallyTo(0); // make sure the row is out of cache assertFalse("row shouldn't be selected when scrolling " - + "back into view", isSelected(getRow(0))); + + "back into view", getRow(0).isSelected()); + } + + @Test + public void testSingleSelectionUpdatesFromServer() { + openTestURL(); + setSelectionModelSingle(); + + GridElement grid = getGridElement(); + assertFalse("First row was selected from start", grid.getRow(0) + .isSelected()); + toggleFirstRowSelection(); + assertTrue("First row was not selected.", getRow(0).isSelected()); + grid.getCell(5, 0).click(); + assertTrue("Fifth row was not selected.", getRow(5).isSelected()); + assertFalse("First row was still selected.", getRow(0).isSelected()); + grid.getCell(0, 0).click(); + toggleFirstRowSelection(); + assertFalse("First row was still selected.", getRow(0).isSelected()); + assertFalse("Fifth row was still selected.", getRow(5).isSelected()); + + grid.scrollToRow(600); + grid.getCell(595, 0).click(); + assertTrue("Row 595 was not selected.", getRow(595).isSelected()); + toggleFirstRowSelection(); + assertFalse("Row 595 was still selected.", getRow(595).isSelected()); + assertTrue("First row was not selected.", getRow(0).isSelected()); } private void setSelectionModelMulti() { selectMenuPath("Component", "State", "Selection mode", "multi"); } + private void setSelectionModelSingle() { + selectMenuPath("Component", "State", "Selection mode", "single"); + } + @SuppressWarnings("static-method") private boolean isSelected(TestBenchElement row) { /* @@ -113,7 +144,7 @@ public class GridSelectionTest extends GridBasicFeaturesTest { selectMenuPath("Component", "Body rows", "Select first row"); } - private TestBenchElement getRow(int i) { + private GridRowElement getRow(int i) { return getGridElement().getRow(i); } } -- 2.39.5