Skip to content

[lexical-list] Feature: Enforce strict list indentation#7420

Merged
etrepum merged 7 commits intofacebook:mainfrom
dineug:list-strict-indent
Apr 5, 2025
Merged

[lexical-list] Feature: Enforce strict list indentation#7420
etrepum merged 7 commits intofacebook:mainfrom
dineug:list-strict-indent

Conversation

@dineug
Copy link
Copy Markdown
Contributor

@dineug dineug commented Apr 3, 2025

Description

We are adding a strict indent feature for lists.
This feature helps in creating an accurate Markdown converter.
Additionally, it can be optionally enabled as needed.

Test plan

Before

list-before.mp4

After

list-after.mp4

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 3, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
lexical ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 4, 2025 4:10pm
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 4, 2025 4:10pm

etrepum
etrepum previously approved these changes Apr 3, 2025
Copy link
Copy Markdown
Collaborator

@etrepum etrepum left a comment

Choose a reason for hiding this comment

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

This looks reasonable to me, although I think the transform algorithm could probably be optimized if a bit more state was maintained particularly in the recursive case

@dineug
Copy link
Copy Markdown
Contributor Author

dineug commented Apr 3, 2025

I have incorporated the code review! @etrepum

Comment on lines +186 to +206
const $processListWithStrictIndent = (listNode: ListNode): void => {
const queue: ListNode[] = [listNode];

while (queue.length > 0) {
const node = queue.shift();
if (!$isListNode(node)) {
continue;
}

for (const child of node.getChildren()) {
if ($isListItemNode(child)) {
$formatListIndentStrict(child);

const firstChild = child.getFirstChild();
if ($isListNode(firstChild)) {
queue.push(firstChild);
}
}
}
}
};
Copy link
Copy Markdown
Collaborator

@etrepum etrepum Apr 4, 2025

Choose a reason for hiding this comment

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

This isn't really what I meant by preserving information in the recursive case. For example, startingListItemNode can be computed here with much less redundant work in each $formatListIndentStrict. I also don't think it needs to go very deep because I think modifying a list's indentation can only break the indent invariant for itself or its list children so the recursion can stop if no modifications are made (or really even if they are, since the transform is going to run again anyway, but preemptive modifications to further descendants would do fewer rounds of transforms).

Copy link
Copy Markdown
Collaborator

@etrepum etrepum left a comment

Choose a reason for hiding this comment

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

As before I think this looks fine for now, it's up to you if you'd like to optimize the algorithm

@dineug
Copy link
Copy Markdown
Contributor Author

dineug commented Apr 4, 2025

Thanks for the detailed explanation! @etrepum
I think it should be okay to merge now.

@etrepum etrepum added this pull request to the merge queue Apr 5, 2025
Merged via the queue into facebook:main with commit b065079 Apr 5, 2025
36 checks passed
@dineug dineug deleted the list-strict-indent branch April 7, 2025 14:07
@etrepum etrepum mentioned this pull request Apr 7, 2025
GermanJablo added a commit to payloadcms/payload that referenced this pull request Sep 3, 2025
Fixes #13386

Below I write a clarification to copy and paste into the release note,
based on our latest upgrade of Lexical [in
v3.29.0](https://github.com/payloadcms/payload/releases/tag/v3.29.0).

## Important
This release upgrades the lexical dependency from 0.28.0 to 0.34.0.

If you installed lexical manually, update it to 0.34.0. Installing
lexical manually is not recommended, as it may break between updates,
and our re-exported versions should be used. See the [yellow banner
box](https://payloadcms.com/docs/rich-text/custom-features) for details.

If you still encounter richtext-lexical errors, do the following, in
this order:

- Delete node_modules
- Delete your lockfile (e.g. pnpm-lock.json)
- Reinstall your dependencies (e.g. pnpm install)

### Lexical Breaking Changes

The following Lexical releases describe breaking changes. We recommend
reading them if you're using Lexical APIs directly
(`@payloadcms/richtext-lexical/lexical/*`).

- [v.0.33.0](https://github.com/facebook/lexical/releases/tag/v0.33.0)
- [v.0.30.0](https://github.com/facebook/lexical/releases/tag/v0.30.0)
- [v.0.29.0](https://github.com/facebook/lexical/releases/tag/v0.29.0)

___

TODO:
- [x] facebook/lexical#7719
- [x] facebook/lexical#7362
- [x] facebook/lexical#7707
- [x] facebook/lexical#7388
- [x] facebook/lexical#7357
- [x] facebook/lexical#7352
- [x] facebook/lexical#7472
- [x] facebook/lexical#7556
- [x] facebook/lexical#7417
- [x] facebook/lexical#1036
- [x] facebook/lexical#7509
- [x] facebook/lexical#7693
- [x] facebook/lexical#7408
- [x] facebook/lexical#7450
- [x] facebook/lexical#7415
- [x] facebook/lexical#7368
- [x] facebook/lexical#7372
- [x] facebook/lexical#7572
- [x] facebook/lexical#7558
- [x] facebook/lexical#7613
- [x] facebook/lexical#7405
- [x] facebook/lexical#7420
- [x] facebook/lexical#7662

---
- To see the specific tasks where the Asana app for GitHub is being
used, see below:
  - https://app.asana.com/0/0/1211202581885926
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. extended-tests Run extended e2e tests on a PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants