141 move in cell (plotly#172)

* Propagate to ControlledTable on navKeys * Test if arrow keys can navigate and save cell value * Add keyboard navigation tests for focused inputs * Update changelog * Add onMouseUp functionality for arrow key navigation * Calculate derivedHandlers for onMouseUp * Add test for caret selection on focused input * Rebuild bundle * Jump to next cell on enter key * Fix focused input test by doubleclicking instead of enter * Remove .only from copy paste test * Remove artifact from it's tomb of copy_paste_test
HammadTheOne · Oct 30, 2018 · 8fad8d6 · 8fad8d6
1 parent 75fa7c3
commit 8fad8d6
Show file tree

Hide file tree

Showing 14 changed files with 198 additions and 31 deletions.
diff --git a/packages/dash-table/CHANGELOG.md b/packages/dash-table/CHANGELOG.md
@@ -399,7 +399,7 @@ Derived properties allow the component to expose complex state that can be usefu
 
 ## RC8 - Improve props typing
 
-    Issue: https://github.com/plotly/dash-table/issues/143
+    Issue: https://github.com/plotly/dash-table/issues/143    
 
 ## RC9 - Sort ascending on first click
 
@@ -417,4 +417,9 @@ Derived properties allow the component to expose complex state that can be usefu
 
 ## RC12 - Rename selected_cell -> selected_cells
 
-    Issue: https://github.com/plotly/dash-table/issues/177
+    Issue: https://github.com/plotly/dash-table/issues/177
+
+## RC13 - Allow keyboard navigation on focused input
+
+    Issue: https://github.com/plotly/dash-table/issues/141
+    Issue: https://github.com/plotly/dash-table/issues/143
diff --git a/packages/dash-table/dash_table/bundle.js b/packages/dash-table/dash_table/bundle.js
diff --git a/packages/dash-table/dash_table/demo.js b/packages/dash-table/dash_table/demo.js
diff --git a/packages/dash-table/dash_table/package.json b/packages/dash-table/dash_table/package.json
@@ -1,6 +1,6 @@
 {
   "name": "dash-table",
-  "version": "3.1.0rc12",
+  "version": "3.1.0rc13",
   "description": "Dash table",
   "main": "build/index.js",
   "scripts": {

diff --git a/packages/dash-table/package.json b/packages/dash-table/package.json
@@ -1,6 +1,6 @@
 {
   "name": "dash-table",
-  "version": "3.1.0rc12",
+  "version": "3.1.0rc13",
   "description": "Dash table",
   "main": "build/index.js",
   "scripts": {

diff --git a/packages/dash-table/src/dash-table/components/CellInput/index.tsx b/packages/dash-table/src/dash-table/components/CellInput/index.tsx
@@ -14,7 +14,7 @@ import {
 } from 'dash-table/components/CellInput/props';
 
 import {
-    KEY_CODES
+    KEY_CODES, isNavKey
 } from 'dash-table/utils/unicode';
 import { ColumnType } from 'dash-table/components/Table/props';
 import dropdownHelper from 'dash-table/components/dropdownHelper';
@@ -93,6 +93,7 @@ export default class CellInput extends PureComponent<ICellProps, ICellState> {
             focused,
             onClick,
             onDoubleClick,
+            onMouseUp,
             onPaste
         } = this.propsWithDefaults;
 
@@ -105,7 +106,8 @@ export default class CellInput extends PureComponent<ICellProps, ICellState> {
         const attributes = {
             className: classes.join(' '),
             onClick: onClick,
-            onDoubleClick: onDoubleClick
+            onDoubleClick: onDoubleClick,
+            onMouseUp: onMouseUp
         };
 
         const readonly = (!active && this.state.value === this.props.value) || !editable;
@@ -166,7 +168,15 @@ export default class CellInput extends PureComponent<ICellProps, ICellState> {
     }
 
     handleKeyDown = (e: KeyboardEvent) => {
-        if (e.keyCode !== KEY_CODES.ENTER && e.keyCode !== KEY_CODES.TAB) {
+        const is_focused = this.props.focused;
+
+        if (is_focused && 
+            (e.keyCode !== KEY_CODES.TAB && e.keyCode !== KEY_CODES.ENTER)
+        ) {
+            return;
+        }
+
+        if(!is_focused && !isNavKey(e.keyCode)) {
             return;
         }
 

diff --git a/packages/dash-table/src/dash-table/components/CellInput/props.ts b/packages/dash-table/src/dash-table/components/CellInput/props.ts
@@ -14,6 +14,7 @@ export interface ICellHandlerProps {
     onChange: (e: ChangeEvent) => void;
     onClick: (e: React.MouseEvent) => void;
     onDoubleClick: (e: React.MouseEvent) => void;
+    onMouseUp: (e: React.MouseEvent) => void;
     onPaste: (e: React.ClipboardEvent<Element>) => void;
 }
 

diff --git a/packages/dash-table/src/dash-table/components/ControlledTable/index.tsx b/packages/dash-table/src/dash-table/components/ControlledTable/index.tsx
@@ -207,23 +207,20 @@ export default class ControlledTable extends PureComponent<ControlledTableProps,
             return;
         }
 
-        if (
-            e.keyCode === KEY_CODES.ENTER &&
-            !is_focused &&
-            isEditable(editable, columns[active_cell[1]])
+        if(!is_focused &&
+            isNavKey(e.keyCode)
         ) {
-            setProps({ is_focused: true });
-            return;
+            this.switchCell(e);
         }
 
         if (
             is_focused &&
-            (e.keyCode !== KEY_CODES.TAB && e.keyCode !== KEY_CODES.ENTER)
+            !isNavKey(e.keyCode)
         ) {
             return;
         }
 
-        if (isNavKey(e.keyCode)) {
+        if (e.keyCode === KEY_CODES.TAB || e.keyCode === KEY_CODES.ENTER) {
             this.switchCell(e);
             return;
         }
@@ -236,11 +233,11 @@ export default class ControlledTable extends PureComponent<ControlledTableProps,
         } else if (
             // if we have any non-meta key enter editable mode
 
-            !this.props.is_focused &&
+            !is_focused &&
             isEditable(editable, columns[active_cell[1]]) &&
             !isMetaKey(e.keyCode)
         ) {
-            setProps({ is_focused: true });
+            // setProps({ is_focused: true });
         }
 
         return;

diff --git a/packages/dash-table/src/dash-table/components/Table/Table.less b/packages/dash-table/src/dash-table/components/Table/Table.less
@@ -310,6 +310,12 @@
                 position: absolute;
                 left: 0;
                 top: 0;
+                &.unfocused::selection {
+                    background-color: transparent;
+                }
+                &.unfocused {
+                    caret-color: transparent;
+                }
             }
 
             div.dash-cell-value {

diff --git a/packages/dash-table/src/dash-table/derived/cell/eventHandler.ts b/packages/dash-table/src/dash-table/derived/cell/eventHandler.ts
@@ -2,7 +2,7 @@
 import { memoizeOneFactory } from 'core/memoizer';
 import memoizerCache from 'core/memoizerCache';
 import { ICellFactoryOptions } from 'dash-table/components/Table/props';
-import { handleChange, handleClick, handleDoubleClick, handlePaste } from 'dash-table/handlers/cellEvents';
+import { handleChange, handleClick, handleDoubleClick, handleOnMouseUp, handlePaste } from 'dash-table/handlers/cellEvents';
 
 type CacheArgs = [Handler, number, number];
 type GetterArgs = [HandlerFn, number, number];
@@ -11,6 +11,7 @@ export enum Handler {
     Change = 'change',
     Click = 'click',
     DoubleClick = 'doubleclick',
+    MouseUp = 'mouseup',
     Paste = 'paste'
 }
 
@@ -32,6 +33,7 @@ const getter = (propsFn: () => ICellFactoryOptions): CacheFn => {
         [Handler.Change, handleChange.bind(undefined, propsFn)],
         [Handler.Click, handleClick.bind(undefined, propsFn)],
         [Handler.DoubleClick, handleDoubleClick.bind(undefined, propsFn)],
+        [Handler.MouseUp, handleOnMouseUp.bind(undefined, propsFn)],
         [Handler.Paste, handlePaste.bind(undefined, propsFn)]
     ]);
 

diff --git a/packages/dash-table/src/dash-table/derived/cell/eventHandlerProps.ts b/packages/dash-table/src/dash-table/derived/cell/eventHandlerProps.ts
@@ -22,6 +22,7 @@ const getter = (propsFn: () => ICellFactoryOptions): CacheFn => {
             onChange: derivedHandlers(Handler.Change, rowIndex, columnIndex),
             onClick: derivedHandlers(Handler.Click, rowIndex, columnIndex),
             onDoubleClick: derivedHandlers(Handler.DoubleClick, rowIndex, columnIndex),
+            onMouseUp: derivedHandlers(Handler.MouseUp, rowIndex, columnIndex),
             onPaste: derivedHandlers(Handler.Paste, rowIndex, columnIndex)
         } as ICellHandlerProps;
     };

diff --git a/packages/dash-table/src/dash-table/handlers/cellEvents.ts b/packages/dash-table/src/dash-table/handlers/cellEvents.ts
@@ -1,5 +1,6 @@
 import * as R from 'ramda';
 import { SelectedCells, ICellFactoryOptions } from 'dash-table/components/Table/props';
+import isActive from 'dash-table/derived/cell/isActive';
 
 function isCellSelected(selectedCells: SelectedCells, idx: number, i: number) {
     return selectedCells && R.contains([idx, i], selectedCells);
@@ -8,19 +9,15 @@ function isCellSelected(selectedCells: SelectedCells, idx: number, i: number) {
 export const handleClick = (propsFn: () => ICellFactoryOptions, idx: number, i: number, e: any) => {
     const {
         editable,
-        is_focused,
         selected_cells,
         setProps
     } = propsFn();
 
-    const selected = isCellSelected(selected_cells, idx, i);
-
     if (!editable) {
         return;
     }
-    if (!is_focused) {
-        e.preventDefault();
-    }
+
+    const selected = isCellSelected(selected_cells, idx, i);
 
     // don't update if already selected
     if (selected) {
@@ -105,6 +102,24 @@ export const handleChange = (propsFn: () => ICellFactoryOptions, idx: number, i:
     setProps({
         data: newData
     });
+
+};
+
+export const handleOnMouseUp = (propsFn: () => ICellFactoryOptions, idx: number, i: number, e: any) => {
+    const {
+        active_cell,
+        is_focused,
+    } = propsFn();
+
+    const active = isActive(active_cell, idx, i);
+
+    if(!is_focused && active) {
+        e.preventDefault();
+        // We do this because mouseMove can change the selection, we don't want
+        // to check for all mouse movements, for performance reasons.
+        const input = e.target;
+        input.setSelectionRange(0, input.value ? input.value.length : 0);
+    }
 };
 
 export const handlePaste = (_propsFn: () => ICellFactoryOptions, _idx: number, _i: number, e: ClipboardEvent) => {

diff --git a/packages/dash-table/tests/cypress/tests/server/dash_test.ts b/packages/dash-table/tests/cypress/tests/server/dash_test.ts
@@ -56,6 +56,78 @@ describe('dash basic', () => {
             });
         });
     });
+    describe('ArrowKeys navigation', () => {
+        describe('When active, but not focused', () => {
+            // https://github.com/plotly/dash-table/issues/141
+            it('can edit last, update data on "arrowleft", and move one cell to the left', () => {
+                const startingCell = [249,1]
+                const targetCell = [249, 0]
+                DashTable.getCell(startingCell[0], startingCell[1]).click();
+                DOM.focused.then($input => {
+                    const initialValue = $input.val();
+
+                    DOM.focused.type(`abc${Key.ArrowLeft}`);
+
+                    cy.get('#container').should($container => {
+                        expect($container.first()[0].innerText).to.equal(`[249][1] = ${initialValue} -> abc`);
+                    });
+                });
+                DashTable.getCell(targetCell[0], targetCell[1]).should('have.class', 'focused');
+            });
+
+            // https://github.com/plotly/dash-table/issues/141
+            it('can edit last, update data on "arrowup", and move one cell up', () => {
+                const startingCell = [249,0]
+                const targetCell = [248, 0]
+                DashTable.getCell(startingCell[0], startingCell[1]).click();
+                DOM.focused.then($input => {
+                    const initialValue = $input.val();
+
+                    DOM.focused.type(`abc${Key.ArrowUp}`);
+
+                    cy.get('#container').should($container => {
+                        expect($container.first()[0].innerText).to.equal(`[249][0] = ${initialValue} -> abc`);
+                    });
+                });
+                DashTable.getCell(targetCell[0], targetCell[1]).should('have.class', 'focused');
+            });
+
+            // https://github.com/plotly/dash-table/issues/141
+            it('can edit last, update data on "arrowright", and move one cell to the right', () => {
+                const startingCell = [249,0]
+                const targetCell = [249, 1]
+                DashTable.getCell(startingCell[0], startingCell[1]).click();
+                DOM.focused.then($input => {
+                    const initialValue = $input.val();
+
+                    DOM.focused.type(`abc${Key.ArrowRight}`);
+
+                    cy.get('#container').should($container => {
+                        expect($container.first()[0].innerText).to.equal(`[249][0] = ${initialValue} -> abc`);
+                    });
+                });
+                DashTable.getCell(targetCell[0], targetCell[1]).should('have.class', 'focused');
+            });
+
+
+            // https://github.com/plotly/dash-table/issues/141
+            it('can edit last, update data on "arrowdown", and move one cell down', () => {
+                const startingCell = [249,0]
+                const targetCell = [249, 1]
+                DashTable.getCell(startingCell[0], startingCell[1]).click();
+                DOM.focused.then($input => {
+                    const initialValue = $input.val();
+
+                    DOM.focused.type(`abc${Key.ArrowRight}`);
+
+                    cy.get('#container').should($container => {
+                        expect($container.first()[0].innerText).to.equal(`[249][0] = ${initialValue} -> abc`);
+                    });
+                });
+                DashTable.getCell(targetCell[0], targetCell[1]).should('have.class', 'focused');
+            });
+        })
+    })
 
     it('can edit last and update data when clicking outside of cell', () => {
         DashTable.getCell(249, 0).click();