Skip to content

Commit de131ce

Browse files
JiuqingSongJiuqingSong
authored
Fix 230556 (#3223)
* Fix 230556 * improve * fix build and improve
1 parent 3a02a5a commit de131ce

File tree

17 files changed

+290
-63
lines changed

17 files changed

+290
-63
lines changed

‎.vscode/settings.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
"builddemo",
3333
"builddoc",
3434
"Calibri",
35+
"cellspacing",
3536
"checkdep",
3637
"colspan",
3738
"compositionend",

‎packages/roosterjs-content-model-api/lib/publicApi/table/insertTable.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,10 @@ import {
88
mergeModel,
99
normalizeTable,
1010
setSelection,
11+
MIN_ALLOWED_TABLE_CELL_WIDTH,
1112
} from 'roosterjs-content-model-dom';
1213
import type {
14+
ContentModelTable,
1315
ContentModelTableFormat,
1416
IEditor,
1517
TableMetadataFormat,
@@ -45,6 +47,7 @@ export function insertTable(
4547
}
4648

4749
normalizeTable(table, editor.getPendingFormat() || insertPosition.marker.format);
50+
initCellWidth(table);
4851

4952
adjustTableIndentation(insertPosition, table);
5053

@@ -74,3 +77,25 @@ export function insertTable(
7477
}
7578
);
7679
}
80+
81+
function initCellWidth(table: ContentModelTable) {
82+
const columns = Math.max(...table.rows.map(row => row.cells.length));
83+
84+
for (let i = 0; i < columns; i++) {
85+
if (table.widths[i] === undefined) {
86+
table.widths[i] = getTableCellWidth(columns);
87+
} else if (table.widths[i] < MIN_ALLOWED_TABLE_CELL_WIDTH) {
88+
table.widths[i] = MIN_ALLOWED_TABLE_CELL_WIDTH;
89+
}
90+
}
91+
}
92+
93+
function getTableCellWidth(columns: number): number {
94+
if (columns <= 4) {
95+
return 120;
96+
} else if (columns <= 6) {
97+
return 100;
98+
} else {
99+
return 70;
100+
}
101+
}

