Skip to content

Commit

Permalink
[IMP] clipboard: improve cut formula behaviour
Browse files Browse the repository at this point in the history
Previously, when cutting a formula `=A1` in sheet1, and pasting it in
sheet2, the pasted formula would refer to `A1` in sheet2. The more
correct behaviour is to refer to `A1` in sheet1, transforming the
pasted formula to `=Sheet1!A1`.

Also when cutting a formula `=Sheet2!A1` from sheet1 to sheet2,
we can drop the prefix `Sheet2!` and only paste `=A1` in sheet2.

closes #4152

Task: 3898438
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
  • Loading branch information
hokolomopo authored and LucasLefevre committed May 10, 2024
1 parent 3662b14 commit 9a49647
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 10 deletions.
20 changes: 11 additions & 9 deletions src/clipboard_handlers/cell_clipboard.ts
Expand Up @@ -238,15 +238,17 @@ export class CellClipboardHandler extends AbstractCellClipboardHandler<
return;
}

const content =
origin.cell && origin.cell.isFormula && !clipboardOption?.isCutOperation
? this.getters.getTranslatedCellFormula(
sheetId,
col - origin.position.col,
row - origin.position.row,
origin.cell.compiledFormula
)
: origin.cell?.content;
let content = origin.cell?.content;
if (origin.cell?.isFormula && !clipboardOption?.isCutOperation) {
content = this.getters.getTranslatedCellFormula(
sheetId,
col - origin.position.col,
row - origin.position.row,
origin.cell.compiledFormula
);
} else if (origin.cell?.isFormula) {
content = this.getters.getFormulaMovedInSheet(sheetId, origin.cell.compiledFormula);
}
if (content !== "" || origin.cell?.format || origin.cell?.style) {
this.dispatch("UPDATE_CELL", {
...target,
Expand Down
3 changes: 2 additions & 1 deletion src/helpers/range.ts
Expand Up @@ -178,7 +178,8 @@ export class RangeImpl implements Range {
: this.parts.map((part) => {
return { rowFixed: part.rowFixed, colFixed: part.colFixed };
}),
prefixSheet: rangeParams?.prefixSheet ? rangeParams.prefixSheet : this.prefixSheet,
prefixSheet:
rangeParams?.prefixSheet !== undefined ? rangeParams.prefixSheet : this.prefixSheet,
},
this.getSheetSize
);
Expand Down
1 change: 1 addition & 0 deletions src/model.ts
Expand Up @@ -223,6 +223,7 @@ export class Model extends EventBus<any> implements CommandDispatcher {
this.coreGetters.isRangeValid = this.range.isRangeValid.bind(this.range);
this.coreGetters.extendRange = this.range.extendRange.bind(this.range);
this.coreGetters.getRangesUnion = this.range.getRangesUnion.bind(this.range);
this.coreGetters.removeRangesSheetPrefix = this.range.removeRangesSheetPrefix.bind(this.range);

this.getters = {
isReadonly: () => this.config.mode === "readonly" || this.config.mode === "dashboard",
Expand Down
7 changes: 7 additions & 0 deletions src/plugins/core/cell.ts
Expand Up @@ -62,6 +62,7 @@ export class CellPlugin extends CorePlugin<CoreState> implements CoreState {
"getTranslatedCellFormula",
"getCellStyle",
"getCellById",
"getFormulaMovedInSheet",
] as const;
readonly nextId = 1;
public readonly cells: { [sheetId: string]: { [id: string]: Cell } } = {};
Expand Down Expand Up @@ -373,6 +374,12 @@ export class CellPlugin extends CorePlugin<CoreState> implements CoreState {
return this.getFormulaCellContent(sheetId, compiledFormula, adaptedDependencies);
}

getFormulaMovedInSheet(targetSheetId: UID, compiledFormula: RangeCompiledFormula) {
const dependencies = compiledFormula.dependencies;
const adaptedDependencies = this.getters.removeRangesSheetPrefix(targetSheetId, dependencies);
return this.getFormulaCellContent(targetSheetId, compiledFormula, adaptedDependencies);
}

getCellStyle(position: CellPosition): Style {
return this.getters.getCell(position)?.style || {};
}
Expand Down
14 changes: 14 additions & 0 deletions src/plugins/core/range.ts
Expand Up @@ -52,6 +52,7 @@ export class RangeAdapter implements CommandHandler<CoreCommand> {
"getRangesUnion",
"recomputeRanges",
"isRangeValid",
"removeRangesSheetPrefix",
] as const;

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -292,6 +293,19 @@ export class RangeAdapter implements CommandHandler<CoreCommand> {
});
}

/**
* Remove the sheet name prefix if a range is part of the given sheet.
*/
removeRangesSheetPrefix(sheetId: UID, ranges: Range[]): Range[] {
return ranges.map((range) => {
const rangeImpl = RangeImpl.fromRange(range, this.getters);
if (rangeImpl.prefixSheet && rangeImpl.sheetId === sheetId) {
return rangeImpl.clone({ prefixSheet: false });
}
return rangeImpl;
});
}

extendRange(range: Range, dimension: Dimension, quantity: number): Range {
const rangeImpl = RangeImpl.fromRange(range, this.getters);
const right = dimension === "COL" ? rangeImpl.zone.right + quantity : rangeImpl.zone.right;
Expand Down
13 changes: 13 additions & 0 deletions tests/clipboard/clipboard_plugin.test.ts
Expand Up @@ -1371,6 +1371,19 @@ describe("clipboard", () => {
expect(getCellText(model, "B2")).toBe("=SUM(C1:C2)");
});

test("cut/paste a formula with references in another sheet updates the sheet references in the formula", () => {
const model = new Model();
createSheet(model, { sheetId: "sh2", name: "Sheet2" });
setCellContent(model, "A1", "=SUM(C1:C2)");
setCellContent(model, "B1", "=Sheet2!A1 + A2");
cut(model, "A1:B1");

activateSheet(model, "sh2");
paste(model, "A1");
expect(getCellText(model, "A1")).toBe("=SUM(Sheet1!C1:C2)");
expect(getCellText(model, "B1")).toBe("=A1 + Sheet1!A2");
});

test("copy/paste a zone present in formulas references does not update references", () => {
const model = new Model();
setCellContent(model, "A1", "=B2");
Expand Down

0 comments on commit 9a49647

Please sign in to comment.