diff options
7 files changed, 286 insertions, 122 deletions
diff --git a/client/src/com/vaadin/client/connectors/GridConnector.java b/client/src/com/vaadin/client/connectors/GridConnector.java index 5a68bbf4b8..0414e82680 100644 --- a/client/src/com/vaadin/client/connectors/GridConnector.java +++ b/client/src/com/vaadin/client/connectors/GridConnector.java @@ -221,14 +221,14 @@ public class GridConnector extends AbstractHasComponentsConnector implements } @Override - public void confirmBind() { - endRequest(); + public void confirmBind(final boolean bindSucceeded) { + endRequest(bindSucceeded); } @Override - public void confirmSave() { - endRequest(); + public void confirmSave(boolean saveSucceeded) { + endRequest(saveSucceeded); } }); } @@ -288,6 +288,7 @@ public class GridConnector extends AbstractHasComponentsConnector implements if (serverInitiated) { serverInitiated = false; + request.success(); return true; } else { return false; @@ -296,10 +297,9 @@ public class GridConnector extends AbstractHasComponentsConnector implements private void startRequest(EditorRequest<?> request) { currentRequest = request; - request.startAsync(); } - private void endRequest() { + private void endRequest(boolean succeeded) { assert currentRequest != null; /* * Clear current request first to ensure the state is valid if @@ -307,7 +307,11 @@ public class GridConnector extends AbstractHasComponentsConnector implements */ EditorRequest<?> request = currentRequest; currentRequest = null; - request.complete(); + if (succeeded) { + request.success(); + } else { + request.fail(); + } } } diff --git a/client/src/com/vaadin/client/widget/grid/EditorHandler.java b/client/src/com/vaadin/client/widget/grid/EditorHandler.java index f834143a45..07ec1b231c 100644 --- a/client/src/com/vaadin/client/widget/grid/EditorHandler.java +++ b/client/src/com/vaadin/client/widget/grid/EditorHandler.java @@ -36,12 +36,9 @@ public interface EditorHandler<T> { * request is callback-based to facilitate usage with remote or otherwise * asynchronous data sources. * <p> - * In any of the EditorHandler methods, an implementation may call - * {@link EditorRequest#startAsync()} to signal the caller that the request - * is handled asynchronously. In that case, {@link EditorRequest#complete()} - * must be called when the request is complete. - * <p> - * TODO Should have a mechanism for signaling a failed request to the caller + * An implementation must call either {@link #success()} or {@link #fail()}, + * according to whether the operation was a success or failed during + * execution, respectively. * * @param <T> * the row data type @@ -56,13 +53,28 @@ public interface EditorHandler<T> { * the row data type */ public interface RequestCallback<T> { - public void onResponse(EditorRequest<T> request); + /** + * The method that must be called when the request has been + * processed correctly. + * + * @param request + * the original request object + */ + public void onSuccess(EditorRequest<T> request); + + /** + * The method that must be called when processing the request has + * produced an aborting error. + * + * @param request + * the original request object + */ + public void onError(EditorRequest<T> request); } private Grid<T> grid; private int rowIndex; private RequestCallback<T> callback; - private boolean async = false; private boolean completed = false; /** @@ -122,19 +134,6 @@ public interface EditorHandler<T> { return w; } - public boolean isAsync() { - return async; - } - - /** - * Marks this request as asynchronous. If this method is invoked, the - * caller must also ensure that {@link #complete()} is invoked once the - * request is finished. - */ - public void startAsync() { - async = true; - } - /** * Completes this request. The request can only be completed once. This * method should only be called by an EditorHandler implementer if the @@ -145,26 +144,52 @@ public interface EditorHandler<T> { * @throws IllegalStateException * if the request is already completed */ - public void complete() { + private void complete() { if (completed) { throw new IllegalStateException( "An EditorRequest must be completed exactly once"); } completed = true; + } + + /** + * Informs Grid that the editor request was a success. + */ + public void success() { + complete(); if (callback != null) { - callback.onResponse(this); + callback.onSuccess(this); } } + + /** + * Informs Grid that an error occurred while trying to process the + * request. + */ + public void fail() { + complete(); + if (callback != null) { + callback.onError(this); + } + } + + /** + * Checks whether the request is completed or not. + * + * @return <code>true</code> iff the request is completed + */ + public boolean isCompleted() { + return completed; + } } /** * Binds row data to the editor widgets. Called by the editor when it is * opened for editing. * <p> - * An implementation may call {@link EditorRequest#startAsync() - * request.startAsync()} to signal the caller that the request is handled - * asynchronously. In that case, {@link EditorRequest#complete()} must be - * called once the binding is complete. + * The implementation <em>must</em> call either + * {@link EditorRequest#success()} or {@link EditorRequest#fail()} to signal + * a successful or a failed (respectively) bind action. * * @param request * the data binding request @@ -177,10 +202,11 @@ public interface EditorHandler<T> { * Called by the editor when editing is cancelled. This method may have an * empty implementation in case no special processing is required. * <p> - * An implementation may call {@link EditorRequest#startAsync() - * request.startAsync()} to signal the caller that the request is handled - * asynchronously. In that case, {@link EditorRequest#complete()} must be - * called once the cancel operation is complete. + * In contrast to {@link #bind(EditorRequest)} and + * {@link #save(EditorRequest)}, any calls to + * {@link EditorRequest#success()} or {@link EditorRequest#fail()} have no + * effect on the outcome of the cancel action. The editor is already closed + * when this method is called. * * @param request * the cancel request @@ -193,10 +219,9 @@ public interface EditorHandler<T> { * Commits changes in the currently active edit to the data source. Called * by the editor when changes are saved. * <p> - * An implementation may call {@link EditorRequest#startAsync() - * request.startAsync()} to signal the caller that the request is handled - * asynchronously. In that case, {@link EditorRequest#complete()} must be - * called once the commit operation is complete. + * The implementation <em>must</em> call either + * {@link EditorRequest#success()} or {@link EditorRequest#fail()} to signal + * a successful or a failed (respectively) save action. * * @param request * the save request diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java index 18115b2a3b..a215b9df6d 100644 --- a/client/src/com/vaadin/client/widgets/Grid.java +++ b/client/src/com/vaadin/client/widgets/Grid.java @@ -961,6 +961,88 @@ public class Grid<T> extends ResizeComposite implements private HandlerRegistration scrollHandler; + private Button saveButton; + private Button cancelButton; + + private static final int SAVE_TIMEOUT_MS = 5000; + private final Timer saveTimeout = new Timer() { + @Override + public void run() { + getLogger().warning( + "Editor save action is taking longer than expected (" + + SAVE_TIMEOUT_MS + "ms). Does your " + + EditorHandler.class.getSimpleName() + + " remember to call success() or fail()?"); + } + }; + + private final RequestCallback<T> saveRequestCallback = new RequestCallback<T>() { + @Override + public void onSuccess(EditorRequest<T> request) { + if (state == State.SAVING) { + cleanup(); + cancel(); + } + } + + @Override + public void onError(EditorRequest<T> request) { + if (state == State.SAVING) { + cleanup(); + + // TODO probably not the most correct thing to do... + getLogger().warning( + "An error occurred when trying to save the " + + "modified row"); + } + } + + private void cleanup() { + state = State.ACTIVE; + enableButtons(true); + saveTimeout.cancel(); + } + }; + + private static final int BIND_TIMEOUT_MS = 5000; + private final Timer bindTimeout = new Timer() { + @Override + public void run() { + getLogger().warning( + "Editor bind action is taking longer than expected (" + + BIND_TIMEOUT_MS + "ms). Does your " + + EditorHandler.class.getSimpleName() + + " remember to call success() or fail()?"); + } + }; + private final RequestCallback<T> bindRequestCallback = new RequestCallback<T>() { + @Override + public void onSuccess(EditorRequest<T> request) { + if (state == State.ACTIVATING) { + state = State.ACTIVE; + bindTimeout.cancel(); + + showOverlay(grid.getEscalator().getBody() + .getRowElement(request.getRowIndex())); + } + } + + @Override + public void onError(EditorRequest<T> request) { + if (state == State.ACTIVATING) { + state = State.INACTIVE; + 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); + } + } + }; + public int getRow() { return rowIndex; } @@ -1021,8 +1103,6 @@ public class Grid<T> extends ResizeComposite implements EditorRequest<T> request = new EditorRequest<T>(grid, rowIndex, null); handler.cancel(request); - completeIfSync(request); - state = State.INACTIVE; } @@ -1045,18 +1125,11 @@ public class Grid<T> extends ResizeComposite implements } state = State.SAVING; + enableButtons(false); + saveTimeout.schedule(SAVE_TIMEOUT_MS); EditorRequest<T> request = new EditorRequest<T>(grid, rowIndex, - new RequestCallback<T>() { - @Override - public void onResponse(EditorRequest<T> request) { - if (state == State.SAVING) { - state = State.ACTIVE; - cancel(); - } - } - }); + saveRequestCallback); handler.save(request); - completeIfSync(request); } /** @@ -1115,23 +1188,10 @@ public class Grid<T> extends ResizeComposite implements protected void show() { if (state == State.ACTIVATING) { + bindTimeout.schedule(BIND_TIMEOUT_MS); EditorRequest<T> request = new EditorRequest<T>(grid, rowIndex, - new RequestCallback<T>() { - @Override - public void onResponse(EditorRequest<T> request) { - if (state == State.ACTIVATING) { - state = State.ACTIVE; - showOverlay(grid - .getEscalator() - .getBody() - .getRowElement( - request.getRowIndex())); - } - } - }); + bindRequestCallback); handler.bind(request); - completeIfSync(request); - grid.getEscalator().setScrollLocked(Direction.VERTICAL, true); } } @@ -1225,30 +1285,31 @@ public class Grid<T> extends ResizeComposite implements } } - Button save = new Button(); - save.setText("Save"); - save.setStyleName(styleName + "-save"); - save.addClickHandler(new ClickHandler() { + saveButton = new Button(); + saveButton.setText("Save"); + saveButton.setStyleName(styleName + "-save"); + saveButton.addClickHandler(new ClickHandler() { @Override public void onClick(ClickEvent event) { - // TODO should have a mechanism for handling failed save save(); } }); - setBounds(save.getElement(), 0, tr.getOffsetHeight() + 5, 50, 25); - attachWidget(save, editorOverlay); - - Button cancel = new Button(); - cancel.setText("Cancel"); - cancel.setStyleName(styleName + "-cancel"); - cancel.addClickHandler(new ClickHandler() { + setBounds(saveButton.getElement(), 0, tr.getOffsetHeight() + 5, 50, + 25); + attachWidget(saveButton, editorOverlay); + + cancelButton = new Button(); + cancelButton.setText("Cancel"); + cancelButton.setStyleName(styleName + "-cancel"); + cancelButton.addClickHandler(new ClickHandler() { @Override public void onClick(ClickEvent event) { cancel(); } }); - setBounds(cancel.getElement(), 55, tr.getOffsetHeight() + 5, 50, 25); - attachWidget(cancel, editorOverlay); + setBounds(cancelButton.getElement(), 55, tr.getOffsetHeight() + 5, + 50, 25); + attachWidget(cancelButton, editorOverlay); } protected void hideOverlay() { @@ -1309,10 +1370,9 @@ public class Grid<T> extends ResizeComposite implements editorOverlay.getStyle().setLeft(-grid.getScrollLeft(), Unit.PX); } - private void completeIfSync(EditorRequest<T> request) { - if (!request.isAsync()) { - request.complete(); - } + private void enableButtons(boolean enabled) { + saveButton.setEnabled(enabled); + cancelButton.setEnabled(enabled); } } @@ -3475,6 +3535,10 @@ public class Grid<T> extends ResizeComposite implements @Override public void setEnabled(boolean enabled) { + if (enabled == this.enabled) { + return; + } + this.enabled = enabled; getElement().setTabIndex(enabled ? 0 : -1); getEscalator().setScrollLocked(Direction.VERTICAL, !enabled); diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java index 18d45b3b9f..bddbd7c731 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -2732,13 +2732,16 @@ public class Grid extends AbstractComponent implements SelectionNotifier, @Override public void bind(int rowIndex) { + boolean success; try { Object id = getContainerDataSource().getIdByIndex(rowIndex); doEditItem(id); + success = true; } catch (Exception e) { handleError(e); + success = false; } - getEditorRpc().confirmBind(); + getEditorRpc().confirmBind(success); } @Override @@ -2753,12 +2756,15 @@ public class Grid extends AbstractComponent implements SelectionNotifier, @Override public void save(int rowIndex) { + boolean success; try { saveEditor(); + success = true; } catch (Exception e) { handleError(e); + success = false; } - getEditorRpc().confirmSave(); + getEditorRpc().confirmSave(success); } private void handleError(Exception e) { diff --git a/shared/src/com/vaadin/shared/ui/grid/EditorClientRpc.java b/shared/src/com/vaadin/shared/ui/grid/EditorClientRpc.java index 7466df9c99..82e08999b4 100644 --- a/shared/src/com/vaadin/shared/ui/grid/EditorClientRpc.java +++ b/shared/src/com/vaadin/shared/ui/grid/EditorClientRpc.java @@ -44,12 +44,18 @@ public interface EditorClientRpc extends ClientRpc { /** * Confirms a pending {@link EditorServerRpc#bind(int) bind request} sent by * the client. + * + * @param bindSucceeded + * <code>true</code> iff the bind action was successful */ - void confirmBind(); + void confirmBind(boolean bindSucceeded); /** * Confirms a pending {@link EditorServerRpc#save(int) save request} sent by * the client. + * + * @param saveSucceeded + * <code>true</code> iff the save action was successful */ - void confirmSave(); + void confirmSave(boolean saveSucceeded); } 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 74c3c0db35..dc87ec2ebe 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 @@ -31,6 +31,7 @@ import org.openqa.selenium.NoSuchElementException; import org.openqa.selenium.WebElement; import org.openqa.selenium.interactions.Actions; +import com.vaadin.testbench.elements.GridElement.GridCellElement; import com.vaadin.testbench.elements.GridElement.GridEditorElement; import com.vaadin.testbench.elements.NotificationElement; import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeatures; @@ -38,16 +39,23 @@ import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeaturesTest; public class GridEditorTest extends GridBasicFeaturesTest { + private static final String[] EDIT_ITEM_5 = new String[] { "Component", + "Editor", "Edit item 5" }; + private static final String[] EDIT_ITEM_100 = new String[] { "Component", + "Editor", "Edit item 100" }; + private static final String[] TOGGLE_EDIT_ENABLED = new String[] { + "Component", "Editor", "Enabled" }; + @Before public void setUp() { setDebug(true); openTestURL(); - selectMenuPath("Component", "Editor", "Enabled"); + selectMenuPath(TOGGLE_EDIT_ENABLED); } @Test public void testProgrammaticOpeningClosing() { - selectMenuPath("Component", "Editor", "Edit item 5"); + selectMenuPath(EDIT_ITEM_5); assertEditorOpen(); selectMenuPath("Component", "Editor", "Cancel edit"); @@ -56,8 +64,8 @@ public class GridEditorTest extends GridBasicFeaturesTest { @Test public void testProgrammaticOpeningWhenDisabled() { - selectMenuPath("Component", "Editor", "Enabled"); - selectMenuPath("Component", "Editor", "Edit item 5"); + selectMenuPath(TOGGLE_EDIT_ENABLED); + selectMenuPath(EDIT_ITEM_5); assertEditorClosed(); boolean thrown = logContainsText("Exception occured, java.lang.IllegalStateException"); assertTrue("IllegalStateException thrown", thrown); @@ -65,8 +73,8 @@ public class GridEditorTest extends GridBasicFeaturesTest { @Test public void testDisablingWhileOpen() { - selectMenuPath("Component", "Editor", "Edit item 5"); - selectMenuPath("Component", "Editor", "Enabled"); + selectMenuPath(EDIT_ITEM_5); + selectMenuPath(TOGGLE_EDIT_ENABLED); assertEditorOpen(); boolean thrown = logContainsText("Exception occured, java.lang.IllegalStateException"); assertTrue("IllegalStateException thrown", thrown); @@ -74,13 +82,13 @@ public class GridEditorTest extends GridBasicFeaturesTest { @Test public void testProgrammaticOpeningWithScroll() { - selectMenuPath("Component", "Editor", "Edit item 100"); + selectMenuPath(EDIT_ITEM_100); assertEditorOpen(); } @Test(expected = NoSuchElementException.class) public void testVerticalScrollLocking() { - selectMenuPath("Component", "Editor", "Edit item 5"); + selectMenuPath(EDIT_ITEM_5); getGridElement().getCell(200, 0); } @@ -97,7 +105,7 @@ public class GridEditorTest extends GridBasicFeaturesTest { assertEditorClosed(); // Disable Editor - selectMenuPath("Component", "Editor", "Enabled"); + selectMenuPath(TOGGLE_EDIT_ENABLED); getGridElement().getCell(5, 0).click(); new Actions(getDriver()).sendKeys(Keys.ENTER).perform(); assertEditorClosed(); @@ -105,7 +113,7 @@ public class GridEditorTest extends GridBasicFeaturesTest { @Test public void testComponentBinding() { - selectMenuPath("Component", "State", "Editor", "Edit item 100"); + selectMenuPath(EDIT_ITEM_100); List<WebElement> widgets = getEditorWidgets(); assertEquals("Number of widgets", GridBasicFeatures.COLUMNS, @@ -119,7 +127,7 @@ public class GridEditorTest extends GridBasicFeaturesTest { @Test public void testSave() { - selectMenuPath("Component", "Editor", "Edit item 100"); + selectMenuPath(EDIT_ITEM_100); WebElement textField = getEditorWidgets().get(0); @@ -138,7 +146,7 @@ public class GridEditorTest extends GridBasicFeaturesTest { @Test public void testProgrammaticSave() { - selectMenuPath("Component", "Editor", "Edit item 100"); + selectMenuPath(EDIT_ITEM_100); WebElement textField = getEditorWidgets().get(0); @@ -153,13 +161,13 @@ public class GridEditorTest extends GridBasicFeaturesTest { } private void assertEditorOpen() { - assertNotNull("Editor open", getEditor()); - assertEquals("Number of widgets", GridBasicFeatures.COLUMNS, + assertNotNull("Editor is supposed to be open", getEditor()); + assertEquals("Unexpected number of widgets", GridBasicFeatures.COLUMNS, getEditorWidgets().size()); } private void assertEditorClosed() { - assertNull("Editor closed", getEditor()); + assertNull("Editor is supposed to be closed", getEditor()); } private List<WebElement> getEditorWidgets() { @@ -170,9 +178,11 @@ public class GridEditorTest extends GridBasicFeaturesTest { @Test public void testInvalidEdition() { - selectMenuPath("Component", "Editor", "Edit item 5"); + selectMenuPath(EDIT_ITEM_5); assertFalse(logContainsText("Exception occured, java.lang.IllegalStateException")); + GridEditorElement editor = getGridElement().getEditor(); + WebElement intField = editor.getField(7); intField.clear(); intField.sendKeys("banana phone"); @@ -180,8 +190,46 @@ public class GridEditorTest extends GridBasicFeaturesTest { assertTrue( "No exception on invalid value.", logContainsText("Exception occured, com.vaadin.data.fieldgroup.FieldGroup$CommitExceptionCommit failed")); - selectMenuPath("Component", "Editor", "Edit item 100"); + editor.cancel(); + + selectMenuPath(EDIT_ITEM_100); assertFalse("Exception should not exist", isElementPresent(NotificationElement.class)); } + + @Test + public void testNoScrollAfterEditByAPI() { + int originalScrollPos = getGridVerticalScrollPos(); + + selectMenuPath(EDIT_ITEM_5); + + scrollGridVerticallyTo(100); + assertEquals("Grid shouldn't scroll vertically while editing", + originalScrollPos, getGridVerticalScrollPos()); + } + + @Test + public void testNoScrollAfterEditByMouse() { + int originalScrollPos = getGridVerticalScrollPos(); + + GridCellElement cell_5_0 = getGridElement().getCell(5, 0); + new Actions(getDriver()).doubleClick(cell_5_0).perform(); + + scrollGridVerticallyTo(100); + assertEquals("Grid shouldn't scroll vertically while editing", + originalScrollPos, getGridVerticalScrollPos()); + } + + @Test + public void testNoScrollAfterEditByKeyboard() { + int originalScrollPos = getGridVerticalScrollPos(); + + GridCellElement cell_5_0 = getGridElement().getCell(5, 0); + cell_5_0.click(); + new Actions(getDriver()).sendKeys(Keys.ENTER).perform(); + + scrollGridVerticallyTo(100); + assertEquals("Grid shouldn't scroll vertically while editing", + originalScrollPos, getGridVerticalScrollPos()); + } } diff --git a/uitest/src/com/vaadin/tests/widgetset/client/grid/GridBasicClientFeaturesWidget.java b/uitest/src/com/vaadin/tests/widgetset/client/grid/GridBasicClientFeaturesWidget.java index 64fc60e488..25a04b88f9 100644 --- a/uitest/src/com/vaadin/tests/widgetset/client/grid/GridBasicClientFeaturesWidget.java +++ b/uitest/src/com/vaadin/tests/widgetset/client/grid/GridBasicClientFeaturesWidget.java @@ -21,6 +21,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Random; +import java.util.logging.Logger; import com.google.gwt.core.client.Scheduler.ScheduledCommand; import com.google.gwt.dom.client.Style.Unit; @@ -111,6 +112,7 @@ public class GridBasicClientFeaturesWidget extends int columnIndex = hasSelectionColumn ? i + 1 : i; getWidget(columnIndex).setText(rowData.get(i).value.toString()); } + request.success(); } @Override @@ -120,22 +122,31 @@ public class GridBasicClientFeaturesWidget extends @Override public void save(EditorRequest<List<Data>> request) { - log.setText("Row " + request.getRowIndex() + " edit committed"); - List<Data> rowData = ds.getRow(request.getRowIndex()); - - int i = 0; - for (; i < COLUMNS - MANUALLY_FORMATTED_COLUMNS; i++) { - rowData.get(i).value = getWidget(i).getText(); - } + try { + log.setText("Row " + request.getRowIndex() + " edit committed"); + List<Data> rowData = ds.getRow(request.getRowIndex()); - rowData.get(i).value = Integer.valueOf(getWidget(i++).getText()); - rowData.get(i).value = new Date(getWidget(i++).getText()); - rowData.get(i).value = getWidget(i++).getText(); - rowData.get(i).value = Integer.valueOf(getWidget(i++).getText()); - rowData.get(i).value = Integer.valueOf(getWidget(i++).getText()); + int i = 0; + for (; i < COLUMNS - MANUALLY_FORMATTED_COLUMNS; i++) { + rowData.get(i).value = getWidget(i).getText(); + } - // notify data source of changes - ds.asList().set(request.getRowIndex(), rowData); + rowData.get(i).value = Integer + .valueOf(getWidget(i++).getText()); + rowData.get(i).value = new Date(getWidget(i++).getText()); + rowData.get(i).value = getWidget(i++).getText(); + rowData.get(i).value = Integer + .valueOf(getWidget(i++).getText()); + rowData.get(i).value = Integer + .valueOf(getWidget(i++).getText()); + + // notify data source of changes + ds.asList().set(request.getRowIndex(), rowData); + request.success(); + } catch (Exception e) { + Logger.getLogger(getClass().getName()).warning(e.toString()); + request.fail(); + } } @Override |