Skip to content

Commit

Permalink
[FIX] xlsx: import new lines in sharedStrings
Browse files Browse the repository at this point in the history
When implementing the multiline text feature, we forgot to change the
xlsx import, that explicitly removed new lines from the sharedStrings
because we didn't support it at the time.

There was also a bug where the whitespace was not preserved in the
sharedStrings, because we extracted the text from the `<si>` element
instead of the `<t>` element that had the attribute xml:space="preserve".

And for multi-line cells to be considered as such by Excel, they need
to have the style `wrapText="1"`.

Task: 4044862
Part-of: #5377
Signed-off-by: Rémi Rahir (rar) <[email protected]>
Signed-off-by: Adrien Minne (adrm) <[email protected]>
  • Loading branch information
hokolomopo committed Jan 6, 2025
1 parent 3627052 commit 3d0c96b
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 14 deletions.
4 changes: 2 additions & 2 deletions src/xlsx/conversion/sheet_conversion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
buildSheetLink,
largeMax,
markdownLink,
replaceNewLines,
splitReference,
toCartesian,
toXC,
Expand Down Expand Up @@ -118,9 +119,8 @@ function convertRows(
return rows;
}

/** Remove newlines (\n) in shared strings, We do not support them */
function convertSharedStrings(xlsxSharedStrings: string[]): string[] {
return xlsxSharedStrings.map((str) => str.replace(/\n/g, ""));
return xlsxSharedStrings.map(replaceNewLines);
}

function convertCells(
Expand Down
11 changes: 3 additions & 8 deletions src/xlsx/extraction/misc_extractor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,10 @@ export class XlsxMiscExtractor extends XlsxBaseExtractor {
{ parent: this.rootFile.file.xml, query: "si" },
(ssElement): string => {
// Shared string can either be a simple text, or a rich text (text with formatting, possibly in multiple parts)
if (ssElement.children[0].tagName === "t") {
return this.extractTextContent(ssElement) || "";
}
// We don't support rich text formatting, we'll only extract the text
else {
return this.mapOnElements({ parent: ssElement, query: "t" }, (textElement): string => {
return this.extractTextContent(textElement) || "";
}).join("");
}
return this.mapOnElements({ parent: ssElement, query: "t" }, (textElement): string => {
return this.extractTextContent(textElement) || "";
}).join("");
}
);
}
Expand Down
4 changes: 2 additions & 2 deletions src/xlsx/helpers/content_helpers.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { DEFAULT_FONT_SIZE } from "../../constants";
import { DEFAULT_FONT_SIZE, NEWLINE } from "../../constants";
import { deepEquals, splitReference, toUnboundedZone } from "../../helpers";
import {
ConditionalFormattingOperatorValues,
Expand Down Expand Up @@ -109,7 +109,7 @@ export function extractStyle(cell: ExcelCellData, data: WorkbookData): Extracted
vertical: style.verticalAlign
? V_ALIGNMENT_EXPORT_CONVERSION_MAP[style.verticalAlign]
: undefined,
wrapText: style.wrapping === "wrap" || undefined,
wrapText: style.wrapping === "wrap" || cell.content?.includes(NEWLINE) ? true : undefined,
},
};

Expand Down
Binary file modified tests/__xlsx__/xlsx_demo_data.xlsx
Binary file not shown.
20 changes: 20 additions & 0 deletions tests/xlsx/xlsx_export.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -745,6 +745,26 @@ describe("Test XLSX export", () => {
});
});

test("Multi-line cells are exported with text wrap", async () => {
const model = new Model();
setCellContent(model, "A1", "This is a\nmultiline cell");

const exportedXlsx = await exportPrettifiedXlsx(model);
const styleSheet = parseXML(
exportedXlsx.files.find((f) => f["contentType"] === "styles")!["content"]
);
const workSheet = parseXML(
exportedXlsx.files.find((f) => f["contentType"] === "sheet")!["content"]
);

const A1 = workSheet.querySelector("c[r='A1']")!;
expect(A1.getAttribute("s")).toBe("1");

const styles = styleSheet.querySelectorAll("xf");
expect(styles[1].getAttribute("applyAlignment")).toBe("1");
expect(styles[1].querySelector("alignment")!.getAttribute("wrapText")).toBe("1");
});

describe("formulas", () => {
beforeAll(() => {
functionRegistry.add("NOW", {
Expand Down
10 changes: 8 additions & 2 deletions tests/xlsx/xlsx_import.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -775,10 +775,16 @@ describe("Import xlsx data", () => {
});

describe("Misc tests", () => {
test("Newlines characters in strings are removed", () => {
test("Newlines characters in strings are preserved", () => {
const testSheet = getWorkbookSheet("jestMiscTest", convertedData)!;
const textWithNewLineInXLSX = testSheet.cells["A1"]?.content;
expect(textWithNewLineInXLSX).toEqual("This text have a newLine"); // newline should have been removed at import
expect(textWithNewLineInXLSX).toEqual("This text have\n a newLine");
});

test("White spaces are preserved", () => {
const testSheet = getWorkbookSheet("jestMiscTest", convertedData)!;
const textWithNewLineInXLSX = testSheet.cells["B1"]?.content;
expect(textWithNewLineInXLSX).toEqual(" with whitespace before");
});

test("Can hide gridLines", () => {
Expand Down

0 comments on commit 3d0c96b

Please sign in to comment.