Skip to content

Commit

Permalink
[FIX] clipboard: cannot paste whole table style
Browse files Browse the repository at this point in the history
Trying to copy a whole table, and then paste only the formatting
didn't work and nothing was pasted. This commit fixes the issue.

closes #5039

Task: 4218988
Signed-off-by: Rémi Rahir (rar) <[email protected]>
  • Loading branch information
hokolomopo committed Sep 30, 2024
1 parent 2289fc3 commit f562b8e
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 22 deletions.
32 changes: 21 additions & 11 deletions src/clipboard_handlers/tables_clipboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ interface CopiedTable {
interface TableCell {
style?: TableStyle;
table?: CopiedTable;
isWholeTableCopied?: boolean;
}

interface ClipboardContent {
Expand Down Expand Up @@ -55,27 +56,30 @@ export class TableClipboardHandler extends AbstractCellClipboardHandler<
for (let col of columnsIndexes) {
const position = { col, row, sheetId };
const table = this.getters.getTable(position);
if (!table || copiedTablesIds.has(table.id)) {
if (!table) {
tableCellsInRow.push({});
continue;
}
let copiedTable: CopiedTable | undefined = undefined;
// Copy whole table
if (zones.some((z) => isZoneInside(table.range.zone, z))) {
if (
!copiedTablesIds.has(table.id) &&
zones.some((z) => isZoneInside(table.range.zone, z))
) {
copiedTablesIds.add(table.id);
const values: Array<string[]> = [];
for (const col of range(table.range.zone.left, table.range.zone.right + 1)) {
values.push(
this.getters.getFilterHiddenValues({ sheetId, col, row: table.range.zone.top })
);
}
tableCellsInRow.push({
table: { filtersValues: values, range: table.range, config: table.config },
});
}
// Copy only style of cell
else {
tableCellsInRow.push({ style: this.getTableStyleToCopy(position) });
copiedTable = { filtersValues: values, range: table.range, config: table.config };
}
tableCellsInRow.push({
table: copiedTable,
style: this.getTableStyleToCopy(position),
isWholeTableCopied: copiedTablesIds.has(table.id),
});
}
}

Expand Down Expand Up @@ -192,8 +196,14 @@ export class TableClipboardHandler extends AbstractCellClipboardHandler<
}

// Do not paste table style if we're inside another table
if (!this.getters.getTable(position)) {
if (tableCell.style?.style && options?.pasteOption !== "asValue") {
if (this.getters.getTable(position) || options?.pasteOption === "asValue") {
return;
}
if (
(!options?.pasteOption && !tableCell.isWholeTableCopied) ||
options?.pasteOption === "onlyFormat"
) {
if (tableCell.style?.style) {
this.dispatch("UPDATE_CELL", { ...position, style: tableCell.style.style });
}
if (tableCell.style?.border) {
Expand Down
30 changes: 19 additions & 11 deletions tests/table/tables_plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { CommandResult, Model } from "../../src";
import { DEFAULT_BORDER_DESC } from "../../src/constants";
import { toUnboundedZone, toZone, zoneToXc } from "../../src/helpers";
import { TABLE_PRESETS } from "../../src/helpers/table_presets";
import { ClipboardPasteOptions, UID } from "../../src/types";
import { UID } from "../../src/types";
import {
addColumns,
addRows,
Expand Down Expand Up @@ -803,17 +803,25 @@ describe("Table plugin", () => {
expect(getCell(model, "B1")?.style?.fillColor).not.toEqual("#FF0000");
});

test.each(["onlyFormat", "asValue"] as ClipboardPasteOptions[])(
"Special paste %s don't paste whole tables",
(pasteOption: ClipboardPasteOptions) => {
createTable(model, "A1:B4");
updateFilter(model, "A1", ["thisIsAValue"]);
test("Paste as value do not copy the table", () => {
createTable(model, "A1:B4", { styleId: "TestStyleAllRed" });

copy(model, "A1:B4");
paste(model, "A5", pasteOption);
expect(getTable(model, "A5")).toBeFalsy();
}
);
copy(model, "A1:B4");
paste(model, "A5", "asValue");
expect(getTable(model, "A5")).toBeFalsy();
expect(getCell(model, "A5")?.style).toBeUndefined();
});

test("Can copy/paste the whole table formatting", () => {
createTable(model, "A1:A2", { styleId: "TestStyleAllRed" });

copy(model, "A1:A2");
paste(model, "A5", "onlyFormat");
expect(getTable(model, "A5")).toBeFalsy();
expect(getCell(model, "A5")?.style).toEqual({ fillColor: "#FF0000", bold: true });
expect(getBorder(model, "A5")).toEqual({ top: DEFAULT_BORDER_DESC });
expect(getCell(model, "A6")?.style).toEqual({ fillColor: "#FF0000" });
});

test("Pasting onlyFormat with a partial table copied paste the table style, not asValue", () => {
createTable(model, "A1:B4");
Expand Down

0 comments on commit f562b8e

Please sign in to comment.