Issue 618 - Keyboard navigation in sparsely readonly table

CHANGELOG.md

@@ -2,6 +2,12 @@
 All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](http://semver.org/).
 
+## Unreleased
+### Fixed
+
+- [#618](https://github.com/plotly/dash-table/issues/618) Fix a bug with keyboard navigation not
+working correctly in certain circumstances when the table contains `readonly` columns.
+
 ## [4.4.0] - 2019-10-08
 ### Added
 [#546](https://github.com/plotly/dash-table/issues/546)

demo/AppMode.ts

@@ -20,6 +20,7 @@ export enum AppMode {
     Default = 'default',
     Formatting = 'formatting',
     ReadOnly = 'readonly',
+    SomeReadOnly = 'someReadonly',
     ColumnsInSpace = 'columnsInSpace',
     SingleHeaders = 'singleHeaders',
     TaleOfTwoTables = 'taleOfTwoTables',
@@ -134,6 +135,18 @@ function getReadonlyState() {
     return state;
 }
 
+function getSomeReadonlyState() {
+    const state = getDefaultState();
+    state.tableProps.editable = true;
+    state.tableProps.row_deletable = false;
+
+    R.forEach(column => {
+        column.editable = !R.includes(column.id, ['bbb', 'eee', 'fff']);
+    }, state.tableProps.columns || []);
+
+    return state;
+}
+
 function getSpaceInColumn() {
     const state = getDefaultState(generateSpaceMockData);
     state.tableProps.filter_action = TableAction.Native;
@@ -336,6 +349,8 @@ function getModeState(mode: string | null) {
             return getFormattingState();
         case AppMode.ReadOnly:
             return getReadonlyState();
+        case AppMode.SomeReadOnly:
+            return getSomeReadonlyState();
         case AppMode.ColumnsInSpace:
             return getSpaceInColumn();
         case AppMode.Tooltips:

src/dash-table/components/CellLabel/index.tsx

@@ -3,6 +3,7 @@ import React, {
 } from 'react';
 
 interface IProps {
+    active: boolean;
     className: string;
     value: any;
 }
@@ -15,11 +16,34 @@ export default class CellLabel extends PureComponent<IProps> {
         } = this.props;
 
         return (<div
+            ref='el'
             className={className}
+            tabIndex={-1}
         >
             {typeof value === 'boolean' ?
                 value.toString() :
                 value}
         </div>);
     }
+
+    componentDidUpdate() {
+        this.setFocus();
+    }
+
+    componentDidMount() {
+        this.setFocus();
+    }
+
+    private setFocus() {
+        const { active } = this.props;
+        if (!active) {
+            return;
+        }
+
+        const el = this.refs.el as HTMLDivElement;
+
+        if (el && document.activeElement !== el) {
+            el.focus();
+        }
+    }
 }

src/dash-table/derived/cell/contents.tsx

@@ -164,6 +164,7 @@ class Contents {
                     formatters[columnIndex](datum[column.id]);
 
                 return (<CellLabel
+                    active={active}
                     className={className}
                     key={`column-${columnIndex}`}
                     value={resolvedValue}

tests/cypress/tests/standalone/readonly_navigation_test.ts

@@ -1,10 +1,12 @@
+import * as R from 'ramda';
+
 import DashTable from 'cypress/DashTable';
 import DOM from 'cypress/DOM';
 import Key from 'cypress/Key';
 
 import { AppMode } from 'demo/AppMode';
 
-Object.values([AppMode.ReadOnly]).forEach(mode => {
+Object.values([AppMode.ReadOnly, AppMode.SomeReadOnly]).forEach(mode => {
     describe(`navigate (readonly), mode=${mode}`, () => {
         beforeEach(() => {
             cy.visit(`http://localhost:8080?mode=${mode}`);
@@ -20,17 +22,37 @@ Object.values([AppMode.ReadOnly]).forEach(mode => {
             // version of the test for the read only table.
             describe('into a dropdown cell', () => {
                 beforeEach(() => {
-                    DashTable.getCell(3, 5).click();
+                    DashTable.getCellById(3, 'ggg').click();
+                });
+
+                it('can move', () => {
+                    R.forEach(() => {
+                        DOM.focused.type(Key.ArrowRight);
+
+                        DashTable.getCellById(3, 'bbb').should('have.class', 'focused');
+                        DOM.focused.type(Key.ArrowLeft, { force: true });
+
+                        DashTable.getCellById(3, 'bbb').should('not.have.class', 'focused');
+                        DashTable.getCellById(3, 'ggg').should('have.class', 'focused');
+                    }, R.range(0, 2));
+                });
+            });
+
+            describe('into a label cell', () => {
+                beforeEach(() => {
+                    DashTable.getCellById(3, 'eee').click();
                 });
 
                 it('can move', () => {
-                    DOM.focused.type(Key.ArrowRight);
+                    R.forEach(() => {
+                        DOM.focused.type(Key.ArrowRight);
 
-                    DashTable.getCell(3, 6).should('have.class', 'focused');
-                    DOM.focused.type(Key.ArrowLeft, { force: true });
+                        DashTable.getCellById(3, 'fff').should('have.class', 'focused');
+                        DOM.focused.type(Key.ArrowLeft, { force: true });
 
-                    DashTable.getCell(3, 6).should('not.have.class', 'focused');
-                    DashTable.getCell(3, 5).should('have.class', 'focused');
+                        DashTable.getCellById(3, 'fff').should('not.have.class', 'focused');
+                        DashTable.getCellById(3, 'eee').should('have.class', 'focused');
+                    }, R.range(0, 2));
                 });
             });
         });