From 266c14d5d28358a1e9e3b83edda855e1adeb740f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Hatcher?= Date: Sat, 19 Jul 2025 07:51:05 +0200 Subject: [PATCH] FIX: Apply copilot suggestions --- base/src/user_model/common.rs | 6 +++-- .../WorksheetCanvas/worksheetCanvas.ts | 23 +++++-------------- 2 files changed, 10 insertions(+), 19 deletions(-) diff --git a/base/src/user_model/common.rs b/base/src/user_model/common.rs index 156a60a..dcb4269 100644 --- a/base/src/user_model/common.rs +++ b/base/src/user_model/common.rs @@ -1603,7 +1603,7 @@ impl UserModel { } /// Returns the largest column in the row less than a column whose cell has a non empty value. - /// If the row is empty, it returns `None`. + /// If there are none it returns `None`. /// This is useful when rendering a part of a worksheet to know which cells spill over pub fn get_last_non_empty_in_row_before_column( &self, @@ -1633,7 +1633,9 @@ impl UserModel { } } - /// Returns the largest column in the row greater than a column whose cell has a non empty value. + /// Returns the smallest column in the row larger than "column" whose cell has a non empty value. + /// If there are none it returns `None`. + /// This is useful when rendering a part of a worksheet to know which cells spill over pub fn get_first_non_empty_in_row_after_column( &self, sheet: u32, diff --git a/webapp/IronCalc/src/components/WorksheetCanvas/worksheetCanvas.ts b/webapp/IronCalc/src/components/WorksheetCanvas/worksheetCanvas.ts index 798449e..e7e8275 100644 --- a/webapp/IronCalc/src/components/WorksheetCanvas/worksheetCanvas.ts +++ b/webapp/IronCalc/src/components/WorksheetCanvas/worksheetCanvas.ts @@ -55,8 +55,6 @@ export const defaultCellFontFamily = fonts.regular; export const headerFontFamily = fonts.regular; export const frozenSeparatorWidth = 3; -type Tuple = [number, number]; - interface TextProperties { row: number; column: number; @@ -136,14 +134,9 @@ export default class WorksheetCanvas { this.onRowHeightChanges = options.onRowHeightChanges; this.resetHeaders(); this.cellOutlineHandle = attachOutlineHandle(this); - // +1 means the cell is filled with a spill from a cell in the left - // -1 means the cell is filled with a spill from a cell in the right - this.spills = new Map(); - this.cells = []; - } - removeSpills(): void { - this.spills.clear(); + // a cell marked as "spill" means its left border should be skipped + this.spills = new Map(); this.cells = []; } @@ -685,12 +678,14 @@ export default class WorksheetCanvas { leftColumnX > minX && this.model.getFormattedCellValue(selectedSheet, row, spillColumn) === "" && - spillColumn > 0 && + spillColumn >= 1 && ((column <= frozenColumnsCount && spillColumn <= frozenColumnsCount) || column > frozenColumnsCount) ) { leftColumnX -= this.getColumnWidth(selectedSheet, spillColumn); - // marks (row, spillColumn) as spilling so we don't draw a border to the left + // This is tricky but correct. The reason is we only draw the left borders of the cells + // (because the left border of a cell MUST be the right border of the one to the left). + // So if we want to remove the right border of this cell we need to skip the left border of the next. this.spills.set(`${row}-${spillColumn + 1}`, 1); spillColumn -= 1; } @@ -1735,12 +1730,6 @@ export default class WorksheetCanvas { } renderSheet(): void { - console.time("renderSheet"); - this._renderSheet(); - console.timeEnd("renderSheet"); - } - - _renderSheet(): void { const context = this.ctx; const { canvas } = this; const selectedSheet = this.model.getSelectedSheet();