This patch reverts the fix #10557 and replaces it with a proper solution from Grid perspective. Fixes #10987 Fixes #10985tags/8.5.0.alpha2
@@ -72,8 +72,11 @@ public class EditorConnector extends AbstractExtensionConnector { | |||
@Override | |||
public void cancel() { | |||
serverInitiated = true; | |||
getParent().getWidget().cancelEditor(); | |||
// Canceling an editor that is not open is a no-op. | |||
if (getParent().getWidget().isEditorActive()) { | |||
serverInitiated = true; | |||
getParent().getWidget().cancelEditor(); | |||
} | |||
} | |||
@Override |
@@ -220,11 +220,6 @@ public class DataCommunicator<T> extends AbstractExtension { | |||
public void attach() { | |||
super.attach(); | |||
attachDataProviderListener(); | |||
if (getPushRows().isEmpty()) { | |||
// Make sure rows are pushed when component is attached. | |||
setPushRows(Range.withLength(0, getMinPushSize())); | |||
} | |||
} | |||
@Override | |||
@@ -313,6 +308,11 @@ public class DataCommunicator<T> extends AbstractExtension { | |||
public void beforeClientResponse(boolean initial) { | |||
super.beforeClientResponse(initial); | |||
if (initial && getPushRows().isEmpty()) { | |||
// Make sure rows are pushed when component is attached. | |||
setPushRows(Range.withLength(0, getMinPushSize())); | |||
} | |||
sendDataToClient(initial); | |||
} | |||
@@ -2507,6 +2507,16 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents, | |||
.forEach(this::addColumn); | |||
} | |||
@Override | |||
public void beforeClientResponse(boolean initial) { | |||
super.beforeClientResponse(initial); | |||
if (initial && editor.isOpen()) { | |||
// Re-attaching grid. Any old editor should be closed. | |||
editor.cancel(); | |||
} | |||
} | |||
/** | |||
* Sets the property set to use for this grid. Does not create or update | |||
* columns in any way but will delete and re-create the editor. | |||
@@ -4776,20 +4786,4 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents, | |||
column.updateSortable(); | |||
} | |||
} | |||
@Override | |||
public void setVisible(boolean visible) { | |||
if (getEditor().isOpen() && !visible) { | |||
getEditor().cancel(); | |||
} | |||
super.setVisible(visible); | |||
} | |||
@Override | |||
public void detach() { | |||
if (getEditor().isOpen()) { | |||
getEditor().cancel(); | |||
} | |||
super.detach(); | |||
} | |||
} |
@@ -585,12 +585,6 @@ public class TabSheet extends AbstractComponentContainer | |||
*/ | |||
private void setSelected(Component component) { | |||
Tab tab = tabs.get(selected); | |||
if (tab != null && !Objects.equals(tab.getComponent(), component) | |||
&& tab.getComponent() != null | |||
&& tab.getComponent().isAttached()) { | |||
tab.getComponent().detach(); | |||
tab.getComponent().attach(); // ugly hack | |||
} | |||
selected = component; | |||
// Repaint of the selected component is needed as only the selected |
@@ -30,9 +30,13 @@ public class AbstractListingTest { | |||
/** | |||
* Used to execute data generation | |||
* | |||
* @param initial | |||
* {@code true} to mock initial data request; {@code false} | |||
* for follow-up request. | |||
*/ | |||
public void runDataGeneration() { | |||
super.getDataCommunicator().beforeClientResponse(true); | |||
public void runDataGeneration(boolean initial) { | |||
super.getDataCommunicator().beforeClientResponse(initial); | |||
} | |||
@Override | |||
@@ -129,7 +133,7 @@ public class AbstractListingTest { | |||
CountGenerator generator = new CountGenerator(); | |||
generator.extend(listing); | |||
listing.setItems("Foo"); | |||
listing.runDataGeneration(); | |||
listing.runDataGeneration(true); | |||
assertEquals("Generator should have been called once", 1, | |||
generator.callCount); | |||
} | |||
@@ -139,7 +143,7 @@ public class AbstractListingTest { | |||
CountGenerator generator = new CountGenerator(); | |||
listing.setItems("Foo"); | |||
generator.extend(listing); | |||
listing.runDataGeneration(); | |||
listing.runDataGeneration(true); | |||
assertEquals("Generator should have been called once", 1, | |||
generator.callCount); | |||
} | |||
@@ -149,10 +153,10 @@ public class AbstractListingTest { | |||
listing.setItems("Foo"); | |||
CountGenerator generator = new CountGenerator(); | |||
generator.extend(listing); | |||
listing.runDataGeneration(); | |||
listing.runDataGeneration(true); | |||
assertEquals("Generator should have been called once", 1, | |||
generator.callCount); | |||
listing.runDataGeneration(); | |||
listing.runDataGeneration(false); | |||
assertEquals("Generator should not have been called again", 1, | |||
generator.callCount); | |||
} | |||
@@ -163,7 +167,7 @@ public class AbstractListingTest { | |||
CountGenerator generator = new CountGenerator(); | |||
generator.extend(listing); | |||
generator.remove(); | |||
listing.runDataGeneration(); | |||
listing.runDataGeneration(true); | |||
assertEquals("Generator should not have been called", 0, | |||
generator.callCount); | |||
} | |||
@@ -173,11 +177,11 @@ public class AbstractListingTest { | |||
listing.setItems("Foo"); | |||
CountGenerator generator = new CountGenerator(); | |||
generator.extend(listing); | |||
listing.runDataGeneration(); | |||
listing.runDataGeneration(true); | |||
assertEquals("Generator should have been called once", 1, | |||
generator.callCount); | |||
generator.refresh("Foo"); | |||
listing.runDataGeneration(); | |||
listing.runDataGeneration(false); | |||
assertEquals("Generator should have been called again", 2, | |||
generator.callCount); | |||
} |
@@ -7,6 +7,7 @@ import java.util.stream.IntStream; | |||
import com.vaadin.annotations.Widgetset; | |||
import com.vaadin.data.ValueProvider; | |||
import com.vaadin.data.converter.StringToIntegerConverter; | |||
import com.vaadin.data.provider.DataProvider; | |||
import com.vaadin.data.provider.ListDataProvider; | |||
import com.vaadin.server.VaadinRequest; | |||
@@ -15,6 +16,7 @@ import com.vaadin.ui.Button; | |||
import com.vaadin.ui.Grid.SelectionMode; | |||
import com.vaadin.ui.Label; | |||
import com.vaadin.ui.TabSheet; | |||
import com.vaadin.ui.TextField; | |||
import com.vaadin.ui.renderers.NumberRenderer; | |||
@Widgetset("com.vaadin.DefaultWidgetSet") | |||
@@ -48,7 +50,12 @@ public class GridInTabSheet extends AbstractTestUIWithLog { | |||
final Grid<Integer> grid = new Grid<>(); | |||
grid.setSelectionMode(SelectionMode.MULTI); | |||
grid.addColumn(ValueProvider.identity(), new NumberRenderer()) | |||
.setId("count"); | |||
.setId("count").setEditorBinding( | |||
grid.getEditor().getBinder().forField(new TextField()) | |||
.withConverter(new StringToIntegerConverter("")) | |||
.bind(i -> i, (i, v) -> { | |||
})); | |||
grid.getEditor().setEnabled(true); | |||
LinkedList<Integer> items = IntStream.range(0, 3).boxed() | |||
.collect(Collectors.toCollection(LinkedList::new)); |
@@ -2,11 +2,12 @@ package com.vaadin.tests.components.grid; | |||
import static org.junit.Assert.assertEquals; | |||
import static org.junit.Assert.assertFalse; | |||
import static org.junit.Assert.assertNotEquals; | |||
import static org.junit.Assert.assertTrue; | |||
import org.junit.Test; | |||
import org.openqa.selenium.Keys; | |||
import com.vaadin.testbench.By; | |||
import com.vaadin.testbench.elements.ButtonElement; | |||
import com.vaadin.testbench.elements.GridElement; | |||
import com.vaadin.testbench.elements.NotificationElement; | |||
@@ -85,6 +86,39 @@ public class GridInTabSheetTest extends MultiBrowserTest { | |||
assertNoNotification(); | |||
} | |||
@Test | |||
public void testEditorOpenWhenSwitchingTab() { | |||
setDebug(true); | |||
openTestURL(); | |||
GridElement grid = $(GridElement.class).first(); | |||
grid.getCell(0, 1).doubleClick(); | |||
assertEquals("Editor should be open", "0", | |||
grid.getEditor().getField(1).getAttribute("value")); | |||
TabSheetElement tabsheet = $(TabSheetElement.class).first(); | |||
tabsheet.openTab("Label"); | |||
tabsheet.openTab("Grid"); | |||
grid = $(GridElement.class).first(); | |||
assertFalse("Editor should be closed.", | |||
grid.isElementPresent(By.vaadin("#editor"))); | |||
grid.getCell(1, 1).doubleClick(); | |||
assertEquals("Editor should open after tab switch", "1", | |||
grid.getEditor().getField(1).getAttribute("value")); | |||
// Close the current editor and reopen on a different row | |||
grid.sendKeys(Keys.ESCAPE); | |||
grid.getCell(0, 1).doubleClick(); | |||
assertEquals("Editor should move", "0", | |||
grid.getEditor().getField(1).getAttribute("value")); | |||
assertNoErrorNotifications(); | |||
} | |||
private void removeGridRow() { | |||
$(ButtonElement.class).caption("Remove row from Grid").first().click(); | |||
} |