diff options
6 files changed, 117 insertions, 57 deletions
diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java index 86d2ed5f00..0555df3c1f 100644 --- a/client/src/com/vaadin/client/widgets/Grid.java +++ b/client/src/com/vaadin/client/widgets/Grid.java @@ -1159,8 +1159,17 @@ public class Grid<T> extends ResizeComposite implements 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; @@ -1223,9 +1232,7 @@ public class Grid<T> extends ResizeComposite implements 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(); } } @@ -1233,16 +1240,17 @@ public class Grid<T> extends ResizeComposite implements @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); } } }; @@ -1337,7 +1345,7 @@ public class Grid<T> extends ResizeComposite implements * * @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"); @@ -1349,14 +1357,23 @@ public class Grid<T> extends ResizeComposite implements } } - 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); } } @@ -1379,13 +1396,16 @@ public class Grid<T> extends ResizeComposite implements 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)); } @@ -1480,7 +1500,7 @@ public class Grid<T> extends ResizeComposite implements this.enabled = enabled; } - protected void show() { + protected void show(int rowIndex) { if (state == State.ACTIVATING) { state = State.BINDING; bindTimeout.schedule(BIND_TIMEOUT_MS); @@ -1498,15 +1518,6 @@ public class Grid<T> extends ResizeComposite implements 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() { @@ -6616,6 +6627,7 @@ public class Grid<T> extends ResizeComposite implements && key == Editor.KEYCODE_HIDE; if (!editorIsActive && editor.isEnabled() && openEvent) { + editor.editRow(eventCell.getRowIndex(), eventCell.getColumnIndexDOM()); fireEvent(new EditorOpenEvent(eventCell)); @@ -6624,16 +6636,16 @@ public class Grid<T> extends ResizeComposite implements 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); diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java index 251ec0f678..f025b6e1c0 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -4129,28 +4129,37 @@ public class Grid extends AbstractFocusable implements SelectionNotifier, @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 @@ -6004,7 +6013,7 @@ public class Grid extends AbstractFocusable implements SelectionNotifier, 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)) { diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java index 272ff1c9ae..3154fd2a85 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java @@ -298,6 +298,7 @@ public class GridBasicFeatures extends AbstractComponentTest<Grid> { 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( diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorBufferedTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorBufferedTest.java index 0f2fe54696..57f4b877df 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorBufferedTest.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorBufferedTest.java @@ -163,7 +163,7 @@ public class GridEditorBufferedTest extends GridEditorTest { } @Test - public void testNoScrollAfterEditByAPI() { + public void testScrollDisabledOnProgrammaticOpen() { int originalScrollPos = getGridVerticalScrollPos(); selectMenuPath(EDIT_ITEM_5); @@ -175,7 +175,7 @@ public class GridEditorBufferedTest extends GridEditorTest { } @Test - public void testNoScrollAfterEditByMouse() { + public void testScrollDisabledOnMouseOpen() { int originalScrollPos = getGridVerticalScrollPos(); GridCellElement cell_5_0 = getGridElement().getCell(5, 0); @@ -188,7 +188,7 @@ public class GridEditorBufferedTest extends GridEditorTest { } @Test - public void testNoScrollAfterEditByKeyboard() { + public void testScrollDisabledOnKeyboardOpen() { int originalScrollPos = getGridVerticalScrollPos(); GridCellElement cell_5_0 = getGridElement().getCell(5, 0); @@ -216,7 +216,28 @@ public class GridEditorBufferedTest extends GridEditorTest { } @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() @@ -232,7 +253,7 @@ public class GridEditorBufferedTest extends GridEditorTest { } @Test - public void testUserSortDisabled() { + public void testUserSortDisabledWhenOpen() { selectMenuPath(EDIT_ITEM_5); getGridElement().getHeaderCell(0, 0).click(); diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorTest.java index 3582038e61..e7eb78c35e 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorTest.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorTest.java @@ -123,7 +123,6 @@ public abstract class GridEditorTest extends GridBasicFeaturesTest { } protected void assertEditorOpen() { - assertNotNull("Editor is supposed to be open", getEditor()); assertEquals("Unexpected number of widgets", GridBasicFeatures.EDITABLE_COLUMNS, getEditorWidgets().size()); } @@ -133,7 +132,7 @@ public abstract class GridEditorTest extends GridBasicFeaturesTest { } protected List<WebElement> getEditorWidgets() { - assertNotNull(getEditor()); + assertNotNull("Editor is supposed to be open", getEditor()); return getEditor().findElements(By.className("v-textfield")); } diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorUnbufferedTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorUnbufferedTest.java index 1058fe2d74..08094b57e3 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorUnbufferedTest.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorUnbufferedTest.java @@ -72,14 +72,32 @@ public class GridEditorUnbufferedTest extends GridEditorTest { 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 @@ -96,7 +114,7 @@ public class GridEditorUnbufferedTest extends GridEditorTest { } @Test - public void testScrollAfterEditByAPI() { + public void testScrollEnabledOnProgrammaticOpen() { int originalScrollPos = getGridVerticalScrollPos(); selectMenuPath(EDIT_ITEM_5); @@ -108,7 +126,7 @@ public class GridEditorUnbufferedTest extends GridEditorTest { } @Test - public void testScrollAfterEditByMouse() { + public void testScrollEnabledOnMouseOpen() { int originalScrollPos = getGridVerticalScrollPos(); GridCellElement cell_5_0 = getGridElement().getCell(5, 0); @@ -121,7 +139,7 @@ public class GridEditorUnbufferedTest extends GridEditorTest { } @Test - public void testScrollAfterEditByKeyboard() { + public void testScrollEnabledOnKeyboardOpen() { int originalScrollPos = getGridVerticalScrollPos(); GridCellElement cell_5_0 = getGridElement().getCell(5, 0); |