Skip to content

Commit 123dbc2

Browse files
committed
[FIX] header_size: wrong row size from wrapped number
If a literal cell containing a number was wrapped (because the number was large, or because it had a long format), the row height was wrong. Task: 4878338
1 parent 498c3fc commit 123dbc2

File tree

4 files changed

+42
-7
lines changed

4 files changed

+42
-7
lines changed

src/helpers/text_helper.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ import {
88
NEWLINE,
99
PADDING_AUTORESIZE_VERTICAL,
1010
} from "../constants";
11-
import { Cell, Pixel, PixelPosition, Style } from "../types";
11+
import { Cell, DEFAULT_LOCALE, Locale, Pixel, PixelPosition, Style } from "../types";
12+
import { CellErrorType } from "../types/errors";
13+
import { parseLiteral } from "./cells";
14+
import { formatValue } from "./format";
1215
import { isMarkdownLink, parseMarkdownLink } from "./misc";
1316

1417
export function computeTextLinesHeight(textLineHeight: number, numberOfLines: number = 1) {
@@ -21,12 +24,22 @@ export function computeTextLinesHeight(textLineHeight: number, numberOfLines: nu
2124
export function getDefaultCellHeight(
2225
ctx: CanvasRenderingContext2D,
2326
cell: Cell | undefined,
27+
locale: Locale,
2428
colSize: number
2529
) {
2630
if (!cell || (!cell.isFormula && !cell.content)) {
2731
return DEFAULT_CELL_HEIGHT;
2832
}
29-
const content = cell.isFormula ? "" : cell.content;
33+
let content = "";
34+
35+
try {
36+
if (!cell.isFormula) {
37+
const localeFormat = { format: cell.format, locale: DEFAULT_LOCALE };
38+
content = formatValue(parseLiteral(cell.content, locale), localeFormat);
39+
}
40+
} catch {
41+
content = CellErrorType.GenericError;
42+
}
3043
return getCellContentHeight(ctx, content, cell.style, colSize);
3144
}
3245

src/plugins/ui_core_views/header_sizes_ui.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ export class HeaderSizeUIPlugin extends UIPlugin<HeaderSizeState> implements Hea
173173
const cell = this.getters.getCell(position);
174174

175175
const colSize = this.getters.getColSize(position.sheetId, position.col);
176-
return getDefaultCellHeight(this.ctx, cell, colSize);
176+
return getDefaultCellHeight(this.ctx, cell, this.getters.getLocale(), colSize);
177177
}
178178

179179
private isInMultiRowMerge(position: CellPosition): boolean {

tests/collaborative/collaborative.test.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,13 @@ import { DEFAULT_REVISION_ID, MESSAGE_VERSION } from "../../src/constants";
33
import { functionRegistry } from "../../src/functions";
44
import { getDefaultCellHeight, range, toCartesian, toZone, zoneToXc } from "../../src/helpers";
55
import { featurePluginRegistry } from "../../src/plugins";
6-
import { Command, CommandResult, CoreCommand, DataValidationCriterion } from "../../src/types";
6+
import {
7+
Command,
8+
CommandResult,
9+
CoreCommand,
10+
DEFAULT_LOCALE,
11+
DataValidationCriterion,
12+
} from "../../src/types";
713
import { CollaborationMessage } from "../../src/types/collaborative/transport_service";
814
import { MockTransportService } from "../__mocks__/transport_service";
915
import {
@@ -1083,7 +1089,7 @@ describe("Multi users synchronisation", () => {
10831089
const ctx = document.createElement("canvas").getContext("2d")!;
10841090
expect([alice, bob, charlie]).toHaveSynchronizedValue(
10851091
(user) => user.getters.getRowSize("sheet2", 0),
1086-
getDefaultCellHeight(ctx, getCell(alice, "A1"), colSize)
1092+
getDefaultCellHeight(ctx, getCell(alice, "A1"), DEFAULT_LOCALE, colSize)
10871093
);
10881094
});
10891095

tests/headers/resizing_plugin.test.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {
66
} from "../../src/constants";
77
import { getDefaultCellHeight as getDefaultCellHeightHelper, toXC } from "../../src/helpers";
88
import { Model } from "../../src/model";
9-
import { Cell, CommandResult, Sheet, Wrapping } from "../../src/types";
9+
import { Cell, CommandResult, DEFAULT_LOCALE, Sheet, Wrapping } from "../../src/types";
1010
import {
1111
activateSheet,
1212
addColumns,
@@ -22,15 +22,17 @@ import {
2222
resizeColumns,
2323
resizeRows,
2424
setCellContent,
25+
setFormat,
2526
setStyle,
2627
unMerge,
2728
undo,
2829
} from "../test_helpers/commands_helpers";
2930
import { getCell } from "../test_helpers/getters_helpers";
31+
import { target } from "../test_helpers/helpers";
3032

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

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

446+
test("wrapped formatted text updates the row size", () => {
447+
setStyle(model, "C1", { fontSize: 10, wrapping: "wrap" });
448+
resizeColumns(model, ["C"], 100);
449+
setCellContent(model, "C1", "200");
450+
451+
expect(model.getters.getRowSize(sheet.id, 0)).toBe(DEFAULT_CELL_HEIGHT);
452+
453+
setFormat(model, "0[$long escaped string in the format that will be wrapped]", target("C1"));
454+
const cell = getCell(model, "C1");
455+
const expectedHeight = getDefaultCellHeight(cell, 100);
456+
expect(expectedHeight).toBeGreaterThan(DEFAULT_CELL_HEIGHT);
457+
expect(model.getters.getRowSize(sheet.id, 0)).toBe(expectedHeight);
458+
});
459+
444460
test.each<Wrapping>(["overflow", "clip"])(
445461
`wrapping text with style %s does not update the row size`,
446462
(wrap: Wrapping) => {

0 commit comments

Comments
 (0)