Skip to content

Commit 97cc370

Browse files
Marc-Andre-RivetMarc-Andre-Rivet
committed
Regression fix multi table typing (#468)
1 parent 5065d88 commit 97cc370

File tree

7 files changed

+157
-118
lines changed

7 files changed

+157
-118
lines changed

‎packages/dash-table/demo/App.js

Lines changed: 0 additions & 75 deletions
This file was deleted.

‎packages/dash-table/demo/App.tsx

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
/* eslint no-magic-numbers: 0 */
2+
import * as R from 'ramda';
3+
import React, {Component} from 'react';
4+
import { DataTable } from 'dash-table/index';
5+
import Environment from 'core/environment';
6+
import { memoizeOne } from 'core/memoizer';
7+
import Logger from 'core/Logger';
8+
import AppState, { AppMode } from './AppMode';
9+
import memoizerCache from 'core/cache/memoizer';
10+
11+
import './style.less';
12+
13+
class App extends Component<any, any> {
14+
constructor(props: any) {
15+
super(props);
16+
17+
this.state = {
18+
...AppState,
19+
temp_filtering: ''
20+
};
21+
}
22+
23+
renderMode() {
24+
const mode = Environment.searchParams.get('mode');
25+
26+
if (mode === AppMode.Filtering) {
27+
return (<div>
28+
<button
29+
className='clear-filters'
30+
onClick={() => {
31+
const tableProps = R.clone(this.state.tableProps);
32+
tableProps.filter_query = '';
33+
34+
this.setState({ tableProps });
35+
}}
36+
>Clear Filter</button>
37+
<input
38+
style={{ width: '500px' }}
39+
value={this.state.temp_filtering}
40+
onChange={
41+
e => this.setState({ temp_filtering: e.target.value })
42+
}
43+
onBlur={e => {
44+
const tableProps = R.clone(this.state.tableProps);
45+
tableProps.filter_query = e.target.value;
46+
47+
this.setState({ tableProps });
48+
}} />
49+
</div>);
50+
} else if (mode === AppMode.TaleOfTwoTables) {
51+
const props: any = {};
52+
Object.entries(this.state.tableProps).forEach(([key, value]) => {
53+
props[key] = this.propCache.get(key)(value);
54+
});
55+
56+
return (<DataTable
57+
{...props}
58+
/>);
59+
}
60+
}
61+
62+
render() {
63+
return (<div>
64+
{this.renderMode()}
65+
<DataTable
66+
setProps={this.setProps()}
67+
{...this.state.tableProps}
68+
/>
69+
</div>);
70+
}
71+
72+
private propCache = memoizerCache<[string]>()(R.clone);
73+
74+
private setProps = memoizeOne(() => {
75+
return (newProps: any) => {
76+
Logger.debug('--->', newProps);
77+
this.setState((prevState: any) => ({
78+
tableProps: R.merge(prevState.tableProps, newProps)
79+
}));
80+
};
81+
});
82+
}
83+
84+
export default App;

‎packages/dash-table/demo/AppMode.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ export enum AppMode {
2222
Formatting = 'formatting',
2323
ReadOnly = 'readonly',
2424
ColumnsInSpace = 'columnsInSpace',
25+
TaleOfTwoTables = 'taleOfTwoTables',
2526
Tooltips = 'tooltips',
2627
Typed = 'typed',
2728
Virtualized = 'virtualized'
@@ -335,6 +336,7 @@ function getState() {
335336
return getVirtualizedState();
336337
case AppMode.Typed:
337338
return getTypedState();
339+
case AppMode.TaleOfTwoTables:
338340
case AppMode.Default:
339341
default:
340342
return getDefaultState();

‎packages/dash-table/src/dash-table/dash/DataTable.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,21 +8,22 @@ import Logger from 'core/Logger';
88

99
import genRandomId from 'dash-table/utils/generate';
1010
import isValidProps from './validate';
11-
import sanitizeProps from './sanitize';
11+
import Sanitizer from './Sanitizer';
1212

1313
export default class DataTable extends Component {
1414
constructor(props) {
1515
super(props);
1616
let id;
1717
this.getId = () => (id = id || genRandomId('table-'));
18+
this.sanitizer = new Sanitizer();
1819
}
1920

2021
render() {
2122
if (!isValidProps(this.props)) {
2223
return (<div>Invalid props combination</div>);
2324
}
2425

25-
const sanitizedProps = sanitizeProps(this.props);
26+
const sanitizedProps = this.sanitizer.sanitize(this.props);
2627
return this.props.id ?
2728
(<RealTable {...sanitizedProps} />) :
2829
(<RealTable {...sanitizedProps} id={this.getId()} />);

‎packages/dash-table/src/dash-table/dash/sanitize.ts renamed to ‎packages/dash-table/src/dash-table/dash/Sanitizer.ts

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -29,23 +29,6 @@ const D3_DEFAULT_LOCALE: INumberLocale = {
2929
const DEFAULT_NULLY = '';
3030
const DEFAULT_SPECIFIER = '';
3131

32-
const applyDefaultToLocale = memoizeOne((locale: INumberLocale) => getLocale(locale));
33-
34-
const applyDefaultsToColumns = memoizeOne(
35-
(defaultLocale: INumberLocale, defaultSort: SortAsNull, columns: Columns, editable: boolean ) => R.map(column => {
36-
const c = R.clone(column);
37-
c.editable = isEditable(editable, column.editable);
38-
c.sort_as_null = c.sort_as_null || defaultSort;
39-
40-
if (c.type === ColumnType.Numeric && c.format) {
41-
c.format.locale = getLocale(defaultLocale, c.format.locale);
42-
c.format.nully = getNully(c.format.nully);
43-
c.format.specifier = getSpecifier(c.format.specifier);
44-
}
45-
return c;
46-
}, columns)
47-
);
48-
4932
const data2number = (data?: any) => +data || 0;
5033

5134
const getFixedColumns = (
@@ -64,16 +47,37 @@ const getFixedRows = (
6447
0 :
6548
headerRows(columns) + (filter_action !== TableAction.None ? 1 : 0) + data2number(fixed.data);
6649

67-
export default (props: PropsWithDefaults): SanitizedProps => {
68-
const locale_format = applyDefaultToLocale(props.locale_format);
50+
const applyDefaultsToColumns = (defaultLocale: INumberLocale, defaultSort: SortAsNull, columns: Columns, editable: boolean) => R.map(column => {
51+
const c = R.clone(column);
52+
c.editable = isEditable(editable, column.editable);
53+
c.sort_as_null = c.sort_as_null || defaultSort;
6954

70-
return R.merge(props, {
71-
columns: applyDefaultsToColumns(locale_format, props.sort_as_null, props.columns, props.editable),
72-
fixed_columns: getFixedColumns(props.fixed_columns, props.row_deletable, props.row_selectable),
73-
fixed_rows: getFixedRows(props.fixed_rows, props.columns, props.filter_action),
74-
locale_format
75-
});
76-
};
55+
if (c.type === ColumnType.Numeric && c.format) {
56+
c.format.locale = getLocale(defaultLocale, c.format.locale);
57+
c.format.nully = getNully(c.format.nully);
58+
c.format.specifier = getSpecifier(c.format.specifier);
59+
}
60+
return c;
61+
}, columns);
62+
63+
const applyDefaultToLocale = (locale: INumberLocale) => getLocale(locale);
64+
65+
export default class Sanitizer {
66+
sanitize(props: PropsWithDefaults): SanitizedProps {
67+
const locale_format = this.applyDefaultToLocale(props.locale_format);
68+
69+
return R.merge(props, {
70+
columns: this.applyDefaultsToColumns(locale_format, props.sort_as_null, props.columns, props.editable),
71+
fixed_columns: getFixedColumns(props.fixed_columns, props.row_deletable, props.row_selectable),
72+
fixed_rows: getFixedRows(props.fixed_rows, props.columns, props.filter_action),
73+
locale_format
74+
});
75+
}
76+
77+
private readonly applyDefaultToLocale = memoizeOne(applyDefaultToLocale);
78+
79+
private readonly applyDefaultsToColumns = memoizeOne(applyDefaultsToColumns);
80+
}
7781

7882
export const getLocale = (...locales: Partial<INumberLocale>[]): INumberLocale =>
7983
R.mergeAll([

‎packages/dash-table/tests/cypress/tests/standalone/edit_cell_test.ts

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,43 @@ import Key from 'cypress/Key';
44

55
import { AppMode, ReadWriteModes } from 'demo/AppMode';
66

7+
Object.values([
8+
...ReadWriteModes,
9+
AppMode.TaleOfTwoTables
10+
]).forEach(mode => {
11+
describe(`edit, mode=${mode}`, () => {
12+
beforeEach(() => {
13+
cy.visit(`http://localhost:8080?mode=${mode}`);
14+
DashTable.toggleScroll(false);
15+
});
16+
17+
// Case: "Pressing enter to confirm change does not work on the last row"
18+
// Issue: https://github.com/plotly/dash-table/issues/50
19+
it('can edit on "enter"', () => {
20+
DashTable.getCell(0, 1).click();
21+
// Case: 2+ tables results in infinite rendering loop b/c of shared cache
22+
// Issue: https://github.com/plotly/dash-table/pull/468
23+
//
24+
// Artificial delay added to make sure re-rendering has time to occur.
25+
cy.wait(1000);
26+
DOM.focused.type(`abc${Key.Enter}`);
27+
DashTable.getCell(0, 1).within(() => cy.get('.dash-cell-value').should('have.html', `abc`));
28+
});
29+
30+
it('can edit when clicking outside of cell', () => {
31+
DashTable.getCell(0, 1).click();
32+
DOM.focused.type(`abc`);
33+
// Case: 2+ tables results in infinite rendering loop b/c of shared cache
34+
// Issue: https://github.com/plotly/dash-table/pull/468
35+
//
36+
// Artificial delay added to make sure re-rendering has time to occur.
37+
cy.wait(1000);
38+
DashTable.getCell(0, 0).click();
39+
DashTable.getCell(0, 1).within(() => cy.get('.dash-cell-value').should('have.html', `abc`));
40+
});
41+
});
42+
});
43+
744
Object.values(ReadWriteModes).forEach(mode => {
845
describe(`edit, mode=${mode}`, () => {
946
beforeEach(() => {
@@ -106,20 +143,6 @@ Object.values(ReadWriteModes).forEach(mode => {
106143
cy.get('.Select-value-label').should('have.html', expectedValue);
107144
});
108145
});
109-
110-
// https://github.com/plotly/dash-table/issues/50
111-
it('can edit on "enter"', () => {
112-
DashTable.getCell(0, 1).click();
113-
DOM.focused.type(`abc${Key.Enter}`);
114-
DashTable.getCell(0, 1).within(() => cy.get('.dash-cell-value').should('have.html', `abc`));
115-
});
116-
117-
it('can edit when clicking outside of cell', () => {
118-
DashTable.getCell(0, 1).click();
119-
DOM.focused.type(`abc`);
120-
DashTable.getCell(0, 0).click();
121-
DashTable.getCell(0, 1).within(() => cy.get('.dash-cell-value').should('have.html', `abc`));
122-
});
123146
});
124147
});
125148

‎packages/dash-table/tests/cypress/tests/unit/formatting_test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { getFormatter } from 'dash-table/type/number';
2-
import { getLocale, getNully, getSpecifier } from 'dash-table/dash/sanitize';
2+
import { getLocale, getNully, getSpecifier } from 'dash-table/dash/Sanitizer';
33

44
describe('formatting', () => {
55
describe('number', () => {

0 commit comments

Comments
 (0)