Handle possible race condition by disabling the editor's widget while awaiting validation from the server. Fixes #12270tags/8.14.0.alpha1
@@ -29,7 +29,6 @@ import com.vaadin.client.extensions.AbstractExtensionConnector; | |||
import com.vaadin.client.widget.grid.EditorHandler; | |||
import com.vaadin.client.widgets.Grid; | |||
import com.vaadin.client.widgets.Grid.Column; | |||
import com.vaadin.shared.Range; | |||
import com.vaadin.shared.data.DataCommunicatorConstants; | |||
import com.vaadin.shared.ui.Connect; | |||
import com.vaadin.shared.ui.grid.editor.EditorClientRpc; | |||
@@ -104,6 +103,12 @@ public class EditorConnector extends AbstractExtensionConnector { | |||
getParent().getWidget().getEditor() | |||
.setEditorError(errorMessage, errorColumns); | |||
} | |||
@Override | |||
public void confirmValidity(boolean isValid) { | |||
getParent().getWidget().getEditor().getEventHandler() | |||
.confirmValidity(isValid); | |||
} | |||
}); | |||
} | |||
@@ -130,6 +135,11 @@ public class EditorConnector extends AbstractExtensionConnector { | |||
currentEditedRow = null; | |||
} | |||
@Override | |||
public void checkValidity() { | |||
rpc.checkValidity(); | |||
} | |||
@Override | |||
public Widget getWidget(Column<?, JsonObject> column) { | |||
String connId = getState().columnFields |
@@ -15,6 +15,10 @@ | |||
*/ | |||
package com.vaadin.client.widget.grid; | |||
import java.util.List; | |||
import java.util.logging.Level; | |||
import java.util.logging.Logger; | |||
import com.google.gwt.core.client.Duration; | |||
import com.google.gwt.dom.client.BrowserEvents; | |||
import com.google.gwt.dom.client.Element; | |||
@@ -30,8 +34,6 @@ import com.vaadin.client.widgets.Grid; | |||
import com.vaadin.client.widgets.Grid.Editor; | |||
import com.vaadin.client.widgets.Grid.EditorDomEvent; | |||
import java.util.List; | |||
/** | |||
* The default handler for Grid editor events. Offers several overridable | |||
* protected methods for easier customization. | |||
@@ -51,6 +53,7 @@ public class DefaultEditorEventHandler<T> implements Editor.EventHandler<T> { | |||
private int lastTouchEventX = -1; | |||
private int lastTouchEventY = -1; | |||
private int lastTouchEventRow = -1; | |||
private PendingEdit pendingEdit; | |||
/** | |||
* Returns whether the given event is a touch event that should open the | |||
@@ -223,7 +226,16 @@ public class DefaultEditorEventHandler<T> implements Editor.EventHandler<T> { | |||
} | |||
} | |||
editRow(event, rowIndex + delta.rowDelta, colIndex); | |||
int newRowIndex = rowIndex + delta.rowDelta; | |||
if (newRowIndex != event.getRowIndex()) { | |||
triggerValueChangeEvent(event); | |||
// disable until validity check is done | |||
setWidgetEnabled(event.getEditorWidget(), false); | |||
event.getEditor().getHandler().checkValidity(); | |||
pendingEdit = new PendingEdit(event, newRowIndex, colIndex); | |||
} else { | |||
editRow(event, newRowIndex, colIndex); | |||
} | |||
} | |||
return changed; | |||
@@ -401,10 +413,6 @@ public class DefaultEditorEventHandler<T> implements Editor.EventHandler<T> { | |||
// Limit colIndex between 0 and colCount - 1 | |||
colIndex = Math.max(0, Math.min(colCount - 1, colIndex)); | |||
if (rowIndex != event.getRowIndex()) { | |||
triggerValueChangeEvent(event); | |||
} | |||
event.getEditor().editRow(rowIndex, colIndex); | |||
} | |||
@@ -451,4 +459,53 @@ public class DefaultEditorEventHandler<T> implements Editor.EventHandler<T> { | |||
return handled || swallowEvent; | |||
} | |||
@Override | |||
public void confirmValidity(boolean isValid) { | |||
if (pendingEdit == null) { | |||
getLogger().log(Level.SEVERE, | |||
"An editor's validation confirmation was received, but" | |||
+ " no pending edit object was found "); | |||
return; | |||
} | |||
setWidgetEnabled(pendingEdit.pendingEvent.getEditorWidget(), true); | |||
if (isValid) { | |||
editRow(pendingEdit.pendingEvent, pendingEdit.pendingRowIndex, | |||
pendingEdit.pendingColIndex); | |||
} else { | |||
pendingEdit.pendingEvent.getEditorWidget().getElement().focus(); | |||
} | |||
pendingEdit = null; | |||
} | |||
private void setWidgetEnabled(Widget widget, boolean widgetEnabled) { | |||
final ComponentConnector connector = Util.findConnectorFor(widget); | |||
// only enable widget if it hasn't been disabled programmatically | |||
if (connector.getState().enabled) { | |||
connector.setWidgetEnabled(widgetEnabled); | |||
} | |||
} | |||
private static final Logger getLogger() { | |||
return Logger.getLogger(DefaultEditorEventHandler.class.getName()); | |||
} | |||
private final class PendingEdit { | |||
private EditorDomEvent<T> pendingEvent; | |||
private int pendingRowIndex; | |||
private int pendingColIndex; | |||
private PendingEdit(EditorDomEvent<T> pendingEvent, int pendingRowIndex, | |||
int pendingColIndex) { | |||
if (pendingEvent == null) { | |||
throw new IllegalArgumentException( | |||
"The pending event cannot be null"); | |||
} | |||
this.pendingEvent = pendingEvent; | |||
this.pendingRowIndex = pendingRowIndex; | |||
this.pendingColIndex = pendingColIndex; | |||
} | |||
} | |||
} |
@@ -189,4 +189,11 @@ public interface EditorHandler<T> { | |||
* editable | |||
*/ | |||
public Widget getWidget(Grid.Column<?, T> column); | |||
/** | |||
* Called by the editor's event handler when editing is shifting to a new | |||
* row in order to check the validity of the binder's value. | |||
* | |||
*/ | |||
public void checkValidity(); | |||
} |
@@ -1416,6 +1416,15 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>, | |||
* done, false otherwise | |||
*/ | |||
boolean handleEvent(EditorDomEvent<T> event); | |||
/** | |||
* Confirms the valid status of the binder so as to determine | |||
* whether to allow pending navigation action. | |||
* | |||
* @param isValid | |||
* {@code true} if the binder value is valid | |||
*/ | |||
void confirmValidity(boolean isValid); | |||
} | |||
protected enum State { |
@@ -151,6 +151,11 @@ public class EditorImpl<T> extends AbstractGridExtension<T> | |||
rpc.confirmBind(true); | |||
doEdit(getData(key)); | |||
} | |||
@Override | |||
public void checkValidity() { | |||
rpc.confirmValidity(getBinder().validate().isOk()); | |||
} | |||
}); | |||
setBinder(Binder.withPropertySet(propertySet)); |
@@ -70,4 +70,13 @@ public interface EditorClientRpc extends ClientRpc { | |||
* | |||
*/ | |||
void setErrorMessage(String errorMessage, List<String> errorColumnsIds); | |||
/** | |||
* Confirms whether the binder's validation has passed so as to determine | |||
* whether to allow the pending navigation action. | |||
* | |||
* @param isValid | |||
* {@code true} if the binder value is valid | |||
*/ | |||
void confirmValidity(boolean isValid); | |||
} |
@@ -53,4 +53,11 @@ public interface EditorServerRpc extends ServerRpc { | |||
* after save action, otherwise it represents a cancel action | |||
*/ | |||
void cancel(boolean afterBeingSaved); | |||
/** | |||
* Asks the server to check the validity of the current values in the | |||
* editor. When a check-validity request is sent, the server must respond | |||
* with a {@link EditorClientRpc#confirmValidity(boolean) confirm call}. | |||
*/ | |||
void checkValidity(); | |||
} |
@@ -0,0 +1,103 @@ | |||
package com.vaadin.tests.components.grid; | |||
import java.util.ArrayList; | |||
import java.util.Collection; | |||
import java.util.List; | |||
import java.util.Locale; | |||
import java.util.Random; | |||
import com.vaadin.data.Binder; | |||
import com.vaadin.data.Binder.Binding; | |||
import com.vaadin.server.VaadinRequest; | |||
import com.vaadin.tests.components.AbstractReindeerTestUIWithLog; | |||
import com.vaadin.tests.util.Person; | |||
import com.vaadin.tests.util.TestDataGenerator; | |||
import com.vaadin.ui.Grid; | |||
import com.vaadin.ui.Grid.Column; | |||
import com.vaadin.ui.TextField; | |||
import com.vaadin.ui.renderers.NumberRenderer; | |||
public class GridEditorNonBuffered extends AbstractReindeerTestUIWithLog { | |||
final static String VALIDATION_ERROR_MESSAGE = "Validator error. Name cannot be empty"; | |||
@Override | |||
protected void setup(VaadinRequest request) { | |||
Grid<Person> grid = createGrid(); | |||
grid.setItems(createTestData()); | |||
addComponent(grid); | |||
} | |||
protected Collection<Person> createTestData() { | |||
return createTestData(100); | |||
} | |||
protected Collection<Person> createTestData(int size) { | |||
Random r = new Random(0); | |||
List<Person> testData = new ArrayList<>(); | |||
for (int i = 0; i < size; i++) { | |||
Person person = new Person(); | |||
person.setFirstName(TestDataGenerator.getFirstName(r)); | |||
person.setLastName(TestDataGenerator.getLastName(r)); | |||
person.getAddress().setCity(TestDataGenerator.getCity(r)); | |||
person.setEmail(person.getFirstName().toLowerCase(Locale.ROOT) + "." | |||
+ person.getLastName().toLowerCase(Locale.ROOT) | |||
+ "@vaadin.com"); | |||
person.setPhoneNumber(TestDataGenerator.getPhoneNumber(r)); | |||
person.getAddress() | |||
.setPostalCode(TestDataGenerator.getPostalCode(r)); | |||
person.getAddress() | |||
.setStreetAddress(TestDataGenerator.getStreetAddress(r)); | |||
testData.add(person); | |||
} | |||
return testData; | |||
} | |||
protected Grid<Person> createGrid() { | |||
Grid<Person> grid = new Grid<>(); | |||
grid.addColumn(Person::getEmail).setCaption("Email").setId("email"); | |||
Column<Person, String> firstNameColumn = grid | |||
.addColumn(Person::getFirstName).setCaption("First Name") | |||
.setId("firstName"); | |||
Column<Person, String> lastNameColumn = grid | |||
.addColumn(Person::getLastName).setCaption("Last Name") | |||
.setId("lastName"); | |||
grid.addColumn(Person::getPhoneNumber).setCaption("Phone Number") | |||
.setId("phone"); | |||
grid.addColumn(person -> person.getAddress().getStreetAddress()) | |||
.setCaption("Street Address").setId("street"); | |||
grid.addColumn(person -> person.getAddress().getPostalCode(), | |||
new NumberRenderer()).setCaption("Postal Code").setId("zip"); | |||
grid.addColumn(person -> person.getAddress().getCity()) | |||
.setCaption("City").setId("city"); | |||
Binder<Person> binder = new Binder<>(); | |||
TextField firstNameEditor = new TextField(); | |||
Binding<Person, String> firstNamebinding = binder | |||
.forField(firstNameEditor) | |||
.withValidator(v -> (v != null && !v.isEmpty()), | |||
VALIDATION_ERROR_MESSAGE) | |||
.bind(Person::getFirstName, Person::setFirstName); | |||
firstNameColumn.setEditorBinding(firstNamebinding); | |||
TextField lastNameEditor = new TextField(); | |||
Binding<Person, String> lastNamebinding = binder | |||
.forField(lastNameEditor) | |||
.withValidator(v -> (v != null && !v.isEmpty()), | |||
VALIDATION_ERROR_MESSAGE) | |||
.bind(Person::getLastName, Person::setLastName); | |||
lastNameColumn.setEditorBinding(lastNamebinding); | |||
grid.getEditor().setBuffered(false); | |||
grid.getEditor().setEnabled(true); | |||
grid.getEditor().setBinder(binder); | |||
return grid; | |||
} | |||
} |
@@ -0,0 +1,99 @@ | |||
package com.vaadin.tests.components.grid; | |||
import static org.junit.Assert.assertEquals; | |||
import static org.junit.Assert.assertFalse; | |||
import static org.junit.Assert.assertTrue; | |||
import org.junit.Test; | |||
import org.openqa.selenium.By; | |||
import org.openqa.selenium.Keys; | |||
import org.openqa.selenium.WebElement; | |||
import org.openqa.selenium.interactions.Actions; | |||
import com.vaadin.testbench.elements.GridElement; | |||
import com.vaadin.testbench.elements.GridElement.GridCellElement; | |||
import com.vaadin.testbench.elements.NotificationElement; | |||
import com.vaadin.testbench.elements.TextFieldElement; | |||
import com.vaadin.testbench.parallel.TestCategory; | |||
import com.vaadin.tests.tb3.MultiBrowserTest; | |||
@TestCategory("grid") | |||
public class GridEditorNonBufferedTest extends MultiBrowserTest { | |||
@Override | |||
public void setup() throws Exception { | |||
super.setup(); | |||
setDebug(true); | |||
openTestURL(); | |||
} | |||
private void openEditor(int rowIndex, int cellIndex) { | |||
GridElement grid = $(GridElement.class).first(); | |||
GridCellElement cell = grid.getCell(rowIndex, cellIndex); | |||
new Actions(driver).moveToElement(cell).doubleClick().build().perform(); | |||
} | |||
@Test | |||
public void testEditor() { | |||
openEditor(5, 1); | |||
assertTrue("Editor should be opened with a TextField", | |||
isElementPresent(TextFieldElement.class)); | |||
assertFalse("Notification was present", | |||
isElementPresent(NotificationElement.class)); | |||
} | |||
@Test | |||
public void testEscClosesEditor() { | |||
openEditor(5, 1); | |||
assertTrue("Editor should be opened with a TextField", | |||
isElementPresent(TextFieldElement.class)); | |||
new Actions(getDriver()).sendKeys(Keys.ESCAPE).perform(); | |||
assertFalse("Editor should be closed", | |||
isElementPresent(TextFieldElement.class)); | |||
} | |||
@Test | |||
public void preventNavigationToNextRowIfEditorValueIsInvalid() { | |||
// Test with navigation to next row | |||
openEditor(5, 2); | |||
WebElement field = findElement(By.className("v-textfield-focus")); | |||
String selectAll = Keys.chord(Keys.CONTROL, "a"); | |||
field.sendKeys(selectAll); | |||
field.sendKeys(Keys.DELETE); | |||
field.sendKeys(Keys.TAB); | |||
String editorMessage = getEditorMessage(); | |||
assertEquals( | |||
"Last Name: " + GridEditorNonBuffered.VALIDATION_ERROR_MESSAGE, | |||
editorMessage); | |||
} | |||
@Test | |||
public void preventNavigationToPrevRowIfEditorValueIsInvalid() { | |||
// Test with navigation to previous row | |||
openEditor(5, 1); | |||
WebElement field = findElement(By.className("v-textfield-focus")); | |||
String selectAll = Keys.chord(Keys.CONTROL, "a"); | |||
field.sendKeys(selectAll); | |||
field.sendKeys(Keys.DELETE); | |||
field.sendKeys(Keys.chord(Keys.SHIFT, Keys.TAB)); | |||
String editorMessage = getEditorMessage(); | |||
assertEquals( | |||
"First Name: " + GridEditorNonBuffered.VALIDATION_ERROR_MESSAGE, | |||
editorMessage); | |||
} | |||
private String getEditorMessage() { | |||
return findElement( | |||
By.xpath("//div[@class = 'v-grid-editor-message']/div")) | |||
.getText(); | |||
} | |||
} |