Skip to content

Commit 207b1c4

Browse files
committed
[FIX] xlsx: import new lines in sharedStrings
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: #5408 Signed-off-by: Rémi Rahir (rar) <[email protected]> Signed-off-by: Adrien Minne (adrm) <[email protected]>
1 parent a071ee0 commit 207b1c4

File tree

7 files changed

+40
-15
lines changed

7 files changed

+40
-15
lines changed

src/xlsx/conversion/sheet_conversion.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import {
22
buildSheetLink,
33
largeMax,
44
markdownLink,
5+
replaceNewLines,
56
splitReference,
67
toCartesian,
78
toXC,
@@ -120,9 +121,8 @@ function convertRows(
120121
return rows;
121122
}
122123

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

128128
function convertCells(

src/xlsx/extraction/misc_extractor.ts

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,10 @@ export class XlsxMiscExtractor extends XlsxBaseExtractor {
3939
{ parent: this.rootFile.file.xml, query: "si" },
4040
(ssElement): string => {
4141
// Shared string can either be a simple text, or a rich text (text with formatting, possibly in multiple parts)
42-
if (ssElement.children[0].tagName === "t") {
43-
return this.extractTextContent(ssElement) || "";
44-
}
4542
// We don't support rich text formatting, we'll only extract the text
46-
else {
47-
return this.mapOnElements({ parent: ssElement, query: "t" }, (textElement): string => {
48-
return this.extractTextContent(textElement) || "";
49-
}).join("");
50-
}
43+
return this.mapOnElements({ parent: ssElement, query: "t" }, (textElement): string => {
44+
return this.extractTextContent(textElement) || "";
45+
}).join("");
5146
}
5247
);
5348
}

src/xlsx/functions/worksheet.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,10 @@ export function addRows(
9797
const attributes: XMLAttributes = [["r", xc]];
9898

9999
// style
100-
const id = normalizeStyle(construct, extractStyle(data, styleId, formatId, borderId));
100+
const id = normalizeStyle(
101+
construct,
102+
extractStyle(data, content, styleId, formatId, borderId)
103+
);
101104
// don't add style if default
102105
if (id) {
103106
attributes.push(["s", id]);

src/xlsx/helpers/content_helpers.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { DEFAULT_FONT_SIZE } from "../../constants";
1+
import { DEFAULT_FONT_SIZE, NEWLINE } from "../../constants";
22
import { deepEquals, splitReference, toUnboundedZone } from "../../helpers";
33
import {
44
ConditionalFormattingOperatorValues,
@@ -91,6 +91,7 @@ export function convertWidthFromExcel(width: number | undefined): number | undef
9191

9292
export function extractStyle(
9393
data: WorkbookData,
94+
content: string | undefined,
9495
styleId: number | undefined,
9596
formatId: number | undefined,
9697
borderId: number | undefined
@@ -116,7 +117,7 @@ export function extractStyle(
116117
vertical: style.verticalAlign
117118
? V_ALIGNMENT_EXPORT_CONVERSION_MAP[style.verticalAlign]
118119
: undefined,
119-
wrapText: style.wrapping === "wrap" || undefined,
120+
wrapText: style.wrapping === "wrap" || content?.includes(NEWLINE) ? true : undefined,
120121
},
121122
};
122123

tests/__xlsx__/xlsx_demo_data.xlsx

-756 Bytes
Binary file not shown.

tests/xlsx/xlsx_export.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -832,6 +832,26 @@ describe("Test XLSX export", () => {
832832
});
833833
});
834834

835+
test("Multi-line cells are exported with text wrap", async () => {
836+
const model = new Model();
837+
setCellContent(model, "A1", "This is a\nmultiline cell");
838+
839+
const exportedXlsx = await exportPrettifiedXlsx(model);
840+
const styleSheet = parseXML(
841+
exportedXlsx.files.find((f) => f["contentType"] === "styles")!["content"]
842+
);
843+
const workSheet = parseXML(
844+
exportedXlsx.files.find((f) => f["contentType"] === "sheet")!["content"]
845+
);
846+
847+
const A1 = workSheet.querySelector("c[r='A1']")!;
848+
expect(A1.getAttribute("s")).toBe("1");
849+
850+
const styles = styleSheet.querySelectorAll("xf");
851+
expect(styles[1].getAttribute("applyAlignment")).toBe("1");
852+
expect(styles[1].querySelector("alignment")!.getAttribute("wrapText")).toBe("1");
853+
});
854+
835855
describe("formulas", () => {
836856
beforeAll(() => {
837857
functionRegistry.add("NOW", {

tests/xlsx/xlsx_import.test.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -865,10 +865,16 @@ describe("Import xlsx data", () => {
865865
});
866866

867867
describe("Misc tests", () => {
868-
test("Newlines characters in strings are removed", () => {
868+
test("Newlines characters in strings are preserved", () => {
869869
const testSheet = getWorkbookSheet("jestMiscTest", convertedData)!;
870870
const textWithNewLineInXLSX = testSheet.cells["A1"];
871-
expect(textWithNewLineInXLSX).toEqual("This text have a newLine"); // newline should have been removed at import
871+
expect(textWithNewLineInXLSX).toEqual("This text have\n a newLine");
872+
});
873+
874+
test("White spaces are preserved", () => {
875+
const testSheet = getWorkbookSheet("jestMiscTest", convertedData)!;
876+
const textWithNewLineInXLSX = testSheet.cells["B1"];
877+
expect(textWithNewLineInXLSX).toEqual(" with whitespace before");
872878
});
873879

874880
test("Can hide gridLines", () => {

0 commit comments

Comments
 (0)