Highlights erroneous cells in Grid editor (#16575)

Change-Id: Ie1f9d738db7a03ddb01b968782ad5e4877af1d7e

WebContent/VAADIN/themes/base/grid/grid.scss

         max-width: 100%;
       }
     }
+
+    .error::before {
+      position: absolute;
+      display: block;
+      height: 0;
+      width: 0;
+      content: "";
+      border-top: 5px solid red;
+      border-right: 5px solid transparent;
+    }
+
+    .error,
+    .error > input {
+      background-color: #fee;
+    }
   }
 
   .#{$primaryStyleName}-editor-footer {

WebContent/VAADIN/themes/valo/components/_grid.scss

       height: 100%;
       vertical-align: middle;
     }
+    
+    .error::before {
+      border-top: round($v-unit-size / 4) solid $v-error-indicator-color;
+      border-right: round($v-unit-size / 4) solid transparent;
+    }
+    
+    .error,
+    .error > input {
+      // taken from @mixin valo-textfield-error-style()
+      background-color: scale-color($v-error-indicator-color, $lightness: 98%);
+    }
 
     .v-textfield,
     .v-textfield-focus,

client/src/com/vaadin/client/connectors/GridConnector.java

 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 
         private EditorServerRpc rpc = getRpcProxy(EditorServerRpc.class);
 
-        private EditorRequest<?> currentRequest = null;
+        private EditorRequest<JsonObject> currentRequest = null;
         private boolean serverInitiated = false;
 
         public CustomEditorHandler() {
 
                 @Override
                 public void confirmBind(final boolean bindSucceeded) {
-                    endRequest(bindSucceeded);
+                    endRequest(bindSucceeded, null);
                 }
 
                 @Override
-                public void confirmSave(boolean saveSucceeded) {
-                    endRequest(saveSucceeded);
+                public void confirmSave(boolean saveSucceeded,
+                        List<String> errorColumnsIds) {
+                    endRequest(saveSucceeded, errorColumnsIds);
                 }
             });
         }
             }
         }
 
