3.0 copy paste regression (#48)

Marc-Andre-Rivet · Marc-Andre-Rivet · commit 3bd604d852f1 · 2018-08-22T12:38:38.000-04:00
* fix copy/paste regression, FE-BE props events

* fix copy paste regression - basic testing

* remove console.log

* undo useless changes

* pylint

* pylint

* fix pr comment

* fix pr comments

* pylint

* pylint
diff --git a/packages/dash-table/dash_table/bundle.js b/packages/dash-table/dash_table/bundle.js
diff --git a/packages/dash-table/dash_table/demo.js b/packages/dash-table/dash_table/demo.js
diff --git a/packages/dash-table/dash_table/metadata.json b/packages/dash-table/dash_table/metadata.json
@@ -60,17 +60,6 @@
         "required": false,
         "description": ""
       },
-      "update_on_unfocus": {
-        "type": {
-          "name": "bool"
-        },
-        "required": false,
-        "description": "Only send an update of  `dataframe_previous` and `dataframe_timestamp`\nwhen the cell is unfocused ()",
-        "defaultValue": {
-          "value": "true",
-          "computed": false
-        }
-      },
       "dropdown_properties": {
         "type": {
           "name": "objectOf",
diff --git a/packages/dash-table/dash_table/package.json b/packages/dash-table/dash_table/package.json
@@ -1,15 +1,17 @@
 {
   "name": "dash-table",
-  "version": "3.0.0rc3",
+  "version": "3.0.0rc4",
   "description": "Dash table",
   "main": "build/index.js",
   "scripts": {
     "preprivate::opentests": "run-s private::wait*",
     "preprivate::runtests": "run-s private::wait*",
     "private::build": "webpack --display-reasons --bail",
-    "private::host_dash": "python tests/e2e/dash/v_be_page.py",
+    "private::host_dash8081": "python tests/e2e/dash/v_be_page.py",
+    "private::host_dash8082": "python tests/e2e/dash/v_copy_paste.py",
     "private::host_js": "http-server ./dash_table -c-1 --silent",
-    "private::wait_dash": "wait-on http://localhost:8081",
+    "private::wait_dash8081": "wait-on http://localhost:8081",
+    "private::wait_dash8082": "wait-on http://localhost:8082",
     "private::wait_js": "wait-on http://localhost:8080",
     "private::lint.js": "eslint src",
     "private::lint.ts": "tslint --project . src/**/*.ts",
diff --git a/packages/dash-table/npm-shrinkwrap.json b/packages/dash-table/npm-shrinkwrap.json
diff --git a/packages/dash-table/package.json b/packages/dash-table/package.json
@@ -1,15 +1,17 @@
 {
   "name": "dash-table",
-  "version": "3.0.0rc3",
+  "version": "3.0.0rc4",
   "description": "Dash table",
   "main": "build/index.js",
   "scripts": {
     "preprivate::opentests": "run-s private::wait*",
     "preprivate::runtests": "run-s private::wait*",
     "private::build": "webpack --display-reasons --bail",
-    "private::host_dash": "python tests/e2e/dash/v_be_page.py",
+    "private::host_dash8081": "python tests/e2e/dash/v_be_page.py",
+    "private::host_dash8082": "python tests/e2e/dash/v_copy_paste.py",
     "private::host_js": "http-server ./dash_table -c-1 --silent",
-    "private::wait_dash": "wait-on http://localhost:8081",
+    "private::wait_dash8081": "wait-on http://localhost:8081",
+    "private::wait_dash8082": "wait-on http://localhost:8082",
     "private::wait_js": "wait-on http://localhost:8080",
     "private::lint.js": "eslint src",
     "private::lint.ts": "tslint --project . src/**/*.ts",
diff --git a/packages/dash-table/src/dash-table/components/Cell.tsx b/packages/dash-table/src/dash-table/components/Cell.tsx
@@ -101,13 +101,19 @@ export default class Cell extends Component<IProps, IState> {
             />);
     }
 
+    private onPaste = (e: React.ClipboardEvent<Element>) => {
+        const { onPaste } = this.propsWithDefaults;
+
+        onPaste(e);
+        e.stopPropagation();
+    }
+
     private renderInput() {
         const {
             active,
             focused,
             onClick,
-            onDoubleClick,
-            onPaste
+            onDoubleClick
         } = this.propsWithDefaults;
 
         const classes = [
@@ -129,7 +135,7 @@ export default class Cell extends Component<IProps, IState> {
                 type='text'
                 value={this.state.value}
                 onChange={this.handleChange}
-                onPaste={onPaste}
+                onPaste={this.onPaste}
                 {...attributes}
             />);
     }
diff --git a/packages/dash-table/src/dash-table/components/ControlledTable.js b/packages/dash-table/src/dash-table/components/ControlledTable.js
@@ -509,7 +509,7 @@ export default class ControlledTable extends Component {
                 );
                 setProps({
                     dataframe: newDataframe,
-                    columns: newColumns,
+                    columns: newColumns
                 });
             }
         }
diff --git a/packages/dash-table/src/dash-table/components/Table.js b/packages/dash-table/src/dash-table/components/Table.js
@@ -55,32 +55,12 @@ export default class Table extends Component {
             <ControlledTable
                 {...R.merge(this.props, {
                     virtualizer: this.virtualizer,
-
                     setProps: newProps => {
+                        if (R.has('dataframe', newProps)) {
+                            const { dataframe } = this.props;
 
-                        // !is_focused -> is_focused: save the current dataframe
-                        if (newProps.is_focused && !this.props.is_focused) {
-                            console.warn('Saving dataframe', this.props.dataframe);
-                            this.dataframe_previous = this.props.dataframe;
-                        }
-
-                        // unfocused -> send the old dataframe and the update time
-                        if (!newProps.is_focused && this.props.is_focused &&
-                            this.props.update_on_unfocus) {
-                            console.warn('Updating timestamp');
-                            newProps.dataframe_timestamp = Date.now();
-                            newProps.dataframe_previous = this.dataframe_previous;
-
-                        // table is unfocused but user copied and pasted data
-                        } else if (!this.props.is_focused &&
-                                   R.has('dataframe', newProps)) {
-                            newProps.dataframe_previous = this.props.dataframe;
-                            newProps.dataframe_timestamp = Date.now();
-                        // user wants the new dataframe on every letter press
-                        } else if (!this.props.update_on_unfocus &&
-                                   R.has('dataframe', newProps)) {
-                            newProps.dataframe_previous = this.dataframe_previous;
                             newProps.dataframe_timestamp = Date.now();
+                            newProps.dataframe_previous = dataframe;
                         }
 
                         this.props.setProps(newProps);
@@ -154,8 +134,7 @@ export const defaultProps = {
         th: {},
 
         td: {},
-    },
-    update_on_unfocus: true
+    }
 };
 
 export const propTypes = {
@@ -165,11 +144,6 @@ export const propTypes = {
     dataframe: PropTypes.arrayOf(PropTypes.object),
     dataframe_previous: PropTypes.arrayOf(PropTypes.object),
     dataframe_timestamp: PropTypes.any,
-    /**
-     * Only send an update of  `dataframe_previous` and `dataframe_timestamp`
-     * when the cell is unfocused ()
-     */
-    update_on_unfocus: PropTypes.bool,
 
     dropdown_properties: PropTypes.objectOf(
         PropTypes.arrayOf(PropTypes.shape({
diff --git a/packages/dash-table/tests/e2e/cypress/integration/copy_paste_test.ts b/packages/dash-table/tests/e2e/cypress/integration/copy_paste_test.ts
@@ -0,0 +1,35 @@
+import DashTable from 'cypress/DashTable';
+import DOM from 'cypress/DOM';
+import Key from 'cypress/Key';
+import Resolve from 'cypress/Resolve';
+
+describe('copy paste', () => {
+    beforeEach(() => {
+        cy.visit('http://localhost:8082');
+    });
+
+    it('can do BE roundtrip on cell modification', () => {
+        DashTable.getCell(0, 0).click();
+        DOM.focused.type(`1${Key.Enter}`);
+
+        DashTable
+            .getCell(0, 0)
+            .within(() => cy.get('.cell-value').should('have.html', '10'));
+
+        DashTable.getCell(0, 1)
+            .within(() => cy.get('.cell-value').should('have.html', 'MODIFIED'));
+    });
+
+    it('can do BE roundtrip on copy-paste', async () => {
+        await Resolve(DashTable.getCell(0, 0).click());
+        await Resolve(DOM.focused.type(`${Key.Control}c`));
+
+        await Resolve(DashTable.getCell(1, 0).click());
+        await Resolve(DOM.focused.type(`${Key.Control}v`));
+
+        DashTable
+            .getCell(1, 1)
+            .within(() => cy.get('.cell-value').should('have.html', 'MODIFIED'));
+
+    });
+});
diff --git a/packages/dash-table/tests/e2e/cypress/integration/dash_test.ts b/packages/dash-table/tests/e2e/cypress/integration/dash_test.ts
@@ -1,15 +1,15 @@
 import DashTable from 'cypress/DashTable';
 
-describe('dash_test', () => {
+describe('dash basic', () => {
     beforeEach(() => {
         cy.visit('http://localhost:8081');
     });
 
     it('can get cell', () => {
         DashTable.getCell(0, 0).click();
-        DashTable.getCellInput(0, 0).should('have.value', '0');
+        DashTable.getCell(0, 0).within(() => cy.get('input').should('have.value', '0'));
 
         cy.get('button.next-page').click();
-        DashTable.getCellInput(0, 0).should('have.value', '250');
+        DashTable.getCell(0, 0).within(() => cy.get('input').should('have.value', '250'));
     });
-});
+});
diff --git a/packages/dash-table/tests/e2e/cypress/integration/navigation_test.ts b/packages/dash-table/tests/e2e/cypress/integration/navigation_test.ts
@@ -27,15 +27,15 @@ describe('navigate', () => {
         describe('from a focused cell input', () => {
             beforeEach(() => {
                 DOM.focused.type(Key.Enter);
-                DashTable.getCellInput(3, 3).should('have.class', 'focused');
+                DashTable.getCell(3, 3).within(() => cy.get('.cell-value').should('have.class', 'focused'));
             });
 
             it('does not focus on next cell input on "enter"', () => {
                 DOM.focused.type(Key.Enter);
-                DashTable.getCell(3, 3).should('not.have.class', 'focused');
+                DashTable.getCell(3, 3).within(() => cy.get('.cell-value').should('not.have.class', 'focused'));
 
                 DashTable.getCell(4, 3).should('have.class', 'focused');
-                DashTable.getCellInput(4, 3).should('not.have.class', 'focused');
+                DashTable.getCell(4, 3).within(() => cy.get('.cell-value').should('not.have.class', 'focused'));
             });
 
             it('does not focus on next cell input on "tab"', async () => {
@@ -44,7 +44,7 @@ describe('navigate', () => {
                 DashTable.getCell(3, 3).should('not.have.class', 'focused');
 
                 DashTable.getCell(3, 4).should('have.class', 'focused');
-                DashTable.getCellInput(3, 4).should('not.have.class', 'focused');
+                DashTable.getCell(3, 4).within(() => cy.get('.cell-value').should('not.have.class', 'focused'));
             });
         });
 
diff --git a/packages/dash-table/tests/e2e/cypress/plugins/index.js b/packages/dash-table/tests/e2e/cypress/plugins/index.js
@@ -4,5 +4,6 @@ module.exports = (on) => {
     const options = {
         webpackOptions: require('../../../../webpack.config'),
     }
+
     on('file:preprocessor', wp(options))
 }
diff --git a/packages/dash-table/tests/e2e/cypress/src/DashTable.ts b/packages/dash-table/tests/e2e/cypress/src/DashTable.ts
@@ -3,10 +3,6 @@ export default class DashTable {
         return cy.get(`#table tbody tr td.column-${column}`).eq(row);
     }
 
-    static getCellInput(row: number, column: number) {
-        return DashTable.getCell(row, column).get('input');
-    }
-
     static getSelectedCells() {
         return cy.get(`#table tbody td.cell--selected`);
     }
diff --git a/packages/dash-table/tests/e2e/cypress/src/Key.ts b/packages/dash-table/tests/e2e/cypress/src/Key.ts
@@ -3,6 +3,7 @@ enum Key {
     ArrowLeft = '{leftarrow}',
     ArrowRight = '{rightarrow}',
     ArrowUp = '{uparrow}',
+    Control = '{ctrl}',
     Enter = '{enter}',
     Escape = '{esc}',
     Shift = '{shift}'
diff --git a/packages/dash-table/tests/e2e/dash/v_copy_paste.py b/packages/dash-table/tests/e2e/dash/v_copy_paste.py
@@ -0,0 +1,88 @@
+# pylint: disable=global-statement
+import dash
+from dash.dependencies import Input, Output, State
+import dash_html_components as html
+import os
+import pandas as pd
+import sys
+
+sys.path.append(
+    os.path.abspath(
+        os.path.join(
+            os.path.dirname(sys.argv[0]),
+            os.pardir,
+            os.pardir,
+            os.pardir
+        )
+    )
+)
+module_names = ['dash_table']
+modules = [__import__(x) for x in module_names]
+dash_table = modules[0]
+
+url = (
+    'https://github.com/plotly/datasets/raw/master/'
+    '26k-consumer-complaints.csv'
+)
+df = pd.read_csv(url)
+df = df.values
+
+app = dash.Dash()
+app.css.config.serve_locally = True
+app.scripts.config.serve_locally = True
+
+app.layout = html.Div([
+    html.Div(
+        id='container',
+        children='Hello World'
+    ),
+    dash_table.Table(
+        id='table',
+        dataframe=df[0:250],
+        navigation='page',
+        columns=[
+            {'id': 0, 'name': 'Complaint ID'},
+            {'id': 1, 'name': 'Product'},
+            {'id': 2, 'name': 'Sub-product'},
+            {'id': 3, 'name': 'Issue'},
+            {'id': 4, 'name': 'Sub-issue'},
+            {'id': 5, 'name': 'State'},
+            {'id': 6, 'name': 'ZIP'},
+            {'id': 7, 'name': 'code'},
+            {'id': 8, 'name': 'Date received'},
+            {'id': 9, 'name': 'Date sent to company'},
+            {'id': 10, 'name': 'Company'},
+            {'id': 11, 'name': 'Company response'},
+            {'id': 12, 'name': 'Timely response?'},
+            {'id': 13, 'name': 'Consumer disputed?'}
+        ],
+        editable=True
+    )
+])
+
+
+@app.callback(
+    Output('table', 'dataframe'),
+    [Input('table', 'dataframe_timestamp')],
+    [
+        State('table', 'dataframe'),
+        State('table', 'dataframe_previous')
+    ]
+)
+# pylint: disable=unused-argument
+def updateDataframe(timestamp, current, previous):
+    # pylint: enable=unused-argument
+    if current is None or previous is None:
+        return current
+
+    if len(current) == len(previous):
+        for (i, datum) in enumerate(current):
+            previous_datum = previous[i]
+            if datum[0] != previous_datum[0]:
+                datum[1] = 'MODIFIED'
+
+    return current
+
+
+if __name__ == '__main__':
+    app.run_server(port=8082, debug=False)

Original file line number	Diff line number	Diff line change
`@@ -509,7 +509,7 @@ export default class ControlledTable extends Component {`
`509`	`509`	`);`
`510`	`510`	`setProps({`
`511`	`511`	`dataframe: newDataframe,`
`512`		`- columns: newColumns,`
	`512`	`+ columns: newColumns`
`513`	`513`	`});`
`514`	`514`	`}`
`515`	`515`	`}`