Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[lexical-list] Feature: Continuous list numbering #7059

Closed
wants to merge 5 commits into from
Closed

[lexical-list] Feature: Continuous list numbering #7059

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b3b6ba7
add continuePreviousNumbering property
patrick-atticus Jan 14, 2025
d12b421
add continuous numbering option to playground UI
patrick-atticus Jan 14, 2025
ca526fb
tests
patrick-atticus Jan 15, 2025
cf4aa2e
handle deleted list
patrick-atticus Jan 17, 2025
9b68d8e
prevent continued numbers in nested lists
patrick-atticus Jan 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 67 additions & 4 deletions packages/lexical-list/src/LexicalListNode.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export type SerializedListNode = Spread<
listType: ListType;
start: number;
tag: ListNodeTagType;
continuePreviousNumbering?: boolean;
},
SerializedElementNode
>;
Expand All @@ -59,6 +60,8 @@ export class ListNode extends ElementNode {
__start: number;
/** @internal */
__listType: ListType;
/** @internal */
__continuePreviousNumbering: boolean;

static getType(): string {
return 'list';
Expand All @@ -67,15 +70,26 @@ export class ListNode extends ElementNode {
static clone(node: ListNode): ListNode {
const listType = node.__listType || TAG_TO_LIST_TYPE[node.__tag];

return new ListNode(listType, node.__start, node.__key);
return new ListNode(
listType,
node.__start,
node.__continuePreviousNumbering,
node.__key,
);
}

constructor(listType: ListType = 'number', start: number = 1, key?: NodeKey) {
constructor(
listType: ListType = 'number',
start: number = 1,
continuePreviousNumbering: boolean = false,
key?: NodeKey,
) {
super(key);
const _listType = TAG_TO_LIST_TYPE[listType] || listType;
this.__listType = _listType;
this.__tag = _listType === 'number' ? 'ol' : 'ul';
this.__start = start;
this.__continuePreviousNumbering = continuePreviousNumbering;
}

getTag(): ListNodeTagType {
Expand Down Expand Up @@ -103,6 +117,19 @@ export class ListNode extends ElementNode {
return self;
}

getContinuePreviousNumbering(): boolean {
return this.__continuePreviousNumbering;
}

setContinuePreviousNumbering(value: boolean): this {
const self = this.getWritable();
self.__continuePreviousNumbering = value;
if (!value) {
self.__start = 1;
}
return self;
}

// View

createDOM(config: EditorConfig, _editor?: LexicalEditor): HTMLElement {
Expand All @@ -124,6 +151,10 @@ export class ListNode extends ElementNode {
return true;
}

if (prevNode.__start !== this.__start) {
return true;
}

$setListThemeClassNames(dom, config.theme, this);

return false;
Expand Down Expand Up @@ -158,7 +189,8 @@ export class ListNode extends ElementNode {
return super
.updateFromJSON(serializedNode)
.setListType(serializedNode.listType)
.setStart(serializedNode.start);
.setStart(serializedNode.start)
.setContinuePreviousNumbering(!!serializedNode.continuePreviousNumbering);
}

exportDOM(editor: LexicalEditor): DOMExportOutput {
Expand All @@ -179,6 +211,7 @@ export class ListNode extends ElementNode {
exportJSON(): SerializedListNode {
return {
...super.exportJSON(),
continuePreviousNumbering: this.getContinuePreviousNumbering(),
listType: this.getListType(),
start: this.getStart(),
tag: this.getTag(),
Expand Down Expand Up @@ -223,6 +256,33 @@ export class ListNode extends ElementNode {
extractWithChild(child: LexicalNode): boolean {
return $isListItemNode(child);
}

updateStartFromPreviousList(previousList: ListNode | null) {
if (
this.__listType !== 'number' ||
!this.__continuePreviousNumbering ||
this.getParentOrThrow().getType() === 'listitem'
) {
// Numbering update not applicable
return;
}

if (!previousList && this.__start !== 1) {
// Previous list was deleted, reset numbering
this.setStart(1);
return;
}

if (previousList && previousList.getListType() === 'number') {
// Continue previous numbering
const listItemCount = previousList.getChildrenSize();
const lastNumber = previousList.getStart() + listItemCount - 1;
const nextNumber = lastNumber + 1;
if (this.__start !== nextNumber && lastNumber > 0) {
this.setStart(nextNumber);
}
}
}
}

function $setListThemeClassNames(
Expand Down Expand Up @@ -363,8 +423,11 @@ const TAG_TO_LIST_TYPE: Record<string, ListType> = {
export function $createListNode(
listType: ListType = 'number',
start = 1,
continuePreviousNumbering = false,
): ListNode {
return $applyNodeReplacement(new ListNode(listType, start));
return $applyNodeReplacement(
new ListNode(listType, start, continuePreviousNumbering),
);
}

/**
Expand Down
27 changes: 26 additions & 1 deletion packages/lexical-list/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import type {SerializedListItemNode} from './LexicalListItemNode';
import type {ListType, SerializedListNode} from './LexicalListNode';
import type {LexicalCommand, LexicalEditor} from 'lexical';

import {mergeRegister} from '@lexical/utils';
import {$dfs, mergeRegister} from '@lexical/utils';
import {
COMMAND_PRIORITY_LOW,
createCommand,
Expand Down Expand Up @@ -97,10 +97,35 @@ export function registerList(editor: LexicalEditor): () => void {
},
COMMAND_PRIORITY_LOW,
),
editor.registerNodeTransform(ListNode, $updateListNumbering),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried to use node transforms where possible, but ran in to a bug with this approach (see video).

I know mutation listeners should not trigger additional updates, but one is needed to handle deleted list events. Would it be preferable to use it for all updates in this case?

CleanShot.2025-01-17.at.11.39.22-converted.mp4

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating from an update listener is the same anti-pattern as updating from a mutation listener. The reason for an update should always have something to do with something external to the editor's state.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding deletion I don't think there's a good way to react to a deletion that doesn't cause an update cascade, unattached nodes are ignored from transforms, something else would have to be added to the editor API in order to support this, whether that's something like an update listener that runs before transforms or a registerTransform-like API that executes in the cases when nodes are no longer attached.

I think a solution that depends more on DOM manipulation and CSS counters can be achieved without something like this though, shouldn't really even need a transform to support it.

editor.registerMutationListener(ListNode, (mutations) => {
let shouldUpdate = false;
for (const [, type] of mutations) {
if (type === 'destroyed') {
shouldUpdate = true;
break;
}
}

if (shouldUpdate) {
editor.update($updateListNumbering);
}
Comment on lines +110 to +112
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the update cascade anti-pattern. Mutation listeners are intended to be used for updating DOM or sending state out of Lexical but it's not a good idea to trigger another update in here. Code like this is not good for performance and can cause visual artifacts (two reconciliations for each related update) and and infinite updates when there's a bug.

}),
);
return removeListener;
}

function $updateListNumbering() {
let previousList: ListNode | null = null;
$dfs().forEach(({node}) => {
if (!$isListNode(node) || node.getListType() !== 'number') {
return;
}
node.updateStartFromPreviousList(previousList);
previousList = node;
});
}

/**
* @deprecated use {@link $insertList} from an update or command listener.
*
Expand Down
116 changes: 116 additions & 0 deletions packages/lexical-playground/__tests__/e2e/List.spec.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1947,3 +1947,119 @@ test.describe.parallel('Nested List', () => {
);
});
});

test.describe('List Continue Previous Numbering', () => {
test.beforeEach(({isCollab, page}) => initialize({isCollab, page}));

test('continues numbering from previous list when continuePreviousNumbering is true', async ({
page,
}) => {
await focusEditor(page);

// Create first numbered list
await toggleNumberedList(page);
await page.keyboard.type('First');
await page.keyboard.press('Enter');
await page.keyboard.type('Second');

// Exit list and add paragraph
await page.keyboard.press('Enter');
await page.keyboard.press('Enter');
await page.keyboard.type('Separating paragraph');
await page.keyboard.press('Enter');

// Create second numbered list
await toggleNumberedList(page);
await page.keyboard.type('Fourth');
await page.keyboard.press('Enter');
await page.keyboard.type('Fifth');

await assertHTML(
page,
html`
<ol class="PlaygroundEditorTheme__ol1">
<li
class="PlaygroundEditorTheme__listItem PlaygroundEditorTheme__ltr"
dir="ltr"
value="1">
<span data-lexical-text="true">First</span>
</li>
<li
class="PlaygroundEditorTheme__listItem PlaygroundEditorTheme__ltr"
dir="ltr"
value="2">
<span data-lexical-text="true">Second</span>
</li>
</ol>
<p
class="PlaygroundEditorTheme__paragraph PlaygroundEditorTheme__ltr"
dir="ltr">
<span data-lexical-text="true">Separating paragraph</span>
</p>
<ol class="PlaygroundEditorTheme__ol1">
<li
class="PlaygroundEditorTheme__listItem PlaygroundEditorTheme__ltr"
dir="ltr"
value="1">
<span data-lexical-text="true">Fourth</span>
</li>
<li
class="PlaygroundEditorTheme__listItem PlaygroundEditorTheme__ltr"
dir="ltr"
value="2">
<span data-lexical-text="true">Fifth</span>
</li>
</ol>
`,
);

// Partially select text in second list to trigger floating menu
await page.keyboard.press('Shift+ArrowLeft');

// Click continue numbering button in floating menu
await click(
page,
'button[aria-label="Continue numbering from previous list"]',
);

// Verify the second list now continues numbering
await assertHTML(
page,
html`
<ol class="PlaygroundEditorTheme__ol1">
<li
class="PlaygroundEditorTheme__listItem PlaygroundEditorTheme__ltr"
dir="ltr"
value="1">
<span data-lexical-text="true">First</span>
</li>
<li
class="PlaygroundEditorTheme__listItem PlaygroundEditorTheme__ltr"
dir="ltr"
value="2">
<span data-lexical-text="true">Second</span>
</li>
</ol>
<p
class="PlaygroundEditorTheme__paragraph PlaygroundEditorTheme__ltr"
dir="ltr">
<span data-lexical-text="true">Separating paragraph</span>
</p>
<ol class="PlaygroundEditorTheme__ol1" start="3">
<li
class="PlaygroundEditorTheme__listItem PlaygroundEditorTheme__ltr"
dir="ltr"
value="3">
<span data-lexical-text="true">Fourth</span>
</li>
<li
class="PlaygroundEditorTheme__listItem PlaygroundEditorTheme__ltr"
dir="ltr"
value="4">
<span data-lexical-text="true">Fifth</span>
</li>
</ol>
`,
);
});
});
3 changes: 2 additions & 1 deletion packages/lexical-playground/src/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -884,7 +884,8 @@ i.page-break,
}

.icon.numbered-list,
.icon.number {
.icon.number,
i.numbered-list {
background-image: url(images/icons/list-ol.svg);
}

Expand Down
Loading
Loading