From 1ed5ab32de0d4b4678fcf42f4724776e11c85828 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andre=CC=81=20Rivet?= Date: Tue, 8 Oct 2019 23:24:56 -0400 Subject: [PATCH 1/2] - hidden columns on copy/paste - new clipboard tests --- .../components/ControlledTable/index.tsx | 2 + src/dash-table/utils/TableClipboardHelper.ts | 2 + src/dash-table/utils/applyClipboardToData.ts | 27 +++-- tests/cypress/tests/unit/clipboard_test.ts | 101 +++++++++++++++--- 4 files changed, 109 insertions(+), 23 deletions(-) diff --git a/src/dash-table/components/ControlledTable/index.tsx b/src/dash-table/components/ControlledTable/index.tsx index e2634bcab..da46d47d3 100644 --- a/src/dash-table/components/ControlledTable/index.tsx +++ b/src/dash-table/components/ControlledTable/index.tsx @@ -587,6 +587,7 @@ export default class ControlledTable extends PureComponent onPaste = (e: any) => { const { active_cell, + columns, data, editable, filter_query, @@ -606,6 +607,7 @@ export default class ControlledTable extends PureComponent e, active_cell, viewport.indices, + columns, visibleColumns, data, true, diff --git a/src/dash-table/utils/TableClipboardHelper.ts b/src/dash-table/utils/TableClipboardHelper.ts index 91d02c58e..5cbaca587 100644 --- a/src/dash-table/utils/TableClipboardHelper.ts +++ b/src/dash-table/utils/TableClipboardHelper.ts @@ -47,6 +47,7 @@ export default class TableClipboardHelper { activeCell: ICellCoordinates, derived_viewport_indices: number[], columns: Columns, + visibleColumns: Columns, data: Data, overflowColumns: boolean = true, overflowRows: boolean = true, @@ -68,6 +69,7 @@ export default class TableClipboardHelper { activeCell, derived_viewport_indices, columns, + visibleColumns, data, overflowColumns, overflowRows diff --git a/src/dash-table/utils/applyClipboardToData.ts b/src/dash-table/utils/applyClipboardToData.ts index 6a27a0b19..c06ee9e95 100644 --- a/src/dash-table/utils/applyClipboardToData.ts +++ b/src/dash-table/utils/applyClipboardToData.ts @@ -9,7 +9,8 @@ export default ( values: any[][], activeCell: ICellCoordinates, derived_viewport_indices: number[], - columns: Columns, + columns_: Columns, + visibleColumns: Columns, data: Data, overflowColumns: boolean = true, overflowRows: boolean = true @@ -23,16 +24,20 @@ export default ( } // don't modify the data and columns directly -- we may abort the paste + // Individual rows will be modified, needs to be a deep clone let newData = R.clone(data); - const newColumns = R.clone(columns); + // Might add columns, not modifying the columns themselves, shallow clone is sufficient + let newColumns = columns_.slice(0); + let newVisibleColumns = visibleColumns.slice(0); - if (overflowColumns && values[0].length + (activeCell as any).column >= columns.length) { + if (overflowColumns && values[0].length + (activeCell as any).column >= visibleColumns.length) { + const _newColumns = []; for ( - let i = columns.length; + let i = visibleColumns.length; i < values[0].length + (activeCell as any).column; i++ ) { - newColumns.push({ + _newColumns.push({ id: `Column ${i + 1}`, name: `Column ${i + 1}`, type: ColumnType.Any, @@ -40,12 +45,20 @@ export default ( } as any); newData.forEach(row => (row[`Column ${i}`] = '')); } + + newColumns = R.insertAll( + R.indexOf(R.last(visibleColumns), columns_) + 1, + _newColumns, + newColumns + ); + + newVisibleColumns = R.concat(newVisibleColumns, _newColumns); } const realActiveRow = derived_viewport_indices[(activeCell as any).row]; if (overflowRows && values.length + realActiveRow >= data.length) { const emptyRow: any = {}; - columns.forEach(c => (emptyRow[c.id] = '')); + visibleColumns.forEach(c => (emptyRow[c.id] = '')); newData = R.concat( newData, R.repeat( @@ -73,7 +86,7 @@ export default ( } const jOffset = (activeCell as any).column + j; - const col = newColumns[jOffset]; + const col = newVisibleColumns[jOffset]; if (!col || !col.editable) { continue; } diff --git a/tests/cypress/tests/unit/clipboard_test.ts b/tests/cypress/tests/unit/clipboard_test.ts index 2c6efbbc6..7ed414b91 100644 --- a/tests/cypress/tests/unit/clipboard_test.ts +++ b/tests/cypress/tests/unit/clipboard_test.ts @@ -3,13 +3,75 @@ import * as R from 'ramda'; import applyClipboardToData from 'dash-table/utils/applyClipboardToData'; describe('clipboard', () => { + const columns = ['c1'].map(id => ({ id: id, name: id, editable: true, sort_as_null: [] })); + + describe('with hidden columns', () => { + const altColumns = [ + 'c1', + 'c2' + ].map(id => ({ id: id, name: id, editable: true, sort_as_null: [] })); + + describe('with column overflow', () => { + it('returns all columns, 1st column visible only', () => { + const altVisibleColumns = altColumns.slice(0, 1); + + const res = applyClipboardToData( + R.range(0, 2).map(value => [`${value}`, `${value + 1}`]), + { row: 0, column: 0, column_id: '' }, + R.range(0, 1), + altColumns, + altVisibleColumns, + R.range(0, 1).map(() => ({ c1: 'c1', c2: 'c2' })), + true, + true + ); + + expect(res).to.not.equal(undefined); + + if (res) { + expect(res.data.length).to.equal(2); + expect(Object.entries(res.data[0]).length).to.equal(3); + expect(res.columns.length).to.equal(3); + expect(res.columns[0].id).to.equal('c1'); + expect(res.columns[2].id).to.equal('c2'); + } + }); + }); + + it('returns all columns, 2nd column visible only', () => { + const altVisibleColumns = altColumns.slice(-1); + + const res = applyClipboardToData( + R.range(0, 2).map(value => [`${value}`, `${value + 1}`]), + { row: 0, column: 0, column_id: '' }, + R.range(0, 1), + altColumns, + altVisibleColumns, + R.range(0, 1).map(() => ({ c1: 'c1', c2: 'c2' })), + true, + true + ); + + expect(res).to.not.equal(undefined); + + if (res) { + expect(res.data.length).to.equal(2); + expect(Object.entries(res.data[0]).length).to.equal(3); + expect(res.columns.length).to.equal(3); + expect(res.columns[0].id).to.equal('c1'); + expect(res.columns[1].id).to.equal('c2'); + } + }); + }); + describe('with column/row overflow allowed', () => { it('pastes one line at [0, 0] in one line df', () => { const res = applyClipboardToData( R.range(0, 1).map(value => [`${value}`]), - {row: 0, column: 0, column_id: ''}, + { row: 0, column: 0, column_id: '' }, R.range(0, 1), - ['c1'].map(id => ({ id: id, name: id, editable: true, sort_as_null: [] })), + columns, + columns, R.range(0, 1).map(() => ({ c1: 'c1' })), true, true @@ -26,9 +88,10 @@ describe('clipboard', () => { it('pastes two lines at [0, 0] in one line df', () => { const res = applyClipboardToData( R.range(0, 2).map(value => [`${value}`]), - {row: 0, column: 0, column_id: ''}, + { row: 0, column: 0, column_id: '' }, R.range(0, 1), - ['c1'].map(id => ({ id: id, name: id, editable: true, sort_as_null: [] })), + columns, + columns, R.range(0, 1).map(() => ({ c1: 'c1' })), true, true @@ -46,9 +109,10 @@ describe('clipboard', () => { it('pastes ten lines at [0, 0] in three line df', () => { const res = applyClipboardToData( R.range(0, 10).map(value => [`${value}`]), - {row: 0, column: 0, column_id: ''}, + { row: 0, column: 0, column_id: '' }, R.range(0, 3), - ['c1'].map(id => ({ id: id, name: id, editable: true, sort_as_null: [] })), + columns, + columns, R.range(0, 3).map(() => ({ c1: 'c1' })), true, true @@ -67,9 +131,10 @@ describe('clipboard', () => { it('pastes ten lines at [1, 0] in three line df', () => { const res = applyClipboardToData( R.range(0, 10).map(value => [`${value}`]), - {row: 1, column: 0, column_id: ''}, + { row: 1, column: 0, column_id: '' }, R.range(0, 3), - ['c1'].map(id => ({ id: id, name: id, editable: true, sort_as_null: [] })), + columns, + columns, R.range(0, 3).map(() => ({ c1: 'c1' })), true, true @@ -91,9 +156,10 @@ describe('clipboard', () => { it('pastes one line at [0, 0] in one line df', () => { const res = applyClipboardToData( R.range(0, 1).map(value => [`${value}`]), - {row: 0, column: 0, column_id: ''}, + { row: 0, column: 0, column_id: '' }, R.range(0, 1), - ['c1'].map(id => ({ id: id, name: id, editable: true, sort_as_null: [] })), + columns, + columns, R.range(0, 1).map(() => ({ c1: 'c1' })), true, false @@ -110,9 +176,10 @@ describe('clipboard', () => { it('pastes two lines at [0, 0] in one line df', () => { const res = applyClipboardToData( R.range(0, 2).map(value => [`${value}`]), - {row: 0, column: 0, column_id: ''}, + { row: 0, column: 0, column_id: '' }, R.range(0, 1), - ['c1'].map(id => ({ id: id, name: id, editable: true, sort_as_null: [] })), + columns, + columns, R.range(0, 1).map(() => ({ c1: 'c1' })), true, false @@ -129,9 +196,10 @@ describe('clipboard', () => { it('pastes ten lines at [0, 0] in three line df', () => { const res = applyClipboardToData( R.range(0, 10).map(value => [`${value}`]), - {row: 0, column: 0, column_id: ''}, + { row: 0, column: 0, column_id: '' }, R.range(0, 3), - ['c1'].map(id => ({ id: id, name: id, editable: true, sort_as_null: [] })), + columns, + columns, R.range(0, 3).map(() => ({ c1: 'c1' })), true, false @@ -150,9 +218,10 @@ describe('clipboard', () => { it('pastes ten lines at [1, 0] in three line df', () => { const res = applyClipboardToData( R.range(0, 10).map(value => [`${value}`]), - {row: 1, column: 0, column_id: ''}, + { row: 1, column: 0, column_id: '' }, R.range(0, 3), - ['c1'].map(id => ({ id: id, name: id, editable: true, sort_as_null: [] })), + columns, + columns, R.range(0, 3).map(() => ({ c1: 'c1' })), true, false From 7ce02569a992e08de2907ac5d8930a185f905744 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andre=CC=81=20Rivet?= Date: Thu, 17 Oct 2019 08:22:08 -0400 Subject: [PATCH 2/2] changelog entry --- CHANGELOG.md | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 21766c755..de695dc76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,15 +4,10 @@ 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. - -- [#206](https://github.com/plotly/dash-table/issues/206) Fix a bug with copy/paste to and from -column filters not working. - -- [#561](https://github.com/plotly/dash-table/issues/561) Fix an incorrect React PureComponent -usage causing warnings in DevTools. +- [#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. +- [#206](https://github.com/plotly/dash-table/issues/206) Fix a bug with copy/paste to and from column filters not working. +- [#561](https://github.com/plotly/dash-table/issues/561) Fix an incorrect React PureComponent usage causing warnings in DevTools. +- [#611](https://github.com/plotly/dash-table/issues/611) Fix a bug with copy/paste causing hidden columns to be removed from the table ## [4.4.0] - 2019-10-08 ### Added