‎packages/roosterjs-content-model-dom/lib/formatHandlers/defaultFormatHandlers.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { htmlAlignFormatHandler } from './block/htmlAlignFormatHandler';
1616
import { idFormatHandler } from './common/idFormatHandler';
1717
import { imageStateFormatHandler } from './segment/imageStateFormatHandler';
1818
import { italicFormatHandler } from './segment/italicFormatHandler';
19+
import { legacyTableBorderFormatHandler } from './table/legacyTableBorderFormatHandler';
1920
import { letterSpacingFormatHandler } from './segment/letterSpacingFormatHandler';
2021
import { lineHeightFormatHandler } from './block/lineHeightFormatHandler';
2122
import { linkFormatHandler } from './segment/linkFormatHandler';
@@ -72,6 +73,7 @@ const defaultFormatHandlerMap: FormatHandlers = {
7273
id: idFormatHandler,
7374
imageState: imageStateFormatHandler,
7475
italic: italicFormatHandler,
76+
legacyTableBorder: legacyTableBorderFormatHandler,
7577
letterSpacing: letterSpacingFormatHandler,
7678
lineHeight: lineHeightFormatHandler,
7779
link: linkFormatHandler,
@@ -183,7 +185,7 @@ export const defaultFormatKeysPerCategory: {
183185
'direction',
184186
'role',
185187
],
186-
tableBorder: ['borderBox', 'tableSpacing'],
188+
tableBorder: ['borderBox', 'tableSpacing', 'legacyTableBorder'],
187189
tableCellBorder: ['borderBox'],
188190
image: [
189191
'id',

‎packages/roosterjs-content-model-dom/lib/formatHandlers/table/legacyTableBorderFormatHandler.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import type { FormatHandler } from '../FormatHandler';
2+
import type { LegacyTableBorderFormat } from 'roosterjs-content-model-types';
3+
4+
/**
5+
* @internal
6+
*/
7+
export const legacyTableBorderFormatHandler: FormatHandler<LegacyTableBorderFormat> = {
8+
parse: (format, element) => {
9+
const border = element.getAttribute('border');
10+
const cellSpacing = element.getAttribute('cellspacing');
11+
const cellpadding = element.getAttribute('cellpadding');
12+
13+
if (border) {
14+
format.legacyTableBorder = border;
15+
}
16+
17+
if (cellSpacing) {
18+
format.cellSpacing = cellSpacing;
19+
}
20+
21+
if (cellpadding) {
22+
format.cellPadding = cellpadding;
23+
}
24+
},
25+
26+
apply: (format, element) => {
27+
if (format.legacyTableBorder) {
28+
element.setAttribute('border', format.legacyTableBorder);
29+
}
30+
31+
if (format.cellSpacing) {
32+
element.setAttribute('cellspacing', format.cellSpacing);
33+
}
34+
35+
if (format.cellPadding) {
36+
element.setAttribute('cellpadding', format.cellPadding);
37+
}
38+
},
39+
};

‎packages/roosterjs-content-model-dom/lib/formatHandlers/table/tableSpacingFormatHandler.ts‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import type { SpacingFormat } from 'roosterjs-content-model-types';
33

44
const BorderCollapsed = 'collapse';
55
const BorderSeparate = 'separate';
6-
const CellPadding = 'cellPadding';
76

87
/**
98
* @internal
@@ -12,11 +11,6 @@ export const tableSpacingFormatHandler: FormatHandler<SpacingFormat> = {
1211
parse: (format, element) => {
1312
if (element.style.borderCollapse == BorderCollapsed) {
1413
format.borderCollapse = true;
15-
} else {
16-
const cellPadding = element.getAttribute(CellPadding);
17-
if (cellPadding) {
18-
format.borderCollapse = true;
19-
}
2014
}
2115

2216
if (element.style.borderCollapse == BorderSeparate) {

‎packages/roosterjs-content-model-dom/lib/modelApi/editing/normalizeTable.ts‎

Lines changed: 17 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export const MIN_ALLOWED_TABLE_CELL_HEIGHT: number = 22;
2323
* Normalize a Content Model table, make sure:
2424
* 1. Fist cells are not spanned
2525
* 2. Only first column and row can have headers
26-
* 3. All cells have content and width
26+
* 3. All cells have content
2727
* 4. Table and table row have correct width/height
2828
* 5. Spanned cell has no child blocks
2929
* 6. default format is correctly applied
@@ -36,10 +36,11 @@ export function normalizeTable(
3636
) {
3737
const table = mutateBlock(readonlyTable);
3838

39-
// Always collapse border and use border box for table in roosterjs to make layout simpler
39+
// Collapse border and use border box for table in roosterjs to make layout simpler
40+
// But if this is a legacy style table (table with deprecated border attributes), we should not change its border model
4041
const format = table.format;
4142

42-
if (!format.borderCollapse || !format.useBorderBox) {
43+
if (!format.cellSpacing && !format.cellPadding && !format.legacyTableBorder) {
4344
format.borderCollapse = true;
4445
format.useBorderBox = true;
4546
}
@@ -84,16 +85,6 @@ export function normalizeTable(
8485
}
8586
});
8687

87-
const columns = Math.max(...table.rows.map(row => row.cells.length));
88-
89-
for (let i = 0; i < columns; i++) {
90-
if (table.widths[i] === undefined) {
91-
table.widths[i] = getTableCellWidth(columns);
92-
} else if (table.widths[i] < MIN_ALLOWED_TABLE_CELL_WIDTH) {
93-
table.widths[i] = MIN_ALLOWED_TABLE_CELL_WIDTH;
94-
}
95-
}
96-
9788
// Move blocks from spanned cell to its main cell if any,
9889
// and remove rows/columns if all cells in it are spanned
9990
const colCount = table.rows[0]?.cells.length || 0;
@@ -109,11 +100,19 @@ export function normalizeTable(
109100

110101
if (table.rows.every(row => row.cells[colIndex]?.spanLeft)) {
111102
table.rows.forEach(row => row.cells.splice(colIndex, 1));
112-
table.widths.splice(
113-
colIndex - 1,
114-
2,
115-
table.widths[colIndex - 1] + table.widths[colIndex]
116-
);
103+
104+
if (
105+
typeof table.widths[colIndex] === 'number' &&
106+
typeof table.widths[colIndex - 1] === 'number'
107+
) {
108+
table.widths.splice(
109+
colIndex - 1,
110+
2,
111+
table.widths[colIndex - 1] + table.widths[colIndex]
112+
);
113+
} else {
114+
table.widths.splice(colIndex, 1);
115+
}
117116
}
118117
}
119118

@@ -134,16 +133,6 @@ export function normalizeTable(
134133
}
135134
}
136135

137-
function getTableCellWidth(columns: number): number {
138-
if (columns <= 4) {
139-
return 120;
140-
} else if (columns <= 6) {
141-
return 100;
142-
} else {
143-
return 70;
144-
}
145-
}
146-
147136
function tryMoveBlocks(
148137
targetCell: ReadonlyContentModelTableCell,
149138
sourceCell: ReadonlyContentModelTableCell

‎packages/roosterjs-content-model-dom/test/endToEndTest.ts‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2539,4 +2539,57 @@ describe('End to end test for DOM => Model => DOM/TEXT', () => {
25392539
'<dl><dt>term</dt><dd>definition</dd></dl>'
25402540
);
25412541
});
2542+
2543+
it('Table with border, cellspacing and padding', () => {
2544+
runTest(
2545+
'<table border="1" cellspacing="2" cellpadding="3"><tr><td>test</td></tr></table>',
2546+
{
2547+
blockGroupType: 'Document',
2548+
blocks: [
2549+
{
2550+
blockType: 'Table',
2551+
rows: [
2552+
{
2553+
height: 0,
2554+
cells: [
2555+
{
2556+
spanAbove: false,
2557+
spanLeft: false,
2558+
isHeader: false,
2559+
blockGroupType: 'TableCell',
2560+
blocks: [
2561+
{
2562+
isImplicit: true,
2563+
segments: [
2564+
{
2565+
text: 'test',
2566+
segmentType: 'Text',
2567+
format: {},
2568+
},
2569+
],
2570+
blockType: 'Paragraph',
2571+
format: {},
2572+
},
2573+
],
2574+
format: {},
2575+
dataset: {},
2576+
},
2577+
],
2578+
format: {},
2579+
},
2580+
],
2581+
format: {
2582+
legacyTableBorder: '1',
2583+
cellSpacing: '2',
2584+
cellPadding: '3',
2585+
},
2586+
dataset: {},
2587+
widths: [],
2588+
},
2589+
],
2590+
},
2591+
'test',
2592+
'<table border="1" cellspacing="2" cellpadding="3"><tbody><tr><td>test</td></tr></tbody></table>'
2593+
);
2594+
});
25422595
});

‎packages/roosterjs-content-model-dom/test/formatHandlers/table/legacyTableBorderFormatHandlerTest.ts‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import { legacyTableBorderFormatHandler } from '../../../lib/formatHandlers/table/legacyTableBorderFormatHandler';
2+
import type {
3+
DomToModelContext,
4+
LegacyTableBorderFormat,
5+
ModelToDomContext,
6+
} from 'roosterjs-content-model-types';
7+
8+
describe('legacyTableBorderFormatHandler.parse', () => {
9+
let format: LegacyTableBorderFormat;
10+
let table: HTMLTableElement;
11+
let context: DomToModelContext;
12+
13+
beforeEach(() => {
14+
format = {};
15+
table = document.createElement('table');
16+
context = {} as any;
17+
});
18+
19+
it('No border attribute', () => {
20+
legacyTableBorderFormatHandler.parse(format, table, context, {});
21+
expect(format.legacyTableBorder).toBeUndefined();
22+
});
23+
24+
it('With border attribute', () => {
25+
table.setAttribute('border', '1');
26+
legacyTableBorderFormatHandler.parse(format, table, context, {});
27+
expect(format.legacyTableBorder).toBe('1');
28+
});
29+
30+
it('With cellspacing attribute', () => {
31+
table.setAttribute('cellspacing', '2');
32+
legacyTableBorderFormatHandler.parse(format, table, context, {});
33+
expect(format.cellSpacing).toBe('2');
34+
});
35+
36+
it('With cellpadding attribute', () => {
37+
table.setAttribute('cellpadding', '3');
38+
legacyTableBorderFormatHandler.parse(format, table, context, {});
39+
expect(format.cellPadding).toBe('3');
40+
});
41+
});
42+
43+
describe('legacyTableBorderFormatHandler.apply', () => {
44+
let format: LegacyTableBorderFormat;
45+
let table: HTMLTableElement;
46+
let context: ModelToDomContext;
47+
48+
beforeEach(() => {
49+
format = {};
50+
table = document.createElement('table');
51+
context = {} as any;
52+
});
53+
54+
it('No border format', () => {
55+
legacyTableBorderFormatHandler.apply(format, table, context);
56+
expect(table.getAttribute('border')).toBeNull();
57+
});
58+
59+
it('With border format', () => {
60+
format.legacyTableBorder = '2';
61+
legacyTableBorderFormatHandler.apply(format, table, context);
62+
expect(table.getAttribute('border')).toBe('2');
63+
});
64+
65+
it('With cellspacing format', () => {
66+
format.cellSpacing = '4';
67+
legacyTableBorderFormatHandler.apply(format, table, context);
68+
expect(table.getAttribute('cellspacing')).toBe('4');
69+
});
70+
71+
it('With cellpadding format', () => {
72+
format.cellPadding = '5';
73+
legacyTableBorderFormatHandler.apply(format, table, context);
74+
expect(table.getAttribute('cellpadding')).toBe('5');
75+
});
76+
});

‎packages/roosterjs-content-model-dom/test/formatHandlers/table/tableSpacingFormatHandlerTest.ts‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,6 @@ describe('tableSpacingFormatHandler.parse', () => {
3030
tableSpacingFormatHandler.parse(format, div, context, {});
3131
expect(format).toEqual({ borderSeparate: true });
3232
});
33-
34-
it('Set border collapsed if element contains cellpadding attribute', () => {
35-
div.setAttribute('cellPadding', '0');
36-
tableSpacingFormatHandler.parse(format, div, context, {});
37-
expect(format).toEqual({ borderCollapse: true });
38-
});
3933
});
4034

4135
describe('tableSpacingFormatHandler.apply', () => {

0 commit comments

Comments
 (0)