Skip to content
This repository was archived by the owner on Aug 29, 2025. It is now read-only.

Issue 611 - Hidden columns on copy/paste #621

Merged
Marc-Andre-Rivet merged 5 commits into from
Oct 17, 2019
Merged

Issue 611 - Hidden columns on copy/paste #621

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1ed5ab3
- hidden columns on copy/paste
Oct 9, 2019
fb3fc1a
Merge branch 'dev' into 611-copy-paste-hideable-columns
Marc-Andre-Rivet Oct 9, 2019
f8c5919
Merge branch 'dev' into 611-copy-paste-hideable-columns
Marc-Andre-Rivet Oct 9, 2019
92e6a18
Merge branch 'dev' into 611-copy-paste-hideable-columns
Marc-Andre-Rivet Oct 17, 2019
7ce0256
changelog entry
Oct 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 4 additions & 9 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/dash-table/components/ControlledTable/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,7 @@ export default class ControlledTable extends PureComponent<ControlledTableProps>
onPaste = (e: any) => {
const {
active_cell,
columns,
data,
editable,
filter_query,
Expand All @@ -606,6 +607,7 @@ export default class ControlledTable extends PureComponent<ControlledTableProps>
e,
active_cell,
viewport.indices,
columns,
visibleColumns,
data,
true,
Expand Down
2 changes: 2 additions & 0 deletions src/dash-table/utils/TableClipboardHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export default class TableClipboardHelper {
activeCell: ICellCoordinates,
derived_viewport_indices: number[],
columns: Columns,
visibleColumns: Columns,
data: Data,
overflowColumns: boolean = true,
overflowRows: boolean = true,
Expand All @@ -73,6 +74,7 @@ export default class TableClipboardHelper {
activeCell,
derived_viewport_indices,
columns,
visibleColumns,
data,
overflowColumns,
overflowRows
Expand Down
27 changes: 20 additions & 7 deletions src/dash-table/utils/applyClipboardToData.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -23,29 +24,41 @@ 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,
sort_as_null: []
} 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);

@Marc-Andre-Rivet Marc-Andre-Rivet Oct 9, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When new columns are created, they are appended after the last visible column. Both visible and "all" columns are updated to both (1) handle the copy/paste below and (2) the columns update correctly.

@alexcjohnson alexcjohnson Oct 17, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm OK - I might have added the new columns after trailing hidden columns as well, but it's probably impossible to declare one way objectively better than the other. This is fine.

}

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(
Expand Down Expand Up @@ -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;
}
Expand Down
101 changes: 85 additions & 16 deletions tests/cypress/tests/unit/clipboard_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');

@Marc-Andre-Rivet Marc-Andre-Rivet Oct 9, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check that all columns are present and in the right order after a copy/paste operation involving hidden columns

}
});
});

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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down