Skip to content

Commit 7eb014d

Browse files
valentijnniemanvalentijnnieman
committed
141 move in cell (#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
1 parent b519240 commit 7eb014d

File tree

14 files changed

+198
-31
lines changed

14 files changed

+198
-31
lines changed

Diff for: ‎packages/dash-table/CHANGELOG.md

+7-2
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,7 @@ Derived properties allow the component to expose complex state that can be usefu
399399

400400
## RC8 - Improve props typing
401401

402-
Issue: https://github.com/plotly/dash-table/issues/143
402+
Issue: https://github.com/plotly/dash-table/issues/143
403403

404404
## RC9 - Sort ascending on first click
405405

@@ -417,4 +417,9 @@ Derived properties allow the component to expose complex state that can be usefu
417417

418418
## RC12 - Rename selected_cell -> selected_cells
419419

420-
Issue: https://github.com/plotly/dash-table/issues/177
420+
Issue: https://github.com/plotly/dash-table/issues/177
421+
422+
## RC13 - Allow keyboard navigation on focused input
423+
424+
Issue: https://github.com/plotly/dash-table/issues/141
425+
Issue: https://github.com/plotly/dash-table/issues/143

Diff for: ‎packages/dash-table/dash_table/bundle.js

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: ‎packages/dash-table/dash_table/demo.js

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: ‎packages/dash-table/dash_table/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "dash-table",
3-
"version": "3.1.0rc12",
3+
"version": "3.1.0rc13",
44
"description": "Dash table",
55
"main": "build/index.js",
66
"scripts": {

Diff for: ‎packages/dash-table/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "dash-table",
3-
"version": "3.1.0rc12",
3+
"version": "3.1.0rc13",
44
"description": "Dash table",
55
"main": "build/index.js",
66
"scripts": {

Diff for: ‎packages/dash-table/src/dash-table/components/CellInput/index.tsx

+13-3
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {
1414
} from 'dash-table/components/CellInput/props';
1515

1616
import {
17-
KEY_CODES
17+
KEY_CODES, isNavKey
1818
} from 'dash-table/utils/unicode';
1919
import { ColumnType } from 'dash-table/components/Table/props';
2020
import dropdownHelper from 'dash-table/components/dropdownHelper';
@@ -93,6 +93,7 @@ export default class CellInput extends PureComponent<ICellProps, ICellState> {
9393
focused,
9494
onClick,
9595
onDoubleClick,
96+
onMouseUp,
9697
onPaste
9798
} = this.propsWithDefaults;
9899

@@ -105,7 +106,8 @@ export default class CellInput extends PureComponent<ICellProps, ICellState> {
105106
const attributes = {
106107
className: classes.join(' '),
107108
onClick: onClick,
108-
onDoubleClick: onDoubleClick
109+
onDoubleClick: onDoubleClick,
110+
onMouseUp: onMouseUp
109111
};
110112

111113
const readonly = (!active && this.state.value === this.props.value) || !editable;
@@ -166,7 +168,15 @@ export default class CellInput extends PureComponent<ICellProps, ICellState> {
166168
}
167169

168170
handleKeyDown = (e: KeyboardEvent) => {
169-
if (e.keyCode !== KEY_CODES.ENTER && e.keyCode !== KEY_CODES.TAB) {
171+
const is_focused = this.props.focused;
172+
173+
if (is_focused &&
174+
(e.keyCode !== KEY_CODES.TAB && e.keyCode !== KEY_CODES.ENTER)
175+
) {
176+
return;
177+
}
178+
179+
if(!is_focused && !isNavKey(e.keyCode)) {
170180
return;
171181
}
172182

Diff for: ‎packages/dash-table/src/dash-table/components/CellInput/props.ts

+1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ export interface ICellHandlerProps {
1414
onChange: (e: ChangeEvent) => void;
1515
onClick: (e: React.MouseEvent) => void;
1616
onDoubleClick: (e: React.MouseEvent) => void;
17+
onMouseUp: (e: React.MouseEvent) => void;
1718
onPaste: (e: React.ClipboardEvent<Element>) => void;
1819
}
1920

Diff for: ‎packages/dash-table/src/dash-table/components/ControlledTable/index.tsx

+7-10
Original file line numberDiff line numberDiff line change
@@ -207,23 +207,20 @@ export default class ControlledTable extends PureComponent<ControlledTableProps,
207207
return;
208208
}
209209

210-
if (
211-
e.keyCode === KEY_CODES.ENTER &&
212-
!is_focused &&
213-
isEditable(editable, columns[active_cell[1]])
210+
if(!is_focused &&
211+
isNavKey(e.keyCode)
214212
) {
215-
setProps({ is_focused: true });
216-
return;
213+
this.switchCell(e);
217214
}
218215

219216
if (
220217
is_focused &&
221-
(e.keyCode !== KEY_CODES.TAB && e.keyCode !== KEY_CODES.ENTER)
218+
!isNavKey(e.keyCode)
222219
) {
223220
return;
224221
}
225222

226-
if (isNavKey(e.keyCode)) {
223+
if (e.keyCode === KEY_CODES.TAB || e.keyCode === KEY_CODES.ENTER) {
227224
this.switchCell(e);
228225
return;
229226
}
@@ -236,11 +233,11 @@ export default class ControlledTable extends PureComponent<ControlledTableProps,
236233
} else if (
237234
// if we have any non-meta key enter editable mode
238235

239-
!this.props.is_focused &&
236+
!is_focused &&
240237
isEditable(editable, columns[active_cell[1]]) &&
241238
!isMetaKey(e.keyCode)
242239
) {
243-
setProps({ is_focused: true });
240+
// setProps({ is_focused: true });
244241
}
245242

246243
return;

Diff for: ‎packages/dash-table/src/dash-table/components/Table/Table.less

+6
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,12 @@
310310
position: absolute;
311311
left: 0;
312312
top: 0;
313+
&.unfocused::selection {
314+
background-color: transparent;
315+
}
316+
&.unfocused {
317+
caret-color: transparent;
318+
}
313319
}
314320

315321
div.dash-cell-value {

Diff for: ‎packages/dash-table/src/dash-table/derived/cell/eventHandler.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
import { memoizeOneFactory } from 'core/memoizer';
33
import memoizerCache from 'core/memoizerCache';
44
import { ICellFactoryOptions } from 'dash-table/components/Table/props';
5-
import { handleChange, handleClick, handleDoubleClick, handlePaste } from 'dash-table/handlers/cellEvents';
5+
import { handleChange, handleClick, handleDoubleClick, handleOnMouseUp, handlePaste } from 'dash-table/handlers/cellEvents';
66

77
type CacheArgs = [Handler, number, number];
88
type GetterArgs = [HandlerFn, number, number];
@@ -11,6 +11,7 @@ export enum Handler {
1111
Change = 'change',
1212
Click = 'click',
1313
DoubleClick = 'doubleclick',
14+
MouseUp = 'mouseup',
1415
Paste = 'paste'
1516
}
1617

@@ -32,6 +33,7 @@ const getter = (propsFn: () => ICellFactoryOptions): CacheFn => {
3233
[Handler.Change, handleChange.bind(undefined, propsFn)],
3334
[Handler.Click, handleClick.bind(undefined, propsFn)],
3435
[Handler.DoubleClick, handleDoubleClick.bind(undefined, propsFn)],
36+
[Handler.MouseUp, handleOnMouseUp.bind(undefined, propsFn)],
3537
[Handler.Paste, handlePaste.bind(undefined, propsFn)]
3638
]);
3739

Diff for: ‎packages/dash-table/src/dash-table/derived/cell/eventHandlerProps.ts

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ const getter = (propsFn: () => ICellFactoryOptions): CacheFn => {
2222
onChange: derivedHandlers(Handler.Change, rowIndex, columnIndex),
2323
onClick: derivedHandlers(Handler.Click, rowIndex, columnIndex),
2424
onDoubleClick: derivedHandlers(Handler.DoubleClick, rowIndex, columnIndex),
25+
onMouseUp: derivedHandlers(Handler.MouseUp, rowIndex, columnIndex),
2526
onPaste: derivedHandlers(Handler.Paste, rowIndex, columnIndex)
2627
} as ICellHandlerProps;
2728
};

Diff for: ‎packages/dash-table/src/dash-table/handlers/cellEvents.ts

+21-6
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import * as R from 'ramda';
22
import { SelectedCells, ICellFactoryOptions } from 'dash-table/components/Table/props';
3+
import isActive from 'dash-table/derived/cell/isActive';
34

45
function isCellSelected(selectedCells: SelectedCells, idx: number, i: number) {
56
return selectedCells && R.contains([idx, i], selectedCells);
@@ -8,19 +9,15 @@ function isCellSelected(selectedCells: SelectedCells, idx: number, i: number) {
89
export const handleClick = (propsFn: () => ICellFactoryOptions, idx: number, i: number, e: any) => {
910
const {
1011
editable,
11-
is_focused,
1212
selected_cells,
1313
setProps
1414
} = propsFn();
1515

16-
const selected = isCellSelected(selected_cells, idx, i);
17-
1816
if (!editable) {
1917
return;
2018
}
21-
if (!is_focused) {
22-
e.preventDefault();
23-
}
19+
20+
const selected = isCellSelected(selected_cells, idx, i);
2421

2522
// don't update if already selected
2623
if (selected) {
@@ -105,6 +102,24 @@ export const handleChange = (propsFn: () => ICellFactoryOptions, idx: number, i:
105102
setProps({
106103
data: newData
107104
});
105+
106+
};
107+
108+
export const handleOnMouseUp = (propsFn: () => ICellFactoryOptions, idx: number, i: number, e: any) => {
109+
const {
110+
active_cell,
111+
is_focused,
112+
} = propsFn();
113+
114+
const active = isActive(active_cell, idx, i);
115+
116+
if(!is_focused && active) {
117+
e.preventDefault();
118+
// We do this because mouseMove can change the selection, we don't want
119+
// to check for all mouse movements, for performance reasons.
120+
const input = e.target;
121+
input.setSelectionRange(0, input.value ? input.value.length : 0);
122+
}
108123
};
109124

110125
export const handlePaste = (_propsFn: () => ICellFactoryOptions, _idx: number, _i: number, e: ClipboardEvent) => {

Diff for: ‎packages/dash-table/tests/cypress/tests/server/dash_test.ts

+72
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,78 @@ describe('dash basic', () => {
5656
});
5757
});
5858
});
59+
describe('ArrowKeys navigation', () => {
60+
describe('When active, but not focused', () => {
61+
// https://github.com/plotly/dash-table/issues/141
62+
it('can edit last, update data on "arrowleft", and move one cell to the left', () => {
63+
const startingCell = [249,1]
64+
const targetCell = [249, 0]
65+
DashTable.getCell(startingCell[0], startingCell[1]).click();
66+
DOM.focused.then($input => {
67+
const initialValue = $input.val();
68+
69+
DOM.focused.type(`abc${Key.ArrowLeft}`);
70+
71+
cy.get('#container').should($container => {
72+
expect($container.first()[0].innerText).to.equal(`[249][1] = ${initialValue} -> abc`);
73+
});
74+
});
75+
DashTable.getCell(targetCell[0], targetCell[1]).should('have.class', 'focused');
76+
});
77+
78+
// https://github.com/plotly/dash-table/issues/141
79+
it('can edit last, update data on "arrowup", and move one cell up', () => {
80+
const startingCell = [249,0]
81+
const targetCell = [248, 0]
82+
DashTable.getCell(startingCell[0], startingCell[1]).click();
83+
DOM.focused.then($input => {
84+
const initialValue = $input.val();
85+
86+
DOM.focused.type(`abc${Key.ArrowUp}`);
87+
88+
cy.get('#container').should($container => {
89+
expect($container.first()[0].innerText).to.equal(`[249][0] = ${initialValue} -> abc`);
90+
});
91+
});
92+
DashTable.getCell(targetCell[0], targetCell[1]).should('have.class', 'focused');
93+
});
94+
95+
// https://github.com/plotly/dash-table/issues/141
96+
it('can edit last, update data on "arrowright", and move one cell to the right', () => {
97+
const startingCell = [249,0]
98+
const targetCell = [249, 1]
99+
DashTable.getCell(startingCell[0], startingCell[1]).click();
100+
DOM.focused.then($input => {
101+
const initialValue = $input.val();
102+
103+
DOM.focused.type(`abc${Key.ArrowRight}`);
104+
105+
cy.get('#container').should($container => {
106+
expect($container.first()[0].innerText).to.equal(`[249][0] = ${initialValue} -> abc`);
107+
});
108+
});
109+
DashTable.getCell(targetCell[0], targetCell[1]).should('have.class', 'focused');
110+
});
111+
112+
113+
// https://github.com/plotly/dash-table/issues/141
114+
it('can edit last, update data on "arrowdown", and move one cell down', () => {
115+
const startingCell = [249,0]
116+
const targetCell = [249, 1]
117+
DashTable.getCell(startingCell[0], startingCell[1]).click();
118+
DOM.focused.then($input => {
119+
const initialValue = $input.val();
120+
121+
DOM.focused.type(`abc${Key.ArrowRight}`);
122+
123+
cy.get('#container').should($container => {
124+
expect($container.first()[0].innerText).to.equal(`[249][0] = ${initialValue} -> abc`);
125+
});
126+
});
127+
DashTable.getCell(targetCell[0], targetCell[1]).should('have.class', 'focused');
128+
});
129+
})
130+
})
59131

60132
it('can edit last and update data when clicking outside of cell', () => {
61133
DashTable.getCell(249, 0).click();

0 commit comments

Comments
 (0)