From 38a4ae4130b0e82292bf92d055d0d9784b7de029 Mon Sep 17 00:00:00 2001 From: Teemu Suo-Anttila Date: Fri, 11 Dec 2015 11:39:16 +0200 Subject: [PATCH] Fix selecting a row that was deselected on the server (#19360) Client-side connector of the SingleSelectionModel attempts to keep track of currently selected row. This tracking gets lost when the row get deselected on the server-side. Special case is now correctly handled. Change-Id: I1c45548bd11536bc85cddbc2ba8b6225965c1194 --- .../SingleSelectionModelConnector.java | 42 +++++++-- .../grid/GridDefaultSelectionMode.java | 94 +++++++++++++++++++ .../grid/GridDefaultSelectionModeTest.java | 84 +++++++++++++++++ 3 files changed, 210 insertions(+), 10 deletions(-) create mode 100644 uitest/src/com/vaadin/tests/components/grid/GridDefaultSelectionMode.java create mode 100644 uitest/src/com/vaadin/tests/components/grid/GridDefaultSelectionModeTest.java diff --git a/client/src/com/vaadin/client/connectors/SingleSelectionModelConnector.java b/client/src/com/vaadin/client/connectors/SingleSelectionModelConnector.java index b963014256..f8c2854815 100644 --- a/client/src/com/vaadin/client/connectors/SingleSelectionModelConnector.java +++ b/client/src/com/vaadin/client/connectors/SingleSelectionModelConnector.java @@ -92,21 +92,29 @@ public class SingleSelectionModelConnector extends @Override public boolean select(JsonObject row) { boolean changed = false; - if ((row == null && isDeselectAllowed()) - || (row != null && !getRowHandle(row).equals(selectedRow))) { + + if (row == null && !isDeselectAllowed()) { + // Attempting to deselect, even though it's not allowed. + } else { if (selectedRow != null) { - selectedRow.getRow().remove(GridState.JSONKEY_SELECTED); - selectedRow.updateRow(); - selectedRow.unpin(); - selectedRow = null; + // Check if currently re-selected row was deselected from + // the server. + if (row != null && getRowHandle(row).equals(selectedRow)) { + if (selectedRow.getRow().hasKey( + GridState.JSONKEY_SELECTED)) { + // Everything is OK, no need to do anything. + return false; + } + } + + // Remove old selected row + clearSelectedRow(); changed = true; } if (row != null) { - selectedRow = getRowHandle(row); - selectedRow.pin(); - selectedRow.getRow().put(GridState.JSONKEY_SELECTED, true); - selectedRow.updateRow(); + // Select the new row. + setSelectedRow(row); changed = true; } } @@ -119,6 +127,20 @@ public class SingleSelectionModelConnector extends return changed; } + private void setSelectedRow(JsonObject row) { + selectedRow = getRowHandle(row); + selectedRow.pin(); + selectedRow.getRow().put(GridState.JSONKEY_SELECTED, true); + selectedRow.updateRow(); + } + + private void clearSelectedRow() { + selectedRow.getRow().remove(GridState.JSONKEY_SELECTED); + selectedRow.updateRow(); + selectedRow.unpin(); + selectedRow = null; + } + @Override public boolean deselect(JsonObject row) { if (getRowHandle(row).equals(selectedRow)) { diff --git a/uitest/src/com/vaadin/tests/components/grid/GridDefaultSelectionMode.java b/uitest/src/com/vaadin/tests/components/grid/GridDefaultSelectionMode.java new file mode 100644 index 0000000000..e2f2704a5d --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/grid/GridDefaultSelectionMode.java @@ -0,0 +1,94 @@ +/* + * 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.grid; + +import java.util.ArrayList; + +import com.vaadin.data.util.BeanItemContainer; +import com.vaadin.server.VaadinRequest; +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.Grid; +import com.vaadin.ui.VerticalLayout; + +public class GridDefaultSelectionMode extends AbstractTestUI { + + @Override + protected void setup(VaadinRequest request) { + final Person person1 = new Person(); + person1.setFirstName("person"); + person1.setLastName("one"); + + Person person2 = new Person(); + person2.setFirstName("person"); + person2.setLastName("two"); + + ArrayList items = new ArrayList(); + items.add(person1); + items.add(person2); + + final BeanItemContainer container = new BeanItemContainer( + Person.class, items); + + final Grid grid = new Grid(); + grid.setContainerDataSource(container); + + VerticalLayout v = new VerticalLayout(); + + v.addComponent(new Button("Deselect on server", new ClickListener() { + + @Override + public void buttonClick(ClickEvent event) { + grid.select(null); + } + })); + + v.addComponent(new Button("Select on server", new ClickListener() { + + @Override + public void buttonClick(ClickEvent event) { + grid.select(person1); + } + })); + v.addComponent(grid); + + addComponent(v); + } + + public static class Person { + private String firstName; + private String lastName; + + public String getFirstName() { + return firstName; + } + + public void setFirstName(String firstName) { + this.firstName = firstName; + } + + public String getLastName() { + return lastName; + } + + public void setLastName(String lastName) { + this.lastName = lastName; + } + } + +} diff --git a/uitest/src/com/vaadin/tests/components/grid/GridDefaultSelectionModeTest.java b/uitest/src/com/vaadin/tests/components/grid/GridDefaultSelectionModeTest.java new file mode 100644 index 0000000000..df306ee1e6 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/grid/GridDefaultSelectionModeTest.java @@ -0,0 +1,84 @@ +/* + * 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.grid; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +import com.vaadin.testbench.elements.ButtonElement; +import com.vaadin.testbench.elements.GridElement; +import com.vaadin.testbench.elements.GridElement.GridCellElement; +import com.vaadin.tests.tb3.MultiBrowserTest; + +public class GridDefaultSelectionModeTest extends MultiBrowserTest { + + @Test + public void testSelectionFromServer() { + setDebug(true); + openTestURL(); + + $(ButtonElement.class).caption("Select on server").first().click(); + + assertTrue("Row should be selected.", $(GridElement.class).first() + .getRow(0).isSelected()); + + $(ButtonElement.class).caption("Deselect on server").first().click(); + + assertFalse("Row should not be selected.", $(GridElement.class).first() + .getRow(0).isSelected()); + + assertNoErrorNotifications(); + } + + @Test + public void testSelectionWithSort() { + setDebug(true); + openTestURL(); + + GridElement grid = $(GridElement.class).first(); + grid.getCell(0, 0).click(); + + GridCellElement header = grid.getHeaderCell(0, 1); + header.click(); + header.click(); + + assertTrue("Row should be selected.", grid.getRow(1).isSelected()); + + assertNoErrorNotifications(); + } + + @Test + public void testReselectDeselectedRow() { + setDebug(true); + openTestURL(); + + $(ButtonElement.class).caption("Select on server").first().click(); + + GridElement grid = $(GridElement.class).first(); + assertTrue("Row should be selected.", grid.getRow(0).isSelected()); + + $(ButtonElement.class).caption("Deselect on server").first().click(); + + assertFalse("Row should not be selected.", grid.getRow(0).isSelected()); + + grid.getCell(0, 0).click(); + assertTrue("Row should be selected.", grid.getRow(0).isSelected()); + + assertNoErrorNotifications(); + } +} -- 2.39.5