Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 15 additions & 2 deletions src/helpers/text_helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ import {
NEWLINE,
PADDING_AUTORESIZE_VERTICAL,
} from "../constants";
import { Cell, Pixel, PixelPosition, Style } from "../types";
import { Cell, DEFAULT_LOCALE, Locale, Pixel, PixelPosition, Style } from "../types";
import { CellErrorType } from "../types/errors";
import { parseLiteral } from "./cells";
import { formatValue } from "./format";
import { isMarkdownLink, parseMarkdownLink } from "./misc";

export function computeTextLinesHeight(textLineHeight: number, numberOfLines: number = 1) {
Expand All @@ -21,12 +24,22 @@ export function computeTextLinesHeight(textLineHeight: number, numberOfLines: nu
export function getDefaultCellHeight(
ctx: CanvasRenderingContext2D,
cell: Cell | undefined,
locale: Locale,
colSize: number
) {
if (!cell || (!cell.isFormula && !cell.content)) {
return DEFAULT_CELL_HEIGHT;
}
const content = cell.isFormula ? "" : cell.content;
let content = "";

try {
if (!cell.isFormula) {
const localeFormat = { format: cell.format, locale: DEFAULT_LOCALE };
content = formatValue(parseLiteral(cell.content, locale), localeFormat);
}
} catch {
content = CellErrorType.GenericError;
Copy link
Collaborator

Choose a reason for hiding this comment

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

can this fail for other reasons than an invalid format?
and is it possible in real life conditions to have an invalid format?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah only for invalid formats. I don't think it should happens in practice, but we do have a test with invalid format that fails without the try/catch so ... (Update cell with a format is correctly set)

Copy link
Collaborator

@rrahir rrahir Dec 9, 2025

Choose a reason for hiding this comment

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

we should discuss the possibility/necessity to reject invalid formats then ^^

}
return getCellContentHeight(ctx, content, cell.style, colSize);
}

Expand Down
2 changes: 1 addition & 1 deletion src/plugins/ui_core_views/header_sizes_ui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ export class HeaderSizeUIPlugin extends UIPlugin<HeaderSizeState> implements Hea
const cell = this.getters.getCell(position);

const colSize = this.getters.getColSize(position.sheetId, position.col);
return getDefaultCellHeight(this.ctx, cell, colSize);
return getDefaultCellHeight(this.ctx, cell, this.getters.getLocale(), colSize);
}

private isInMultiRowMerge(position: CellPosition): boolean {
Expand Down
10 changes: 8 additions & 2 deletions tests/collaborative/collaborative.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,13 @@ import { DEFAULT_REVISION_ID, MESSAGE_VERSION } from "../../src/constants";
import { functionRegistry } from "../../src/functions";
import { getDefaultCellHeight, range, toCartesian, toZone, zoneToXc } from "../../src/helpers";
import { featurePluginRegistry } from "../../src/plugins";
import { Command, CommandResult, CoreCommand, DataValidationCriterion } from "../../src/types";
import {
Command,
CommandResult,
CoreCommand,
DEFAULT_LOCALE,
DataValidationCriterion,
} from "../../src/types";
import { CollaborationMessage } from "../../src/types/collaborative/transport_service";
import { MockTransportService } from "../__mocks__/transport_service";
import {
Expand Down Expand Up @@ -1083,7 +1089,7 @@ describe("Multi users synchronisation", () => {
const ctx = document.createElement("canvas").getContext("2d")!;
expect([alice, bob, charlie]).toHaveSynchronizedValue(
(user) => user.getters.getRowSize("sheet2", 0),
getDefaultCellHeight(ctx, getCell(alice, "A1"), colSize)
getDefaultCellHeight(ctx, getCell(alice, "A1"), DEFAULT_LOCALE, colSize)
);
});

Expand Down
20 changes: 18 additions & 2 deletions tests/headers/resizing_plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
} from "../../src/constants";
import { getDefaultCellHeight as getDefaultCellHeightHelper, toXC } from "../../src/helpers";
import { Model } from "../../src/model";
import { Cell, CommandResult, Sheet, Wrapping } from "../../src/types";
import { Cell, CommandResult, DEFAULT_LOCALE, Sheet, Wrapping } from "../../src/types";
import {
activateSheet,
addColumns,
Expand All @@ -22,15 +22,17 @@ import {
resizeColumns,
resizeRows,
setCellContent,
setFormat,
setStyle,
unMerge,
undo,
} from "../test_helpers/commands_helpers";
import { getCell } from "../test_helpers/getters_helpers";
import { target } from "../test_helpers/helpers";

const ctx = document.createElement("canvas").getContext("2d")!;
function getDefaultCellHeight(cell: Cell | undefined, colSize = DEFAULT_CELL_WIDTH) {
return Math.round(getDefaultCellHeightHelper(ctx, cell, colSize));
return Math.round(getDefaultCellHeightHelper(ctx, cell, DEFAULT_LOCALE, colSize));
}

describe("Model resizer", () => {
Expand Down Expand Up @@ -441,6 +443,20 @@ describe("Model resizer", () => {
expect(model.getters.getRowSize(sheet.id, 0)).toBe(wrappedCellHeight);
});

test("wrapped formatted text updates the row size", () => {
setStyle(model, "C1", { fontSize: 10, wrapping: "wrap" });
resizeColumns(model, ["C"], 100);
setCellContent(model, "C1", "200");

expect(model.getters.getRowSize(sheet.id, 0)).toBe(DEFAULT_CELL_HEIGHT);

setFormat(model, "0[$long escaped string in the format that will be wrapped]", target("C1"));
const cell = getCell(model, "C1");
const expectedHeight = getDefaultCellHeight(cell, 100);
expect(expectedHeight).toBeGreaterThan(DEFAULT_CELL_HEIGHT);
expect(model.getters.getRowSize(sheet.id, 0)).toBe(expectedHeight);
});

test.each<Wrapping>(["overflow", "clip"])(
`wrapping text with style %s does not update the row size`,
(wrap: Wrapping) => {
Expand Down