Grid Editor: make Tab key skip read-only/disabled fields (#11586)

* Grid Editor: make Tab key skip read-only/disabled fields

Closes #11584

* Extracted DefaultEditorEventHandler.getDeltaFromKeyDownEvent() which allows for easy further Grid Editor customization

* Make DefaultEditorEventHandler.Delta public so that getDeltaFromKeyDownEvent() can be overridden

* Fixed exception in isEditable() if the widget was not a Field

* Refactored DefaultEditorEventHandler.Delta to CursorMoveDelta which expresses the intent more clearly

* Merge branch 'master' into master

3 files changed, 111 insertions, 29 deletions

diff --git a/client/src/main/java/com/vaadin/client/widget/grid/DefaultEditorEventHandler.java b/client/src/main/java/com/vaadin/client/widget/grid/DefaultEditorEventHandler.java
index 426d7c3016..016c36c49a 100644
--- a/client/src/main/java/com/vaadin/client/widget/grid/DefaultEditorEventHandler.java
+++ b/client/src/main/java/com/vaadin/client/widget/grid/DefaultEditorEventHandler.java
@@ -21,7 +21,10 @@ import com.google.gwt.dom.client.Element;
 import com.google.gwt.event.dom.client.KeyCodes;
 import com.google.gwt.user.client.Event;
 import com.google.gwt.user.client.ui.Widget;
+import com.vaadin.client.ComponentConnector;
+import com.vaadin.client.Util;
 import com.vaadin.client.WidgetUtil;
+import com.vaadin.client.ui.AbstractFieldConnector;
 import com.vaadin.client.ui.FocusUtil;
 import com.vaadin.client.widgets.Grid;
 import com.vaadin.client.widgets.Grid.Editor;
@@ -125,6 +128,46 @@ public class DefaultEditorEventHandler<T> implements Editor.EventHandler<T> {
     }
 
     /**
+     * Specifies the direction at which the focus should move.
+     */
+    public enum CursorMoveDelta {
+        UP(-1, 0), RIGHT(0, 1), DOWN(1, 0), LEFT(0, -1);
+
+        public final int rowDelta;
+        public final int colDelta;
+
+        CursorMoveDelta(int rowDelta, int colDelta) {
+            this.rowDelta = rowDelta;
+            this.colDelta = colDelta;
+        }
+
+        public boolean isChanged() {
+            return rowDelta != 0 || colDelta != 0;
+        }
+    }
+
+    /**
+     * Returns the direction to which the cursor should move.
+     *
+     * @param event
+     *            the mouse event, not null.
+     * @return the direction. May return null if the cursor should not move.
+     */
+    protected CursorMoveDelta getDeltaFromKeyDownEvent(
+            EditorDomEvent<T> event) {
+        Event e = event.getDomEvent();
+        if (e.getKeyCode() == KEYCODE_MOVE_VERTICAL) {
+            return e.getShiftKey() ? CursorMoveDelta.UP : CursorMoveDelta.DOWN;
+        } else if (e.getKeyCode() == KEYCODE_MOVE_HORIZONTAL) {
+            // Prevent tab out of Grid Editor
+            event.getDomEvent().preventDefault();
+            return e.getShiftKey() ? CursorMoveDelta.LEFT
+                    : CursorMoveDelta.RIGHT;
+        }
+        return null;
+    }
+
+    /**
      * Moves the editor to another row or another column if the received event
      * is a move event. The default implementation moves the editor to the
      * clicked row if the event is a click; otherwise, if the event is a keydown
@@ -150,47 +193,37 @@ public class DefaultEditorEventHandler<T> implements Editor.EventHandler<T> {
             return true;
         } else if (e.getTypeInt() == Event.ONKEYDOWN) {
 
-            int rowDelta = 0;
-            int colDelta = 0;
-
-            if (e.getKeyCode() == KEYCODE_MOVE_VERTICAL) {
-                rowDelta = (e.getShiftKey() ? -1 : +1);
-            } else if (e.getKeyCode() == KEYCODE_MOVE_HORIZONTAL) {
-                colDelta = (e.getShiftKey() ? -1 : +1);
-                // Prevent tab out of Grid Editor
-                event.getDomEvent().preventDefault();
-            }
-
-            final boolean changed = rowDelta != 0 || colDelta != 0;
+            CursorMoveDelta delta = getDeltaFromKeyDownEvent(event);
+            final boolean changed = delta != null;
 
             if (changed) {
 
                 int columnCount = event.getGrid().getVisibleColumns().size();
 
-                int colIndex = colDelta > 0
+                int colIndex = delta.colDelta > 0
                         ? findNextEditableColumnIndex(event.getGrid(),
-                                event.getFocusedColumnIndex() + colDelta)
+                                event.getFocusedColumnIndex() + delta.colDelta)
                         : findPrevEditableColumnIndex(event.getGrid(),
-                                event.getFocusedColumnIndex() + colDelta);
+                                event.getFocusedColumnIndex() + delta.colDelta);
                 int rowIndex = event.getRowIndex();
 
                 // Handle row change with horizontal move when column goes out
                 // of range.
-                if (rowDelta == 0 && colIndex < 0) {
-                    if (colDelta > 0
+                if (delta.rowDelta == 0 && colIndex < 0) {
+                    if (delta.colDelta > 0
                             && rowIndex < event.getGrid().getDataSource().size()
                                     - 1) {
-                        rowDelta = 1;
+                        delta = CursorMoveDelta.DOWN;
                         colIndex = findNextEditableColumnIndex(event.getGrid(),
                                 0);
-                    } else if (colDelta < 0 && rowIndex > 0) {
-                        rowDelta = -1;
+                    } else if (delta.colDelta < 0 && rowIndex > 0) {
+                        delta = CursorMoveDelta.UP;
                         colIndex = findPrevEditableColumnIndex(event.getGrid(),
                                 columnCount - 1);
                     }
                 }
 
-                editRow(event, rowIndex + rowDelta, colIndex);
+                editRow(event, rowIndex + delta.rowDelta, colIndex);
             }
 
             return changed;
@@ -212,16 +245,44 @@ public class DefaultEditorEventHandler<T> implements Editor.EventHandler<T> {
      *         <code>startingWith</code> itself. Returns -1 if there is no such
      *         column.
      */
-    private int findNextEditableColumnIndex(Grid<T> grid, int startingWith) {
+    protected int findNextEditableColumnIndex(Grid<T> grid, int startingWith) {
         final List<Grid.Column<?, T>> columns = grid.getVisibleColumns();
         for (int i = startingWith; i < columns.size(); i++) {
-            if (columns.get(i).isEditable()) {
+            if (isEditable(grid, columns.get(i))) {
                 return i;
             }
         }
         return -1;
     }
 
+    protected boolean isEditable(Grid<T> grid, Grid.Column<?, T> column) {
+        if (!column.isEditable()) {
+            return false;
+        }
+
+        // figure out whether the widget nested in the editor cell is editable.
+        // if it is disabled or read-only then it is not editable.
+
+        final Widget editorCell = grid.getEditorWidget(column);
+        final ComponentConnector connector = Util.findConnectorFor(editorCell);
+        if (connector == null) {
+            // not a Vaadin Connector, perhaps something generated by the
+            // renderer? Assume it's enabled.
+            return true;
+        }
+
+        if (!connector.isEnabled()) {
+            return false;
+        }
+        if (connector instanceof AbstractFieldConnector) {
+            final AbstractFieldConnector field = (AbstractFieldConnector) connector;
+            if (field.isReadOnly()) {
+                return false;
+            }
+        }
+        return true;
+    }
+
     /**
      * Finds index of the last editable column, searching backwards starting at
      * the specified index.
@@ -235,10 +296,10 @@ public class DefaultEditorEventHandler<T> implements Editor.EventHandler<T> {
      *         <code>startingWith</code> itself. Returns -1 if there is no such
      *         column.
      */
-    private int findPrevEditableColumnIndex(Grid<T> grid, int startingWith) {
+    protected int findPrevEditableColumnIndex(Grid<T> grid, int startingWith) {
         final List<Grid.Column<?, T>> columns = grid.getVisibleColumns();
         for (int i = startingWith; i >= 0; i--) {
-            if (columns.get(i).isEditable()) {
+            if (isEditable(grid, columns.get(i))) {
                 return i;
             }
         }
diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridEditorTabSkipsNonEditableCells.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridEditorTabSkipsNonEditableCells.java
index cf8c673c09..cd7b454380 100644
--- a/uitest/src/main/java/com/vaadin/tests/components/grid/GridEditorTabSkipsNonEditableCells.java
+++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridEditorTabSkipsNonEditableCells.java
@@ -31,7 +31,14 @@ public class GridEditorTabSkipsNonEditableCells extends AbstractTestUI {
         grid.getEditor().setEnabled(true);
         grid.getColumn("col1").setEditorComponent(new TextField());
         grid.getColumn("col3").setEditorComponent(new TextField());
-        grid.setColumnOrder("col0", "col1", "col2", "col3", "col4");
+        final TextField disabledField = new TextField();
+        disabledField.setEnabled(false);
+        grid.getColumn("col5").setEditorComponent(disabledField);
+        final TextField readOnlyField = new TextField();
+        readOnlyField.setReadOnly(true);
+        grid.getColumn("col6").setEditorComponent(readOnlyField);
+        grid.setColumnOrder("col0", "col1", "col2", "col3", "col4", "col5",
+                "col6");
 
         getLayout().addComponent(
                 new Button("Set Editor Buffered Mode On", event -> {
@@ -57,7 +64,7 @@ public class GridEditorTabSkipsNonEditableCells extends AbstractTestUI {
         return "Pressing TAB doesn't shift the focus to non-editable cells when the Grid is in edit mode.";
     }
 
-    public class TestBean {
+    public static class TestBean {
         private final int row;
 
         public TestBean(int row) {
@@ -84,6 +91,14 @@ public class GridEditorTabSkipsNonEditableCells extends AbstractTestUI {
             return "col4_" + row;
         }
 
+        public String getCol5() {
+            return "col5_" + row;
+        }
+
+        public String getCol6() {
+            return "col6_" + row;
+        }
+
         public void setCol0(String value) {
         }
 
@@ -98,5 +113,11 @@ public class GridEditorTabSkipsNonEditableCells extends AbstractTestUI {
 
         public void setCol4(String value) {
         }
+
+        public void setCol5(String value) {
+        }
+
+        public void setCol6(String value) {
+        }
     }
 }
diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridEditorTabSkipsNonEditableCellsTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridEditorTabSkipsNonEditableCellsTest.java
index 9092f0a22f..fca0439fdf 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/grid/GridEditorTabSkipsNonEditableCellsTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridEditorTabSkipsNonEditableCellsTest.java
@@ -20,7 +20,7 @@ import static org.junit.Assert.fail;
 @TestCategory("grid")
 public class GridEditorTabSkipsNonEditableCellsTest extends MultiBrowserTest {
     /**
-     * The grid with 5 columns. First, third and fifth columns are not editable.
+     * The grid with 7 columns. First, third and fifth columns are not editable.
      */
     private GridElement grid;
 
@@ -116,7 +116,7 @@ public class GridEditorTabSkipsNonEditableCellsTest extends MultiBrowserTest {
     private String getFocusedEditorCellContents() {
         final GridElement.GridEditorElement editor = grid.getEditor();
         final WebElement focusedElement = getFocusedElement();
-        for (int i = 0; i < 5; i++) {
+        for (int i = 0; i < 7; i++) {
             if (editor.isEditable(i)
                     && editor.getField(i).equals(focusedElement)) {
                 return (editor.getField(i).wrap(TextFieldElement.class))