Skip to content
This repository has been archived by the owner on Jun 4, 2024. It is now read-only.

3.1 issue142 copy paste #180

Merged
merged 7 commits into from
Oct 31, 2018
Merged

3.1 issue142 copy paste #180

merged 7 commits into from
Oct 31, 2018

Conversation

Marc-Andre-Rivet
Copy link
Contributor

@Marc-Andre-Rivet Marc-Andre-Rivet commented Oct 26, 2018

Fixes #142.

The table currently supports copy/paste that exceeds the number of rows in the viewport if no sorting and no filtering is applied. The code appended the required rows at the end of the data but failed to map to the new rows when pasting -- effectively pasting to the existing rows and adding empty rows without pasting the values to them.

This PR fixes this behavior and adds a few unit tests to validate the paste behavior.

@chriddyp chriddyp temporarily deployed to dash-table-review-pr-180 October 26, 2018 19:10 Inactive
package.json Outdated
@@ -1,6 +1,6 @@
{
"name": "dash-table",
"version": "3.1.0rc11",
"version": "3.1.0rc12",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

bump version

@chriddyp chriddyp temporarily deployed to dash-table-review-pr-180 October 26, 2018 19:11 Inactive
data: Data,
overflowColumns: boolean = true,
overflowRows: boolean = true
): { data: Data, columns: Columns } | void => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isolated the pasting logic so it's not dependent on sheetclip and the clipboard event. Makes unit testing cleaner.

derived_viewport_indices[viewportIndex] :
overflowRows ?
lastEntry + (viewportIndex - viewportSize + 1) :
undefined;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is where the main change is. If the row falls in the existing (non-appended) data, map it directly to the viewport indices, if not, map it to the last entry of the viewport + offset, if overflow is not supported return undefined instead (will be validated below to exclude those rows from being processed)

import applyClipboardToData from 'dash-table/utils/applyClipboardToData';

describe('clipboard', () => {
describe('with column/row overflow allowed', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Various tests to validate behavior when copying n rows into a m rows dataframe with overflow allowed (new rows get appended)

});

describe('with column overflow allowed', () => {
it('pastes one line at [0, 0] in one line df', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Various tests to validate behavior when copying n rows into a m rows dataframe with overflow unallowed (no new rows appended)

@chriddyp chriddyp temporarily deployed to dash-table-review-pr-180 October 26, 2018 19:17 Inactive
…aste

# Conflicts:
#	CHANGELOG.md
#	dash_table/bundle.js
#	dash_table/demo.js
@chriddyp chriddyp temporarily deployed to dash-table-review-pr-180 October 29, 2018 19:10 Inactive
@valentijnnieman
Copy link
Contributor

The BE roundtrip on copy-paste test is failing locally for me!

…aste

# Conflicts:
#	CHANGELOG.md
#	dash_table/bundle.js
#	dash_table/demo.js
#	src/dash-table/utils/TableClipboardHelper.ts
@chriddyp chriddyp temporarily deployed to dash-table-review-pr-180 October 30, 2018 20:41 Inactive
@Marc-Andre-Rivet
Copy link
Contributor Author

@valentijnnieman Not experiencing issues with the local tests and CI seems fine / stable. Will merge and we can have another look at the environment after.

@Marc-Andre-Rivet Marc-Andre-Rivet merged commit 7393ed0 into master Oct 31, 2018
@Marc-Andre-Rivet Marc-Andre-Rivet deleted the 3.1-issue142-copy-paste branch July 18, 2019 12:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

copying and pasting this example skips some rows
3 participants