From 9d1e30b09acb1fd2c8f820f453e0f9eb6979f179 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andre=CC=81=20Rivet?= Date: Tue, 8 Oct 2019 20:03:16 -0400 Subject: [PATCH 1/5] add failing tests --- demo/AppMode.ts | 20 +++++++++++ .../standalone/readonly_navigation_test.ts | 36 +++++++++++++++---- 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/demo/AppMode.ts b/demo/AppMode.ts index 3a293b053..018fdbd49 100644 --- a/demo/AppMode.ts +++ b/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,23 @@ function getReadonlyState() { return state; } +function getSomeReadonlyState() { + const state = getDefaultState(); + state.tableProps.editable = false; + state.tableProps.row_deletable = false; + + R.forEach(id => { + if (state.tableProps.columns) { + const column = state.tableProps.columns.find(c => c.id === id); + if (column) { + column.editable = false; + } + } + }, ['bbb', 'eee', 'fff']); + + return state; +} + function getSpaceInColumn() { const state = getDefaultState(generateSpaceMockData); state.tableProps.filter_action = TableAction.Native; @@ -336,6 +354,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: diff --git a/tests/cypress/tests/standalone/readonly_navigation_test.ts b/tests/cypress/tests/standalone/readonly_navigation_test.ts index 6d0a106a8..a9b9ee932 100644 --- a/tests/cypress/tests/standalone/readonly_navigation_test.ts +++ b/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, 'eee').should('not.have.class', 'focused'); + DashTable.getCellById(3, 'fff').should('have.class', 'focused'); + }, R.range(0, 2)); }); }); }); From 891bce48384a6746af4b4e1dbc99a857df116690 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andre=CC=81=20Rivet?= Date: Tue, 8 Oct 2019 20:08:51 -0400 Subject: [PATCH 2/5] incorrect tests --- tests/cypress/tests/standalone/readonly_navigation_test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/cypress/tests/standalone/readonly_navigation_test.ts b/tests/cypress/tests/standalone/readonly_navigation_test.ts index a9b9ee932..73af55cad 100644 --- a/tests/cypress/tests/standalone/readonly_navigation_test.ts +++ b/tests/cypress/tests/standalone/readonly_navigation_test.ts @@ -50,8 +50,8 @@ Object.values([AppMode.ReadOnly, AppMode.SomeReadOnly]).forEach(mode => { DashTable.getCellById(3, 'fff').should('have.class', 'focused'); DOM.focused.type(Key.ArrowLeft, { force: true }); - DashTable.getCellById(3, 'eee').should('not.have.class', 'focused'); - DashTable.getCellById(3, 'fff').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)); }); }); From fc0682a4b9277f987b9db92ad8a3e7a717941c54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andre=CC=81=20Rivet?= Date: Tue, 8 Oct 2019 20:19:05 -0400 Subject: [PATCH 3/5] focus on label cell if active --- package-lock.json | 2 +- src/dash-table/components/CellLabel/index.tsx | 24 +++++++++++++++++++ src/dash-table/derived/cell/contents.tsx | 1 + 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index a2fbd118f..5305fb71b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "dash-table", - "version": "4.3.0", + "version": "4.4.0", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/src/dash-table/components/CellLabel/index.tsx b/src/dash-table/components/CellLabel/index.tsx index 0c43ef44b..23da68414 100644 --- a/src/dash-table/components/CellLabel/index.tsx +++ b/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 { } = this.props; return (
{typeof value === 'boolean' ? value.toString() : value}
); } + + 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(); + } + } } \ No newline at end of file diff --git a/src/dash-table/derived/cell/contents.tsx b/src/dash-table/derived/cell/contents.tsx index 50cd45f0a..51b5664ed 100644 --- a/src/dash-table/derived/cell/contents.tsx +++ b/src/dash-table/derived/cell/contents.tsx @@ -164,6 +164,7 @@ class Contents { formatters[columnIndex](datum[column.id]); return ( Date: Tue, 8 Oct 2019 21:01:15 -0400 Subject: [PATCH 4/5] changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99ccdaa00..513572c84 100644 --- a/CHANGELOG.md +++ b/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) From 20469f0feeb2a40881848d768af90e0c51c26a81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andre=CC=81=20Rivet?= Date: Tue, 8 Oct 2019 21:10:37 -0400 Subject: [PATCH 5/5] improve test --- demo/AppMode.ts | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/demo/AppMode.ts b/demo/AppMode.ts index 018fdbd49..bf92099ed 100644 --- a/demo/AppMode.ts +++ b/demo/AppMode.ts @@ -137,17 +137,12 @@ function getReadonlyState() { function getSomeReadonlyState() { const state = getDefaultState(); - state.tableProps.editable = false; + state.tableProps.editable = true; state.tableProps.row_deletable = false; - R.forEach(id => { - if (state.tableProps.columns) { - const column = state.tableProps.columns.find(c => c.id === id); - if (column) { - column.editable = false; - } - } - }, ['bbb', 'eee', 'fff']); + R.forEach(column => { + column.editable = !R.includes(column.id, ['bbb', 'eee', 'fff']); + }, state.tableProps.columns || []); return state; }