-        private void startRequest(EditorRequest<?> request) {
+        private void startRequest(EditorRequest<JsonObject> request) {
             assert currentRequest == null : "Earlier request not yet finished";
 
             currentRequest = request;
         }
 
-        private void endRequest(boolean succeeded) {
+        private void endRequest(boolean succeeded, List<String> errorColumnsIds) {
             assert currentRequest != null : "Current request was null";
             /*
              * Clear current request first to ensure the state is valid if
              * another request is made in the callback.
              */
-            EditorRequest<?> request = currentRequest;
+            EditorRequest<JsonObject> request = currentRequest;
             currentRequest = null;
             if (succeeded) {
                 request.success();
             } else {
-                request.fail();
+                Collection<Column<?, JsonObject>> errorColumns;
+                if (errorColumnsIds != null) {
+                    errorColumns = new ArrayList<Grid.Column<?, JsonObject>>();
+                    for (String colId : errorColumnsIds) {
+                        errorColumns.add(columnIdToColumn.get(colId));
+                    }
+                } else {
+                    errorColumns = null;
+                }
+
+                request.failure(errorColumns);
             }
         }
     }

client/src/com/vaadin/client/widget/grid/EditorHandler.java

  */
 package com.vaadin.client.widget.grid;
 
+import java.util.Collection;
+
 import com.google.gwt.user.client.ui.Widget;
 import com.vaadin.client.widgets.Grid;
 
      * @param <T>
      *            the row data type
      */
-    public static class EditorRequest<T> {
-
-        /**
-         * A callback interface used to notify the invoker of the editor handler
-         * of completed editor requests.
-         * 
-         * @param <T>
-         *            the row data type
-         */
-        public interface RequestCallback<T> {
-            /**
-             * 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 completed = false;
-
-        /**
-         * Creates a new editor request.
-         * 
-         * @param rowIndex
-         *            the index of the edited row
-         * @param callback
-         *            the callback invoked when the request is ready, or null if
-         *            no need to call back
-         */
-        public EditorRequest(Grid<T> grid, int rowIndex,
-                RequestCallback<T> callback) {
-            this.grid = grid;
-            this.rowIndex = rowIndex;
-            this.callback = callback;
-        }
-
+    public interface EditorRequest<T> {
         /**
          * Returns the index of the row being requested.
          * 
          * @return the row index
          */
-        public int getRowIndex() {
-            return rowIndex;
-        }
+        public int getRowIndex();
 
         /**
          * Returns the row data related to the row being requested.
          * 
          * @return the row data
          */
-        public T getRow() {
-            return grid.getDataSource().getRow(rowIndex);
-        }
+        public T getRow();
 
         /**
          * Returns the grid instance related to this editor request.
          * 
          * @return the grid instance
          */
-        public Grid<T> getGrid() {
-            return grid;
-        }
+        public Grid<T> getGrid();
 
         /**
          * Returns the editor widget used to edit the values of the given
          *            the column whose widget to get
          * @return the widget related to the column
          */
-        public Widget getWidget(Grid.Column<?, T> column) {
-            Widget w = grid.getEditorWidget(column);
-            assert w != null;
-            return w;
-        }
-
-        /**
-         * Completes this request. The request can only be completed once. This
-         * method should only be called by an EditorHandler implementer if the
-         * request handling is asynchronous in nature and {@link #startAsync()}
-         * is previously invoked for this request. Synchronous requests are
-         * completed automatically by the editor.
-         * 
-         * @throws IllegalStateException
-         *             if the request is already completed
-         */
-        private void complete() {
-            if (completed) {
-                throw new IllegalStateException(
-                        "An EditorRequest must be completed exactly once");
-            }
-            completed = true;
-        }
+        public Widget getWidget(Grid.Column<?, T> column);
 
         /**
          * Informs Grid that the editor request was a success.
          */
-        public void success() {
-            complete();
-            if (callback != null) {
-                callback.onSuccess(this);
-            }
-        }
+        public void success();
 
         /**
          * Informs Grid that an error occurred while trying to process the
          * request.
+         * 
+         * @param errorColumns
+         *            a collection of columns for which an error indicator
+         *            should be shown, or <code>null</code> if no columns should
+         *            be marked as erroneous.
          */
-        public void fail() {
-            complete();
-            if (callback != null) {
-                callback.onError(this);
-            }
-        }
+        public void failure(Collection<Grid.Column<?, T>> errorColumns);
 
         /**
          * Checks whether the request is completed or not.
          * 
          * @return <code>true</code> iff the request is completed
          */
-        public boolean isCompleted() {
-            return completed;
-        }
+        public boolean isCompleted();
     }
 
     /**
      * opened for editing.
      * <p>
      * The implementation <em>must</em> call either
-     * {@link EditorRequest#success()} or {@link EditorRequest#fail()} to signal
-     * a successful or a failed (respectively) bind action.
+     * {@link EditorRequest#success()} or
+     * {@link EditorRequest#failure(Collection)} to signal a successful or a
+     * failed (respectively) bind action.
      * 
      * @param request
      *            the data binding request

client/src/com/vaadin/client/widgets/Grid.java

 import com.vaadin.client.widget.grid.DataAvailableHandler;
 import com.vaadin.client.widget.grid.EditorHandler;
 import com.vaadin.client.widget.grid.EditorHandler.EditorRequest;
-import com.vaadin.client.widget.grid.EditorHandler.EditorRequest.RequestCallback;
 import com.vaadin.client.widget.grid.EventCellReference;
 import com.vaadin.client.widget.grid.RendererCellReference;
 import com.vaadin.client.widget.grid.RowReference;
         }
     }
 
+    private static class EditorRequestImpl<T> implements EditorRequest<T> {
+
+        /**
+         * A callback interface used to notify the invoker of the editor handler
+         * of completed editor requests.
+         * 
+         * @param <T>
+         *            the row data type
+         */
+        public static interface RequestCallback<T> {
+            /**
+             * 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 completed = false;
+
+        public EditorRequestImpl(Grid<T> grid, int rowIndex,
+                RequestCallback<T> callback) {
+            this.grid = grid;
+            this.rowIndex = rowIndex;
+            this.callback = callback;
+        }
+
+        @Override
+        public int getRowIndex() {
+            return rowIndex;
+        }
+
+        @Override
+        public T getRow() {
+            return grid.getDataSource().getRow(rowIndex);
+        }
+
+        @Override
+        public Grid<T> getGrid() {
+            return grid;
+        }
+
+        @Override
+        public Widget getWidget(Grid.Column<?, T> column) {
+            Widget w = grid.getEditorWidget(column);
+            assert w != null;
+            return w;
+        }
+
+        private void complete(Collection<Column<?, T>> errorColumns) {
+            if (completed) {
+                throw new IllegalStateException(
+                        "An EditorRequest must be completed exactly once");
+            }
+            completed = true;
+
+            grid.getEditor().clearEditorColumnErrors();
+            if (errorColumns != null) {
+                for (Column<?, T> column : errorColumns) {
+                    grid.getEditor().setEditorColumnError(column, true);
+                }
+            }
+        }
+
+        @Override
+        public void success() {
+            complete(null);
+            if (callback != null) {
+                callback.onSuccess(this);
+            }
+        }
+
+        @Override
+        public void failure(Collection<Grid.Column<?, T>> errorColumns) {
+            complete(errorColumns);
+            if (callback != null) {
+                callback.onError(this);
+            }
+        }
+
+        @Override
+        public boolean isCompleted() {
+            return completed;
+        }
+    }
+
     /**
      * An editor UI for Grid rows. A single Grid row at a time can be opened for
      * editing.
         public static final int KEYCODE_SHOW = KeyCodes.KEY_ENTER;
         public static final int KEYCODE_HIDE = KeyCodes.KEY_ESCAPE;
 
+        private static final String ERROR_CLASS_NAME = "error";
+
         protected enum State {
             INACTIVE, ACTIVATING, BINDING, ACTIVE, SAVING
         }
             }
         };
 
-        private final RequestCallback<T> saveRequestCallback = new RequestCallback<T>() {
+        private final EditorRequestImpl.RequestCallback<T> saveRequestCallback = new EditorRequestImpl.RequestCallback<T>() {
             @Override
             public void onSuccess(EditorRequest<T> request) {
                 if (state == State.SAVING) {
                                 + " remember to call success() or fail()?");
             }
         };
-        private final RequestCallback<T> bindRequestCallback = new RequestCallback<T>() {
+        private final EditorRequestImpl.RequestCallback<T> bindRequestCallback = new EditorRequestImpl.RequestCallback<T>() {
             @Override
             public void onSuccess(EditorRequest<T> request) {
                 if (state == State.BINDING) {
             }
         };
 
+        /** A set of all the columns that display an error flag. */
+        private final Set<Column<?, T>> columnErrors = new HashSet<Grid.Column<?, T>>();
+
         public Editor() {
             saveButton = new Button();
             saveButton.setText(GridConstants.DEFAULT_SAVE_CAPTION);
             hideOverlay();
             grid.getEscalator().setScrollLocked(Direction.VERTICAL, false);
 
-            EditorRequest<T> request = new EditorRequest<T>(grid, rowIndex,
+            EditorRequest<T> request = new EditorRequestImpl<T>(grid, rowIndex,
                     null);
             handler.cancel(request);
             state = State.INACTIVE;
             state = State.SAVING;
             setButtonsEnabled(false);
             saveTimeout.schedule(SAVE_TIMEOUT_MS);
-            EditorRequest<T> request = new EditorRequest<T>(grid, rowIndex,
+            EditorRequest<T> request = new EditorRequestImpl<T>(grid, rowIndex,
                     saveRequestCallback);
             handler.save(request);
         }
             if (state == State.ACTIVATING) {
                 state = State.BINDING;
                 bindTimeout.schedule(BIND_TIMEOUT_MS);
-                EditorRequest<T> request = new EditorRequest<T>(grid, rowIndex,
-                        bindRequestCallback);
+                EditorRequest<T> request = new EditorRequestImpl<T>(grid,
+                        rowIndex, bindRequestCallback);
                 handler.bind(request);
                 grid.getEscalator().setScrollLocked(Direction.VERTICAL, true);
             }
             editorOverlay.removeFromParent();
 
             scrollHandler.removeHandler();
+
+            clearEditorColumnErrors();
         }
 
         protected void setStylePrimaryName(String primaryName) {
         public String getCancelCaption() {
             return cancelButton.getText();
         }
+
+        public void setEditorColumnError(Column<?, T> column, boolean hasError) {
+            if (state != State.ACTIVE && state != State.SAVING) {
+                throw new IllegalStateException("Cannot set cell error "
+                        + "status: editor is neither active nor saving.");
+            }
+
+            if (isEditorColumnError(column) == hasError) {
+                return;
+            }
+
+            Element editorCell = getWidget(column).getElement()
+                    .getParentElement();
+            if (hasError) {
+                editorCell.addClassName(ERROR_CLASS_NAME);
+                columnErrors.add(column);
+            } else {
+                editorCell.removeClassName(ERROR_CLASS_NAME);
+                columnErrors.remove(column);
+            }
+        }
+
+        public void clearEditorColumnErrors() {
+
+            /*
+             * editorOverlay has no children if it's not active, effectively
+             * making this loop a NOOP.
+             */
+            Element e = editorOverlay.getFirstChildElement();
+            while (e != null) {
+                e.removeClassName(ERROR_CLASS_NAME);
+                e = e.getNextSiblingElement();
+            }
+
+            columnErrors.clear();
+        }
+
+        public boolean isEditorColumnError(Column<?, T> column) {
+            return columnErrors.contains(column);
+        }
     }
 
     public static abstract class AbstractGridKeyEvent<HANDLER extends AbstractGridKeyEventHandler>
          * 
          * @return {@code true} if this column is editable, {@code false}
          *         otherwise
-         *
+         * 
          * @see #setEditable(boolean)
          */
         public boolean isEditable() {

server/src/com/vaadin/ui/Grid.java

 
         private CommitException cause;
 
+        private Set<Column> errorColumns = new HashSet<Column>();
+
         public CommitErrorEvent(Grid grid, CommitException cause) {
             super(grid);
             this.cause = cause;
             return cause.getCause() instanceof InvalidValueException;
         }
 
+        /**
+         * Marks that an error indicator should be shown for the editor of a
+         * column.
+         * 
+         * @param column
+         *            the column to show an error for
+         */
+        public void addErrorColumn(Column column) {
+            errorColumns.add(column);
+        }
+
+        /**
+         * Gets all the columns that have been marked as erroneous.
+         * 
+         * @return an umodifiable collection of erroneous columns
+         */
+        public Collection<Column> getErrorColumns() {
+            return Collections.unmodifiableCollection(errorColumns);
+        }
+
     }
 
     /**
                     .getCause().getInvalidFields();
 
             if (!invalidFields.isEmpty()) {
-                // Validation error, show first failure as
-                // "<Column header>: <message>"
+                Object firstErrorPropertyId = null;
+                Field<?> firstErrorField = null;
+
                 FieldGroup fieldGroup = event.getCause().getFieldGroup();
-                Object propertyId = getFirstPropertyId(fieldGroup,
-                        invalidFields.keySet());
-                Field<?> field = fieldGroup.getField(propertyId);
-                String caption = getColumn(propertyId).getHeaderCaption();
-                // TODO This should be shown in the editor component once
-                // there is a place for that. Optionally, all errors should be
-                // shown
-                Notification.show(caption + ": "
-                        + invalidFields.get(field).getLocalizedMessage(),
-                        Type.ERROR_MESSAGE);
+                for (Column column : getColumns()) {
+                    Object propertyId = column.getPropertyId();
+                    Field<?> field = fieldGroup.getField(propertyId);
+                    if (invalidFields.keySet().contains(field)) {
+                        event.addErrorColumn(column);
+
+                        if (firstErrorPropertyId == null) {
+                            firstErrorPropertyId = propertyId;
+                            firstErrorField = field;
+                        }
+                    }
+                }
+
+                /*
+                 * Validation error, show first failure as
+                 * "<Column header>: <message>"
+                 */
+                String caption = getColumn(firstErrorPropertyId)
+                        .getHeaderCaption();
+                String message = invalidFields.get(firstErrorField)
+                        .getLocalizedMessage();
+                /*
+                 * TODO This should be shown in the editor component once there
+                 * is a place for that. Optionally, all errors should be shown
+                 */
+                Notification.show(caption + ": " + message, Type.ERROR_MESSAGE);
 
             } else {
                 com.vaadin.server.ErrorEvent.findErrorHandler(Grid.this).error(
 
             @Override
             public void save(int rowIndex) {
+                List<String> errorColumnIds = null;
                 boolean success = false;
                 try {
                     saveEditor();
                     success = true;
                 } catch (CommitException e) {
                     try {
-                        getEditorErrorHandler().commitError(
-                                new CommitErrorEvent(Grid.this, e));
+                        CommitErrorEvent event = new CommitErrorEvent(
+                                Grid.this, e);
+                        getEditorErrorHandler().commitError(event);
+
+                        errorColumnIds = new ArrayList<String>();
+                        for (Column column : event.getErrorColumns()) {
+                            errorColumnIds.add(column.state.id);
+                        }
                     } catch (Exception ee) {
                         // A badly written error handler can throw an exception,
                         // which would lock up the Grid
                 } catch (Exception e) {
                     handleError(e);
                 }
-                getEditorRpc().confirmSave(success);
+                getEditorRpc().confirmSave(success, errorColumnIds);
             }
 
             private void handleError(Exception e) {

shared/src/com/vaadin/shared/ui/grid/EditorClientRpc.java

  */
 package com.vaadin.shared.ui.grid;
 
+import java.util.List;
+
 import com.vaadin.shared.communication.ClientRpc;
 
 /**
      * 
      * @param saveSucceeded
      *            <code>true</code> iff the save action was successful
+     * @param errorColumnsIds
+     *            a list of column keys that should get error markers, or
+     *            <code>null</code> if there should be no error markers
      */
-    void confirmSave(boolean saveSucceeded);
+    void confirmSave(boolean saveSucceeded, List<String> errorColumnsIds);
 }

uitest/src/com/vaadin/testbench/elements/GridElement.java

                     .isElementPresent(By.vaadin("#editor[" + colIndex + "]"));
         }
 
+        /**
+         * Checks whether a field is marked with an error.
+         * 
+         * @param colIndex
+         *            column index
+         * @return <code>true</code> iff the field is marked with an error
+         */
+        public boolean isFieldErrorMarked(int colIndex) {
+            return getField(colIndex).getAttribute("class").contains("error");
+        }
+
         /**
          * Saves the fields of this editor.
          * <p>

uitest/src/com/vaadin/tests/components/grid/basicfeatures/client/GridEditorClientTest.java

                         .getText());
     }
 
+    @Test
     public void testUneditableColumn() {
         selectMenuPath("Component", "Editor", "Edit row 5");
 
                 getGridElement().getEditor().isEditable(3));
     }
 
+    @Test
+    public void testErrorField() {
+        selectMenuPath(EDIT_ROW_5);
+
+        assertTrue("No errors should be present",
+                getEditor().findElements(By.className("error")).isEmpty());
+        selectMenuPath("Component", "Editor", "Toggle second editor error");
+        getSaveButton().click();
+
+        assertEquals("Unexpected amount of error fields", 1, getEditor()
+                .findElements(By.className("error")).size());
+    }
+
     protected WebElement getSaveButton() {
         return getEditor().findElement(By.className("v-grid-editor-save"));
     }

uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorTest.java

 
         GridEditorElement editor = getGridElement().getEditor();
 
+        assertFalse(
+                "Field 7 should not have been marked with an error before error",
+                editor.isFieldErrorMarked(7));
+
         WebElement intField = editor.getField(7);
         intField.clear();
         intField.sendKeys("banana phone");
         assertEquals("Column 7: Could not convert value to Integer",
                 n.getCaption());
         n.close();
+        assertTrue("Field 7 should have been marked with an error after error",
+                editor.isFieldErrorMarked(7));
         editor.cancel();
 
         selectMenuPath(EDIT_ITEM_100);

uitest/src/com/vaadin/tests/widgetset/client/grid/GridBasicClientFeaturesWidget.java

 package com.vaadin.tests.widgetset.client.grid;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import com.vaadin.client.widget.grid.events.ScrollHandler;
 import com.vaadin.client.widget.grid.selection.SelectionModel.None;
 import com.vaadin.client.widgets.Grid;
+import com.vaadin.client.widgets.Grid.Column;
 import com.vaadin.client.widgets.Grid.FooterRow;
 import com.vaadin.client.widgets.Grid.HeaderRow;
 import com.vaadin.client.widgets.Grid.SelectionMode;
 
         @Override
         public void save(EditorRequest<List<Data>> request) {
+            if (secondEditorError) {
+                log.setText("Syntethic fail of editor in column 2");
+                request.failure(Collections.<Column<?, List<Data>>> singleton(grid
+                        .getColumn(2)));
+                return;
+            }
             try {
                 log.setText("Row " + request.getRowIndex() + " edit committed");
                 List<Data> rowData = ds.getRow(request.getRowIndex());
                 request.success();
             } catch (Exception e) {
                 Logger.getLogger(getClass().getName()).warning(e.toString());
-                request.fail();
+                request.failure(null);
             }
         }
 
     private final ListDataSource<List<Data>> ds;
     private final ListSorter<List<Data>> sorter;
 
+    private boolean secondEditorError = false;
+
     /**
      * Our basic data object
      */
             }
         }, "Component", "Editor");
 
+        addMenuCommand("Toggle second editor error", new ScheduledCommand() {
+            @Override
+            public void execute() {
+                secondEditorError = !secondEditorError;
+            }
+        }, "Component", "Editor");
     }
 
     private void configureFooterRow(final FooterRow row) {