Skip to content

Commit

Permalink
Prevent Grid editor move in unbuffered mode if validation errors in
Browse files Browse the repository at this point in the history 
fields

Change-Id: I37f3c21f4464c8f83308a741ed51485f7bd0375a
  • Loading branch information
@jdahlstrom
jdahlstrom authored and Teemu Suo-Anttila committed Jul 15, 2015
1 parent ae5793a commit e288b0d
Show file tree
Hide file tree
Showing 6 changed files with 117 additions and 57 deletions.
68 changes: 40 additions & 28 deletions client/src/com/vaadin/client/widgets/Grid.java
View file
Expand Up @@ -1159,8 +1159,17 @@ protected enum State {
private int columnIndex = -1;
private String styleName = null;

/*
* Used to track Grid horizontal scrolling
*/
private HandlerRegistration scrollHandler;

/*
* Used to open editor once Grid has vertically scrolled to the proper
* position and data is available
*/
private HandlerRegistration dataAvailableHandler;

private final Button saveButton;
private final Button cancelButton;

Expand Down Expand Up @@ -1223,26 +1232,25 @@ public void onSuccess(EditorRequest<T> request) {
state = State.ACTIVE;
bindTimeout.cancel();

assert rowIndex == request.getRowIndex() : "Request row index "
+ request.getRowIndex()
+ " did not match the saved row index " + rowIndex;
rowIndex = request.getRowIndex();
showOverlay();
}
}

