Skip to content

Commit 5d13b0a

Browse files
sdip15fasdip15fa
authored
fix: ensure numbered list start property always present (#2241) (#2242)
1 parent afb0a93 commit 5d13b0a

File tree

4 files changed

+219
-5
lines changed

4 files changed

+219
-5
lines changed

‎packages/core/src/blocks/ListItem/NumberedListItem/block.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,9 @@ export const createNumberedListItemBlockSpec = createBlockSpec(
5151

5252
const defaultProps = parseDefaultProps(element);
5353

54-
if (element.previousElementSibling || startIndex === 1) {
55-
return defaultProps;
56-
}
57-
5854
return {
5955
...defaultProps,
60-
start: startIndex,
56+
start: element.previousElementSibling || startIndex === 1 ? undefined : startIndex,
6157
};
6258
}
6359

‎tests/src/unit/core/blocks/NumberedListItem.test.ts‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
import { describe, expect, it } from "vitest";
2+
import { createNumberedListItemBlockSpec } from "../../../../../packages/core/src/blocks/ListItem/NumberedListItem/block.js";
3+
4+
describe("NumberedListItem parse() method", () => {
5+
const blockSpec = createNumberedListItemBlockSpec();
6+
const parseFunc = blockSpec.implementation.parse;
7+
8+
if (!parseFunc) {
9+
throw new Error("parse function not found");
10+
}
11+
12+
it("should always return an object with 'start' property - first item, startIndex=1", () => {
13+
// Create mock DOM elements
14+
const li = document.createElement("li");
15+
const ol = document.createElement("ol");
16+
ol.setAttribute("start", "1");
17+
ol.appendChild(li);
18+
19+
const result = parseFunc(li);
20+
21+
// The parse function should return an object with 'start' property
22+
expect(result).toBeDefined();
23+
expect(result).toHaveProperty("start");
24+
expect(result?.start).toBeUndefined(); // Should be undefined for first item at index 1
25+
});
26+
27+
it("should always return an object with 'start' property - first item, startIndex=5", () => {
28+
const li = document.createElement("li");
29+
const ol = document.createElement("ol");
30+
ol.setAttribute("start", "5");
31+
ol.appendChild(li);
32+
33+
const result = parseFunc(li);
34+
35+
expect(result).toBeDefined();
36+
expect(result).toHaveProperty("start");
37+
expect(result?.start).toBe(5); // Should be 5 for first item at non-standard index
38+
});
39+
40+
it("should always return an object with 'start' property - subsequent item", () => {
41+
const li1 = document.createElement("li");
42+
const li2 = document.createElement("li");
43+
const ol = document.createElement("ol");
44+
ol.setAttribute("start", "1");
45+
ol.appendChild(li1);
46+
ol.appendChild(li2);
47+
48+
const result = parseFunc(li2);
49+
50+
expect(result).toBeDefined();
51+
expect(result).toHaveProperty("start");
52+
expect(result?.start).toBeUndefined(); // Subsequent items don't need explicit start
53+
});
54+
55+
it("should always return an object with 'start' property - subsequent item in list starting at 5", () => {
56+
const li1 = document.createElement("li");
57+
const li2 = document.createElement("li");
58+
const ol = document.createElement("ol");
59+
ol.setAttribute("start", "5");
60+
ol.appendChild(li1);
61+
ol.appendChild(li2);
62+
63+
const result = parseFunc(li2);
64+
65+
expect(result).toBeDefined();
66+
expect(result).toHaveProperty("start");
67+
expect(result?.start).toBeUndefined(); // Subsequent items get undefined
68+
});
69+
70+
it("regression test for issue #2241 - ensures 'start' property is always present", () => {
71+
// This test verifies the fix for issue #2241
72+
// The old code would return defaultProps (without 'start') for certain conditions
73+
// The new code always includes 'start' in the returned object
74+
75+
const testCases = [
76+
{ startAttr: "1", hasPreSibling: false, expectedStart: undefined },
77+
{ startAttr: "1", hasPreSibling: true, expectedStart: undefined },
78+
{ startAttr: "5", hasPreSibling: false, expectedStart: 5 },
79+
{ startAttr: "5", hasPreSibling: true, expectedStart: undefined },
80+
];
81+
82+
testCases.forEach(({ startAttr, hasPreSibling, expectedStart }) => {
83+
const li = document.createElement("li");
84+
const ol = document.createElement("ol");
85+
ol.setAttribute("start", startAttr);
86+
87+
if (hasPreSibling) {
88+
const li1 = document.createElement("li");
89+
ol.appendChild(li1);
90+
}
91+
92+
ol.appendChild(li);
93+
94+
const result = parseFunc(li);
95+
96+
// Critical assertion: 'start' property must ALWAYS be present
97+
expect(result).toHaveProperty("start");
98+
expect(result?.start).toBe(expectedStart);
99+
});
100+
});
101+
});

‎tests/src/unit/core/formatConversion/parse/__snapshots__/markdown/issue2241NumberedListStartProperty.json‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
[
2+
{
3+
"children": [],
4+
"content": [
5+
{
6+
"styles": {},
7+
"text": "First item",
8+
"type": "text",
9+
},
10+
],
11+
"id": "1",
12+
"props": {
13+
"backgroundColor": "default",
14+
"textAlignment": "left",
15+
"textColor": "default",
16+
},
17+
"type": "numberedListItem",
18+
},
19+
{
20+
"children": [],
21+
"content": [
22+
{
23+
"styles": {},
24+
"text": "Second item",
25+
"type": "text",
26+
},
27+
],
28+
"id": "2",
29+
"props": {
30+
"backgroundColor": "default",
31+
"textAlignment": "left",
32+
"textColor": "default",
33+
},
34+
"type": "numberedListItem",
35+
},
36+
{
37+
"children": [],
38+
"content": [
39+
{
40+
"styles": {},
41+
"text": "Third item",
42+
"type": "text",
43+
},
44+
],
45+
"id": "3",
46+
"props": {
47+
"backgroundColor": "default",
48+
"textAlignment": "left",
49+
"textColor": "default",
50+
},
51+
"type": "numberedListItem",
52+
},
53+
{
54+
"children": [],
55+
"content": [
56+
{
57+
"styles": {},
58+
"text": "List starting at 5",
59+
"type": "text",
60+
},
61+
],
62+
"id": "4",
63+
"props": {
64+
"backgroundColor": "default",
65+
"textAlignment": "left",
66+
"textColor": "default",
67+
},
68+
"type": "numberedListItem",
69+
},
70+
{
71+
"children": [],
72+
"content": [
73+
{
74+
"styles": {},
75+
"text": "Second item",
76+
"type": "text",
77+
},
78+
],
79+
"id": "5",
80+
"props": {
81+
"backgroundColor": "default",
82+
"textAlignment": "left",
83+
"textColor": "default",
84+
},
85+
"type": "numberedListItem",
86+
},
87+
{
88+
"children": [],
89+
"content": [
90+
{
91+
"styles": {},
92+
"text": "Third item",
93+
"type": "text",
94+
},
95+
],
96+
"id": "6",
97+
"props": {
98+
"backgroundColor": "default",
99+
"textAlignment": "left",
100+
"textColor": "default",
101+
},
102+
"type": "numberedListItem",
103+
},
104+
]

‎tests/src/unit/core/formatConversion/parse/parseTestInstances.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1111,4 +1111,17 @@ Regular paragraph`,
11111111
},
11121112
executeTest: testParseMarkdown,
11131113
},
1114+
{
1115+
testCase: {
1116+
name: "issue2241NumberedListStartProperty",
1117+
content: `1. First item
1118+
2. Second item
1119+
3. Third item
1120+
1121+
5. List starting at 5
1122+
6. Second item
1123+
7. Third item`,
1124+
},
1125+
executeTest: testParseMarkdown,
1126+
},
11141127
];

0 commit comments

Comments
 (0)