diff --git a/demo/App.js b/demo/App.js deleted file mode 100644 index 6c189180d..000000000 --- a/demo/App.js +++ /dev/null @@ -1,75 +0,0 @@ -/* eslint no-magic-numbers: 0 */ -import * as R from 'ramda'; -import React, {Component} from 'react'; -import { DataTable } from 'dash-table'; -import Environment from 'core/environment'; -import { memoizeOne } from 'core/memoizer'; -import Logger from 'core/Logger'; -import AppState, { AppMode } from './AppMode'; - -import './style.less'; - -class App extends Component { - constructor() { - super(); - - this.state = AppState; - this.state.temp_filtering = ''; - - const setProps = memoizeOne(() => { - return newProps => { - Logger.debug('--->', newProps); - this.setState(prevState => ({ - tableProps: R.merge(prevState.tableProps, newProps) - })); - }; - }); - - Object.defineProperty(this, 'setProps', { - get: () => setProps() - }); - } - - renderMode() { - const mode = Environment.searchParams.get('mode'); - - if (mode === AppMode.Filtering) { - return ( -
- - this.setState({ temp_filtering: e.target.value }) - } - onBlur={e => { - const tableProps = R.clone(this.state.tableProps); - tableProps.filter_query = e.target.value; - - this.setState({ tableProps }); - }} /> -
); - } - } - - render() { - return (
- {this.renderMode()} - -
); - } -} - -export default App; diff --git a/demo/App.tsx b/demo/App.tsx new file mode 100644 index 000000000..160d0a341 --- /dev/null +++ b/demo/App.tsx @@ -0,0 +1,84 @@ +/* eslint no-magic-numbers: 0 */ +import * as R from 'ramda'; +import React, {Component} from 'react'; +import { DataTable } from 'dash-table/index'; +import Environment from 'core/environment'; +import { memoizeOne } from 'core/memoizer'; +import Logger from 'core/Logger'; +import AppState, { AppMode } from './AppMode'; +import memoizerCache from 'core/cache/memoizer'; + +import './style.less'; + +class App extends Component { + constructor(props: any) { + super(props); + + this.state = { + ...AppState, + temp_filtering: '' + }; + } + + renderMode() { + const mode = Environment.searchParams.get('mode'); + + if (mode === AppMode.Filtering) { + return (
+ + this.setState({ temp_filtering: e.target.value }) + } + onBlur={e => { + const tableProps = R.clone(this.state.tableProps); + tableProps.filter_query = e.target.value; + + this.setState({ tableProps }); + }} /> +
); + } else if (mode === AppMode.TaleOfTwoTables) { + const props: any = {}; + Object.entries(this.state.tableProps).forEach(([key, value]) => { + props[key] = this.propCache.get(key)(value); + }); + + return (); + } + } + + render() { + return (
+ {this.renderMode()} + +
); + } + + private propCache = memoizerCache<[string]>()(R.clone); + + private setProps = memoizeOne(() => { + return (newProps: any) => { + Logger.debug('--->', newProps); + this.setState((prevState: any) => ({ + tableProps: R.merge(prevState.tableProps, newProps) + })); + }; + }); +} + +export default App; diff --git a/demo/AppMode.ts b/demo/AppMode.ts index 97aec6b10..06f3fa470 100644 --- a/demo/AppMode.ts +++ b/demo/AppMode.ts @@ -22,6 +22,7 @@ export enum AppMode { Formatting = 'formatting', ReadOnly = 'readonly', ColumnsInSpace = 'columnsInSpace', + TaleOfTwoTables = 'taleOfTwoTables', Tooltips = 'tooltips', Typed = 'typed', Virtualized = 'virtualized' @@ -335,6 +336,7 @@ function getState() { return getVirtualizedState(); case AppMode.Typed: return getTypedState(); + case AppMode.TaleOfTwoTables: case AppMode.Default: default: return getDefaultState(); diff --git a/src/dash-table/dash/DataTable.js b/src/dash-table/dash/DataTable.js index c95266b2e..8f986558b 100644 --- a/src/dash-table/dash/DataTable.js +++ b/src/dash-table/dash/DataTable.js @@ -8,13 +8,14 @@ import Logger from 'core/Logger'; import genRandomId from 'dash-table/utils/generate'; import isValidProps from './validate'; -import sanitizeProps from './sanitize'; +import Sanitizer from './Sanitizer'; export default class DataTable extends Component { constructor(props) { super(props); let id; this.getId = () => (id = id || genRandomId('table-')); + this.sanitizer = new Sanitizer(); } render() { @@ -22,7 +23,7 @@ export default class DataTable extends Component { return (
Invalid props combination
); } - const sanitizedProps = sanitizeProps(this.props); + const sanitizedProps = this.sanitizer.sanitize(this.props); return this.props.id ? () : (); diff --git a/src/dash-table/dash/sanitize.ts b/src/dash-table/dash/Sanitizer.ts similarity index 53% rename from src/dash-table/dash/sanitize.ts rename to src/dash-table/dash/Sanitizer.ts index 0e96a2444..ea35b471d 100644 --- a/src/dash-table/dash/sanitize.ts +++ b/src/dash-table/dash/Sanitizer.ts @@ -29,23 +29,6 @@ const D3_DEFAULT_LOCALE: INumberLocale = { const DEFAULT_NULLY = ''; const DEFAULT_SPECIFIER = ''; -const applyDefaultToLocale = memoizeOne((locale: INumberLocale) => getLocale(locale)); - -const applyDefaultsToColumns = memoizeOne( - (defaultLocale: INumberLocale, defaultSort: SortAsNull, columns: Columns, editable: boolean ) => R.map(column => { - const c = R.clone(column); - c.editable = isEditable(editable, column.editable); - c.sort_as_null = c.sort_as_null || defaultSort; - - if (c.type === ColumnType.Numeric && c.format) { - c.format.locale = getLocale(defaultLocale, c.format.locale); - c.format.nully = getNully(c.format.nully); - c.format.specifier = getSpecifier(c.format.specifier); - } - return c; - }, columns) -); - const data2number = (data?: any) => +data || 0; const getFixedColumns = ( @@ -64,16 +47,37 @@ const getFixedRows = ( 0 : headerRows(columns) + (filter_action !== TableAction.None ? 1 : 0) + data2number(fixed.data); -export default (props: PropsWithDefaults): SanitizedProps => { - const locale_format = applyDefaultToLocale(props.locale_format); +const applyDefaultsToColumns = (defaultLocale: INumberLocale, defaultSort: SortAsNull, columns: Columns, editable: boolean) => R.map(column => { + const c = R.clone(column); + c.editable = isEditable(editable, column.editable); + c.sort_as_null = c.sort_as_null || defaultSort; - return R.merge(props, { - columns: applyDefaultsToColumns(locale_format, props.sort_as_null, props.columns, props.editable), - fixed_columns: getFixedColumns(props.fixed_columns, props.row_deletable, props.row_selectable), - fixed_rows: getFixedRows(props.fixed_rows, props.columns, props.filter_action), - locale_format - }); -}; + if (c.type === ColumnType.Numeric && c.format) { + c.format.locale = getLocale(defaultLocale, c.format.locale); + c.format.nully = getNully(c.format.nully); + c.format.specifier = getSpecifier(c.format.specifier); + } + return c; +}, columns); + +const applyDefaultToLocale = (locale: INumberLocale) => getLocale(locale); + +export default class Sanitizer { + sanitize(props: PropsWithDefaults): SanitizedProps { + const locale_format = this.applyDefaultToLocale(props.locale_format); + + return R.merge(props, { + columns: this.applyDefaultsToColumns(locale_format, props.sort_as_null, props.columns, props.editable), + fixed_columns: getFixedColumns(props.fixed_columns, props.row_deletable, props.row_selectable), + fixed_rows: getFixedRows(props.fixed_rows, props.columns, props.filter_action), + locale_format + }); + } + + private readonly applyDefaultToLocale = memoizeOne(applyDefaultToLocale); + + private readonly applyDefaultsToColumns = memoizeOne(applyDefaultsToColumns); +} export const getLocale = (...locales: Partial[]): INumberLocale => R.mergeAll([ diff --git a/tests/cypress/tests/standalone/edit_cell_test.ts b/tests/cypress/tests/standalone/edit_cell_test.ts index 152063088..7f298f452 100644 --- a/tests/cypress/tests/standalone/edit_cell_test.ts +++ b/tests/cypress/tests/standalone/edit_cell_test.ts @@ -4,6 +4,43 @@ import Key from 'cypress/Key'; import { AppMode, ReadWriteModes } from 'demo/AppMode'; +Object.values([ + ...ReadWriteModes, + AppMode.TaleOfTwoTables +]).forEach(mode => { + describe(`edit, mode=${mode}`, () => { + beforeEach(() => { + cy.visit(`http://localhost:8080?mode=${mode}`); + DashTable.toggleScroll(false); + }); + + // Case: "Pressing enter to confirm change does not work on the last row" + // Issue: https://github.com/plotly/dash-table/issues/50 + it('can edit on "enter"', () => { + DashTable.getCell(0, 1).click(); + // Case: 2+ tables results in infinite rendering loop b/c of shared cache + // Issue: https://github.com/plotly/dash-table/pull/468 + // + // Artificial delay added to make sure re-rendering has time to occur. + cy.wait(1000); + DOM.focused.type(`abc${Key.Enter}`); + DashTable.getCell(0, 1).within(() => cy.get('.dash-cell-value').should('have.html', `abc`)); + }); + + it('can edit when clicking outside of cell', () => { + DashTable.getCell(0, 1).click(); + DOM.focused.type(`abc`); + // Case: 2+ tables results in infinite rendering loop b/c of shared cache + // Issue: https://github.com/plotly/dash-table/pull/468 + // + // Artificial delay added to make sure re-rendering has time to occur. + cy.wait(1000); + DashTable.getCell(0, 0).click(); + DashTable.getCell(0, 1).within(() => cy.get('.dash-cell-value').should('have.html', `abc`)); + }); + }); +}); + Object.values(ReadWriteModes).forEach(mode => { describe(`edit, mode=${mode}`, () => { beforeEach(() => { @@ -106,20 +143,6 @@ Object.values(ReadWriteModes).forEach(mode => { cy.get('.Select-value-label').should('have.html', expectedValue); }); }); - - // https://github.com/plotly/dash-table/issues/50 - it('can edit on "enter"', () => { - DashTable.getCell(0, 1).click(); - DOM.focused.type(`abc${Key.Enter}`); - DashTable.getCell(0, 1).within(() => cy.get('.dash-cell-value').should('have.html', `abc`)); - }); - - it('can edit when clicking outside of cell', () => { - DashTable.getCell(0, 1).click(); - DOM.focused.type(`abc`); - DashTable.getCell(0, 0).click(); - DashTable.getCell(0, 1).within(() => cy.get('.dash-cell-value').should('have.html', `abc`)); - }); }); }); diff --git a/tests/cypress/tests/unit/formatting_test.ts b/tests/cypress/tests/unit/formatting_test.ts index 3e76d6fc0..e0262fd9b 100644 --- a/tests/cypress/tests/unit/formatting_test.ts +++ b/tests/cypress/tests/unit/formatting_test.ts @@ -1,5 +1,5 @@ import { getFormatter } from 'dash-table/type/number'; -import { getLocale, getNully, getSpecifier } from 'dash-table/dash/sanitize'; +import { getLocale, getNully, getSpecifier } from 'dash-table/dash/Sanitizer'; describe('formatting', () => { describe('number', () => {