@Override
public void onError(EditorRequest<T> request) {
if (state == State.BINDING) {
state = State.INACTIVE;
if (rowIndex == -1) {
doCancel();
} else {
state = State.ACTIVE;
}
bindTimeout.cancel();

// TODO show something in the DOM as well?
getLogger().warning(
"An error occurred while trying to show the "
+ "Grid editor");
grid.getEscalator().setScrollLocked(Direction.VERTICAL,
false);
updateSelectionCheckboxesAsNeeded(true);
}
}
};
Expand Down Expand Up @@ -1337,7 +1345,7 @@ public void editRow(int rowIndex) {
*
* @since 7.5
*/
public void editRow(int rowIndex, int columnIndex) {
public void editRow(final int rowIndex, int columnIndex) {
if (!enabled) {
throw new IllegalStateException(
"Cannot edit row: editor is not enabled");
Expand All @@ -1349,14 +1357,23 @@ public void editRow(int rowIndex, int columnIndex) {
}
}

this.rowIndex = rowIndex;
this.columnIndex = columnIndex;

state = State.ACTIVATING;

if (grid.getEscalator().getVisibleRowRange().contains(rowIndex)) {
show();
show(rowIndex);
} else {
hideOverlay();
dataAvailableHandler = grid
.addDataAvailableHandler(new DataAvailableHandler() {
@Override
public void onDataAvailable(DataAvailableEvent event) {
if (event.getAvailableRows().contains(rowIndex)) {
show(rowIndex);
dataAvailableHandler.removeHandler();
}
}
});
grid.scrollToRow(rowIndex, ScrollDestination.MIDDLE);
}
}
Expand All @@ -1379,13 +1396,16 @@ public void cancel() {
throw new IllegalStateException(
"Cannot cancel edit: editor is not in edit mode");
}
hideOverlay();
grid.getEscalator().setScrollLocked(Direction.VERTICAL, false);
handler.cancel(new EditorRequestImpl<T>(grid, rowIndex, null));
doCancel();
}

EditorRequest<T> request = new EditorRequestImpl<T>(grid, rowIndex,
null);
handler.cancel(request);
private void doCancel() {
hideOverlay();
state = State.INACTIVE;
rowIndex = -1;
columnIndex = -1;
grid.getEscalator().setScrollLocked(Direction.VERTICAL, false);
updateSelectionCheckboxesAsNeeded(true);
grid.fireEvent(new EditorCloseEvent(grid.eventCell));
}
Expand Down Expand Up @@ -1480,7 +1500,7 @@ public void setEnabled(boolean enabled) {
this.enabled = enabled;
}

protected void show() {
protected void show(int rowIndex) {
if (state == State.ACTIVATING) {
state = State.BINDING;
bindTimeout.schedule(BIND_TIMEOUT_MS);
Expand All @@ -1498,15 +1518,6 @@ protected void setGrid(final Grid<T> grid) {
assert this.grid == null : "Can only attach editor to Grid once";

this.grid = grid;

grid.addDataAvailableHandler(new DataAvailableHandler() {
@Override
public void onDataAvailable(DataAvailableEvent event) {
if (event.getAvailableRows().contains(rowIndex)) {
show();
}
}
});
}

protected State getState() {
Expand Down Expand Up @@ -6616,6 +6627,7 @@ private boolean handleEditorEvent(Event event, RowContainer container) {
&& key == Editor.KEYCODE_HIDE;

if (!editorIsActive && editor.isEnabled() && openEvent) {

editor.editRow(eventCell.getRowIndex(),
eventCell.getColumnIndexDOM());
fireEvent(new EditorOpenEvent(eventCell));
Expand All @@ -6624,16 +6636,16 @@ private boolean handleEditorEvent(Event event, RowContainer container) {
return true;

} else if (editorIsActive && !editor.isBuffered() && moveEvent) {
cellFocusHandler.setCellFocus(eventCell);

cellFocusHandler.setCellFocus(eventCell);
editor.editRow(eventCell.getRowIndex(),
eventCell.getColumnIndexDOM());

fireEvent(new EditorMoveEvent(eventCell));

return true;

} else if (editorIsActive && closeEvent) {

editor.cancel();
FocusUtil.setFocus(this, true);

Expand Down
39 changes: 24 additions & 15 deletions server/src/com/vaadin/ui/Grid.java
View file
Expand Up @@ -4129,28 +4129,37 @@ public void editorClose(String rowKey) {

@Override
public void bind(int rowIndex) {
Exception exception = null;
try {
Object id = getContainerDataSource().getIdByIndex(rowIndex);
if (!isEditorBuffered() || editedItemId == null) {
editedItemId = id;
}

if (editedItemId.equals(id)) {
doEditItem();
final boolean opening = editedItemId == null;

final boolean moving = !opening && !editedItemId.equals(id);

final boolean allowMove = !isEditorBuffered()
&& getEditorFieldGroup().isValid();

if (opening || !moving || allowMove) {
doBind(id);
} else {
failBind(null);
}
} catch (Exception e) {
exception = e;
failBind(e);
}
}

if (exception != null) {
handleError(exception);
doCancelEditor();
getEditorRpc().confirmBind(false);
} else {
doEditItem();
getEditorRpc().confirmBind(true);
private void doBind(Object id) {
editedItemId = id;
doEditItem();
getEditorRpc().confirmBind(true);
}

private void failBind(Exception e) {
if (e != null) {
handleError(e);
}
getEditorRpc().confirmBind(false);
}

@Override
Expand Down Expand Up @@ -6004,7 +6013,7 @@ public void editItem(Object itemId) throws IllegalStateException,
if (!isEditorEnabled()) {
throw new IllegalStateException("Item editor is not enabled");
} else if (isEditorBuffered() && editedItemId != null) {
throw new IllegalStateException("Editing item + " + itemId
throw new IllegalStateException("Editing item " + itemId
+ " failed. Item editor is already editing item "
+ editedItemId);
} else if (!getContainerDataSource().containsId(itemId)) {
Expand Down
1 change: 1 addition & 0 deletions uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java
View file
Expand Up @@ -298,6 +298,7 @@ public List<Object> getItemIds(int startIndex, int numberOfIds) {
new NumberRenderer(new DecimalFormat("0,000.00",
DecimalFormatSymbols.getInstance(new Locale("fi",
"FI")))));

grid.getColumn(getColumnProperty(col++)).setRenderer(
new DateRenderer(new SimpleDateFormat("dd.MM.yy HH:mm")));
grid.getColumn(getColumnProperty(col++)).setRenderer(
Expand Down
31 changes: 26 additions & 5 deletions uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorBufferedTest.java
View file
Expand Up @@ -163,7 +163,7 @@ public void testVerticalScrollLocking() {
}

@Test
public void testNoScrollAfterEditByAPI() {
public void testScrollDisabledOnProgrammaticOpen() {
int originalScrollPos = getGridVerticalScrollPos();

selectMenuPath(EDIT_ITEM_5);
Expand All @@ -175,7 +175,7 @@ public void testNoScrollAfterEditByAPI() {
}

@Test
public void testNoScrollAfterEditByMouse() {
public void testScrollDisabledOnMouseOpen() {
int originalScrollPos = getGridVerticalScrollPos();

GridCellElement cell_5_0 = getGridElement().getCell(5, 0);
Expand All @@ -188,7 +188,7 @@ public void testNoScrollAfterEditByMouse() {
}

@Test
public void testNoScrollAfterEditByKeyboard() {
public void testScrollDisabledOnKeyboardOpen() {
int originalScrollPos = getGridVerticalScrollPos();

GridCellElement cell_5_0 = getGridElement().getCell(5, 0);
Expand Down Expand Up @@ -216,7 +216,28 @@ public void testMouseOpeningClosing() {
}

@Test
public void testProgrammaticOpeningWhenOpen() {
public void testMouseOpeningDisabledWhenOpen() {
selectMenuPath(EDIT_ITEM_5);

getGridElement().getCell(4, 0).doubleClick();

assertEquals("Editor should still edit row 5", "(5, 0)",
getEditorWidgets().get(0).getAttribute("value"));
}

@Test
public void testKeyboardOpeningDisabledWhenOpen() {
selectMenuPath(EDIT_ITEM_5);

new Actions(getDriver()).click(getGridElement().getCell(4, 0))
.sendKeys(Keys.ENTER).perform();

assertEquals("Editor should still edit row 5", "(5, 0)",
getEditorWidgets().get(0).getAttribute("value"));
}

@Test
public void testProgrammaticOpeningDisabledWhenOpen() {
selectMenuPath(EDIT_ITEM_5);
assertEditorOpen();
assertEquals("Editor should edit row 5", "(5, 0)", getEditorWidgets()
Expand All @@ -232,7 +253,7 @@ public void testProgrammaticOpeningWhenOpen() {
}

@Test
public void testUserSortDisabled() {
public void testUserSortDisabledWhenOpen() {
selectMenuPath(EDIT_ITEM_5);

getGridElement().getHeaderCell(0, 0).click();
Expand Down
3 changes: 1 addition & 2 deletions uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorTest.java
View file
Expand Up @@ -123,7 +123,6 @@ public void testComponentBinding() {
}

protected void assertEditorOpen() {
assertNotNull("Editor is supposed to be open", getEditor());
assertEquals("Unexpected number of widgets",
GridBasicFeatures.EDITABLE_COLUMNS, getEditorWidgets().size());
}
Expand All @@ -133,7 +132,7 @@ protected void assertEditorClosed() {
}

protected List<WebElement> getEditorWidgets() {
assertNotNull(getEditor());
assertNotNull("Editor is supposed to be open", getEditor());
return getEditor().findElements(By.className("v-textfield"));

}
Expand Down
32 changes: 25 additions & 7 deletions ...t/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorUnbufferedTest.java
View file
Expand Up @@ -72,14 +72,32 @@ public void testEditorMove() {

String firstFieldValue = getEditorWidgets().get(0)
.getAttribute("value");
assertTrue("Editor is not at correct row index (5)",
"(5, 0)".equals(firstFieldValue));
assertEquals("Editor should be at row 5", "(5, 0)", firstFieldValue);

getGridElement().getCell(10, 0).click();
firstFieldValue = getEditorWidgets().get(0).getAttribute("value");

assertTrue("Editor is not at correct row index (10)",
"(10, 0)".equals(firstFieldValue));
assertEquals("Editor should be at row 10", "(10, 0)", firstFieldValue);
}

@Test
public void testValidationErrorPreventsMove() {
// Because of "out of view" issues, we need to move this for easy access
selectMenuPath("Component", "Columns", "Column 7", "Column 7 Width",
"50px");
for (int i = 0; i < 6; ++i) {
selectMenuPath("Component", "Columns", "Column 7", "Move left");
}

selectMenuPath(EDIT_ITEM_5);

getEditorWidgets().get(1).click();
getEditorWidgets().get(1).sendKeys("not a number");

getGridElement().getCell(10, 0).click();

assertEquals("Editor should not move from row 5", "(5, 0)",
getEditorWidgets().get(0).getAttribute("value"));
}

@Test
Expand All @@ -96,7 +114,7 @@ public void testErrorMessageWrapperHidden() {
}

@Test
public void testScrollAfterEditByAPI() {
public void testScrollEnabledOnProgrammaticOpen() {
int originalScrollPos = getGridVerticalScrollPos();

selectMenuPath(EDIT_ITEM_5);
Expand All @@ -108,7 +126,7 @@ public void testScrollAfterEditByAPI() {
}

@Test
public void testScrollAfterEditByMouse() {
public void testScrollEnabledOnMouseOpen() {
int originalScrollPos = getGridVerticalScrollPos();

GridCellElement cell_5_0 = getGridElement().getCell(5, 0);
Expand All @@ -121,7 +139,7 @@ public void testScrollAfterEditByMouse() {
}

@Test
public void testScrollAfterEditByKeyboard() {
public void testScrollEnabledOnKeyboardOpen() {
int originalScrollPos = getGridVerticalScrollPos();

GridCellElement cell_5_0 = getGridElement().getCell(5, 0);
Expand Down

0 comments on commit e288b0d

Please sign in to comment.