Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
PR Analysis Report
Generated by PR Enrichment workflow | View current CI run |
…re-mark it With Markdown shortcuts on, a block marker typed at the start of a list item now re-marks the item and keeps its text. `- `, `1. `, or `[ ] ` switches the item's list between bullets, numbers, and tasks, and `# ` or `> ` takes the item out of its list as a heading or a quote, with the rest of the list continuing after it. Each change is one undo step, back to the marker as typed. Lexical's shortcuts only re-mark blocks that sit directly in the root, so headings already re-mark (`### ` before an h2's text makes an h3) and list items never did. Tasks keep RichText's shape: bulleted items with task state, drawn by TaskCheckboxPlugin and written as `[ ]` or `[x]`. A marker counts only when the editor's transformers make the same block from it at the start of a paragraph. Read-only and disabled editors are unchanged. Co-authored-by: Emily Horsman <3536823+emilyhorsman@users.noreply.github.com>
92697a0 to
5d952aa
Compare
There was a problem hiding this comment.
This pull request can't be approved yet, because no current spec covers what Markdown shortcuts do at the start of a list item. An engineering owner needs to provide or identify the governing spec, or approve this behavior directly, including turning the whole list into tasks on [ ] and splitting the list when # or > lifts an item out. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner. Separately, please fix one defect: when transformers is Lexical's set plus CHECK_LIST, a typed [x] draws checkboxes that getMarkdown() does not write (the task branch of $switchList in ListItemShortcutsPlugin.tsx), so the task marker should follow the transformer that matched, with a test for [...TRANSFORMERS, CHECK_LIST].
[Automated review]
Co-authored-by: ernestt <ernestt@users.noreply.github.com>
|
/approve-spec 1cd4ace |
There was a problem hiding this comment.
Changes requested: an engineering owner has approved the list-item shortcut behavior for this head, but three things need fixing first.
- When a caller's
transformersinclude Lexical'sCHECK_LIST, typing[ ]or[x]at the start of a list item draws checkboxes thatgetMarkdown()does not write, so the tasks are lost on reload; build the block the matching transformer would build (achecklist forCHECK_LIST) or leave the marker as typed, and add a test with[...TRANSFORMERS, CHECK_LIST]. - Newly live check: the description and the
$takeOutOfListJSDoc say the items after a lifted item stay "numbered on from where they were", but when the item holds a nested list of the same type the two lists merge, so##atTwoin1. One / 2. Two / 1. Two a / 3. Threeleaves1. Two a / 2. Three; describe that and pin it with a numbered test, or keep the numbering in code. - Newly live check: the changeset, the
hasMarkdownShortcutsJSDoc and the doc entry say#and>take any list item out of its list, but in a nested list they stay as typed, so say "a top-level item" instead.
Two comment trims (the 21-line file header and the JSX comment above the plugin) are optional. The owner approval is tied to this exact head, so after the fixes an engineering owner needs to comment /approve-spec with the new head SHA.
Claims and files checked
- Changeset: a typed
-,1.or[ ]switches the item's whole list between bullets, numbers and tasks — verified (ListItemShortcutsPlugin.tsx:147-179). - Changeset and docs:
#or>takes the item out of its list — false for nested items, where the marker stays as typed (:325-330). - Changeset: the rest of the list continues after the lifted item — verified (
:212-220). - Changeset and description: each change is one undo step back to the typed marker — verified (
:260; test atListItemShortcutsPlugin.test.tsx:224-236). - Description: Lexical's own shortcuts never re-mark a list item — verified in substance: Lexical's runner re-marks only a block whose parent is the root or a shadow root (the description says the root).
- Description: the plugin renders only with
hasMarkdownShortcuts— verified (RichTextEditor.tsx:762-764). - Description:
N.sets the start only at the first item — verified (:172-178). - Description:
[ ]makes every item a task, keeps existing task states and gives the typed item the marker's state;-orN.makes every item plain — verified (:153-169). - Description: no Lexical check list is made, so a typed task matches an imported one — true with the default transformers, false under Lexical's
CHECK_LIST(finding 1). - Description: a take-out keeps the earlier items and promotes the item's nested lists — verified (
:188-225). - Description: the items after a lifted item keep their numbering — false when the item holds a nested list of the same type (finding 2).
- Description: in a nested list
#and>stay as typed while list switches still apply — verified (:325-330; test case atListItemShortcutsPlugin.test.tsx:141). - Description: a marker counts only when the editor's transformers make the same block from it — false: the check matches patterns only (
:110-119), soCHECK_LISTadmits a block it cannot write (finding 1). - Description:
transformers={[]}turns the feature off, and a set without a construct leaves its marker as text — verified (:110-119; test at:268-277). - Description: read-only and disabled editors are unchanged — verified (
:279,RichTextEditor.tsx:578). - Description:
RichTextViewis unchanged — verified (not touched; it renders no shortcut plugin). - Description: no prop, export, type or node is added — verified (package entries unchanged).
- Description: untouched blocks keep their bytes and the typed task list round-trips — verified with the default transformers (tests at
:184-222). - Description: test, typecheck and lint results and the merge's suite run — not verified; the 26-case count and the 21-failed / 5-passed red arm match the test file.
- Description: the merge keeps main's three plugin imports beside the new one — verified (
RichTextEditor.tsx:89-93). packages/richtext/src/ListItemShortcutsPlugin.tsx— findings 1 and 2; optional comment trim.packages/richtext/src/ListItemShortcutsPlugin.test.tsx— clear; no case forCHECK_LISTor a numbered nested take-out.packages/richtext/src/RichTextEditor.tsx— finding 3 (JSDoc); wiring clear; optional comment trim.packages/richtext/src/RichTextEditor.doc.mjs— finding 3..changeset/in-richtexteditor-a-markdown-block-marker-typed--9chd.md— finding 3; bump and tag clear.
| /** | ||
| * @file ListItemShortcutsPlugin.tsx | ||
| * @input Uses @lexical/react (composer context), @lexical/list, | ||
| * @lexical/rich-text, @lexical/markdown (Transformer), lexical, and the task | ||
| * state in markdownTaskList.ts. | ||
| * @output Exports ListItemShortcutsPlugin, which re-marks a list item when a | ||
| * Markdown block marker is typed at its start, keeping the item's text: | ||
| * `- `, `1. `, or `[ ] ` switches the item's list between bullets, numbers, | ||
| * and tasks, and `# ` or `> ` takes the item out of its list as a heading | ||
| * or a quote, the rest of the list continuing after it. | ||
| * @position Rendered by RichTextEditor beside MarkdownShortcutPlugin, only | ||
| * when `hasMarkdownShortcuts` is on. Lexical's shortcuts re-mark a block | ||
| * that sits directly in the root (`### ` before an h2's text makes an h3) | ||
| * but never look inside a list. A marker counts here only when the editor's | ||
| * `transformers` turn the same marker into that block at the start of a | ||
| * paragraph, so the transformers stay the one source of Markdown behavior. | ||
| * Each re-marking is one undo step, back to the typed marker. Tasks keep | ||
| * RichText's shape (markdownTaskList.ts): items of a bulleted list that | ||
| * carry task state, which TaskCheckboxPlugin draws and LIST_EXPORT writes | ||
| * as `[ ]` or `[x]`. | ||
| */ |
There was a problem hiding this comment.
Non-blocking (INV29): this header runs about 21 lines and repeats the function JSDoc below. Suggested: "Lexical's shortcuts never look inside a list. A marker counts only when the editor's transformers make that block from it. Each re-mark is one undo step."
…rites; a top-level item lifts A task marker takes the shape of the transformer that matched it: with Lexical's CHECK_LIST the list becomes a check list, which CHECK_LIST writes back, so typed tasks survive a reload. The lift's JSDoc says Lexical merges the rest into a nested list of the same type and numbers them on, and a test pins it. The changeset and docs say # and > lift a top-level item. Red-first: the CHECK_LIST test failed on 1cd4ace. Co-authored-by: ernestt <ernestt@users.noreply.github.com>
|
/approve-spec d23bb4e |
There was a problem hiding this comment.
Not ready for approval yet: the list-item shortcut behavior needs an engineering owner's decision at this head, and one fix is needed.
No current spec states what Markdown shortcuts do at the start of a list item, and the earlier approval covered an older head, which this head changes for editors that use Lexical's CHECK_LIST. An engineering owner approves the behavior (the list switches, turning every item into a task including the check-list shape, and lifting a top-level item out as a heading or quote) with a /approve-spec <full head SHA> comment on the pull request; the author may do so if they are an engineering owner. The three findings from the last review are fixed.
Blocking:
- Typing
[ ]or[x]at an item of a list that is already a Lexical check list unchecks every other item, because$setTaskState(each, null)writessetChecked(false)on check-list items (ListItemShortcutsPlugin.tsx:151-160): convert only when the list is not yet a check list, and add aCHECK_LISTtest that types[x]at two items.
Claims and files checked
- Changeset: a marker at the start of an item re-marks it and keeps its text: verified.
- Changeset:
-,1.,[ ]switch the list between bullets, numbers and tasks: verified. - Changeset:
#and>lift a top-level item, and the rest of the list continues: verified. - Changeset: each change is one undo step back to the typed marker: verified.
- Description: existing tasks keep their state and the typed item takes the marker's: verified with the default transformers; false for an existing check list under
CHECK_LIST(the blocking item). - Description: switching to bullets or numbers makes every item plain: verified.
- Description: no Lexical check list is created: true for the default transformers; the later update replaces it for
CHECK_LIST, and the earlier sentences should be updated. - Description:
N.sets the start only at the first item: verified. - Description: nested lists keep
#and>as typed, while list switches still apply: verified. - Description: a marker counts only when the editor's transformers make that block, and empty transformers turn it off: verified for shipped transformers; for a custom transformer only the pattern is checked.
- Description: read-only, disabled and
hasMarkdownShortcuts={false}editors are unchanged, and no prop, export, type or node is added: verified. - Description: untouched blocks keep their bytes, and typed tasks round-trip: verified.
- Description: with
CHECK_LIST, typed tasks are written back and survive a reload: written back, verified; the reload holds whenCHECK_LISTcomes before Lexical's bullet transformer. - Description: renumbering after a lift and the top-level wording: verified.
- Description: the RichText suite, eslint and prettier pass: verified.
packages/richtext/src/ListItemShortcutsPlugin.tsx: blocking item; comment-style suggestions.packages/richtext/src/ListItemShortcutsPlugin.test.tsx: no case for a marker typed in an existing check list.packages/richtext/src/RichTextEditor.tsx: clear.packages/richtext/src/RichTextEditor.doc.mjs: clear..changeset/in-richtexteditor-a-markdown-block-marker-typed--9chd.md: clear.
| /** | ||
| * Takes `item` out of `list` as `block`, holding the item's content. The | ||
| * items before it stay in `list`; the lists nested under it follow `block` | ||
| * at the top level, since a heading or a quote cannot hold them; and the | ||
| * items after it continue in a list of the same kind, numbered on from where | ||
| * they were. When the last of those nested lists has the same type, Lexical | ||
| * merges the items after it into that list, which numbers them on from its | ||
| * own items. | ||
| */ |
There was a problem hiding this comment.
Non-blocking (INV29): the new sentence runs 27 words, and the one before it packs three topics. Suggested:
/**
* Takes `item` out of `list` as `block`. Items before it stay in `list`.
* Lists nested under it move to the top level after `block`, because a
* heading or a quote cannot hold a list. Items after it go into a new list
* of the same kind that keeps their numbers. If the last nested list has the
* same type, Lexical merges the new list into it, and those items take its
* numbering.
*/There was a problem hiding this comment.
Changes requested: an engineering owner has approved the list-item shortcut behavior for this head, but two things need fixing.
- With Lexical's
CHECK_LISTintransformers, typing[x]or[ ]at an item of a list that is already a check list unchecks every other item, because$setTaskState(each, null)writessetChecked(false)right after the loop keeps each state (ListItemShortcutsPlugin.tsx:151-160); convert only a list that is not yet a check list, and add a test that types[x]at two items. - The description still says no Lexical
checklist is created and that one was rejected, and it and the test title atListItemShortcutsPlugin.test.tsx:202say theCHECK_LISTtasks survive a reload, which holds only whenCHECK_LISTcomes before Lexical's bullet transformer (the test uses[...TRANSFORMERS, CHECK_LIST]and asserts only the export), so update the description and assert the reload with[CHECK_LIST, ...TRANSFORMERS], or drop the claim.
The approval is tied to this head, so the commit with the fixes needs a new /approve-spec <full head SHA> from an engineering owner; the author may comment it if they are one. The non-blocking notes on the file header's length and the $takeOutOfList JSDoc are in the existing inline threads.
Claims and files checked
- Claim: list markers switch the item's whole list between bullets, numbers and tasks — verified by the plugin tests.
- Claim: a heading or quote marker takes a top-level item out of its list and the rest continues after it — verified by the lift tests, including the renumbering when a nested list of the same type absorbs the rest.
- Claim: each change is one undo step back to the typed marker — verified by the undo test.
- Claim: a marker counts only when the editor's transformers make that block, and an empty set turns this off — verified in code and by test.
- Claim: a numbered start is taken only at the first item — verified.
- Claim: in a nested list, heading and quote markers stay as typed — verified.
- Claim: read-only and disabled editors are unchanged — verified (editability check, read-only test).
- Claim: items that were already tasks keep their state — false with
CHECK_LIST(blocking finding 1). - Claim: no Lexical check list is created, and that option was rejected — false at this head (blocking finding 2).
- Claim: with
CHECK_LIST, typed tasks survive a reload — holds only withCHECK_LISTfirst; the test does not check it (blocking finding 2). - Claim: no public API change — verified with the API surface generator.
- Claim: the plugin file's 28 tests and the package's 796 tests pass — verified.
- File:
.changeset/in-richtexteditor-a-markdown-block-marker-typed--9chd.md— clear. - File:
packages/richtext/src/ListItemShortcutsPlugin.tsx— blocking finding 1; two non-blocking comment-style notes. - File:
packages/richtext/src/ListItemShortcutsPlugin.test.tsx— blocking finding 2; no test for finding 1. - File:
packages/richtext/src/RichTextEditor.doc.mjs— clear. - File:
packages/richtext/src/RichTextEditor.tsx— clear.
With CHECK_LIST, a list that is already a check list keeps every item's state; only another list is converted. A test types [x] at two items and expects both checked; it failed on d23bb4e. The CHECK_LIST test puts CHECK_LIST first, so its tasks also read back as a check list. Co-authored-by: ernestt <ernestt@users.noreply.github.com>
|
/approve-spec 0fa3431 |
There was a problem hiding this comment.
Changes requested: the check-list fix from the last round works, and an engineering owner approved the list-item shortcut behavior for this head, but two things still need fixing.
- Miss: typing a list marker in a list that sits right after a list of the new type leaves two separate lists (
ListItemShortcutsPlugin.tsx:196, also:159and:170), so in1. Draft / 2. Review, a blank line,- Ship, typing1.atShipshows1. Shipon its own while the saved Markdown reloads as one list with3. Ship; after the switch, let Lexical merge the list into a same-type list just before it (for example by marking that list dirty), and add a test for this case. - The description still says "No Lexical
checklist is created", which is not true for editors withCHECK_LIST; delete that sentence or limit it to the default transformers.
The approval is tied to this head, so the commit with the fix needs a new /approve-spec <full head SHA> from an engineering owner; the author may comment it if they are one. Non-blocking: the new check-list test also passes without the plugin, because the literal text exports the same string, so assert the items' checked state too; the description's test counts are out of date.
Claims and files checked
- Claim: list markers switch the item's whole list between bullets, numbers and tasks: verified by the plugin tests and in Chromium, except that a switched list does not join a same-type list just before it (first blocking finding).
- Claim: a numbered start comes from the marker only at the list's first item: verified.
- Claim: a task marker makes every item a task, keeps existing task state, and takes the shape of the matching transformer: verified, including a check list with
CHECK_LIST. - Claim: no Lexical check list is created: false with
CHECK_LIST(second blocking finding). - Claim: in an existing check list a marker changes only its own item: verified in code and in a two-marker check; the new test cannot tell a check list from literal text (non-blocking).
- Claim: with
CHECK_LISTahead of the bullet transformer, typed tasks read back as a check list: verified. - Claim: a heading or quote marker takes a top-level item out of its list, promotes its nested lists, and the rest continues, renumbered when a promoted list of the same type absorbs it: verified.
- Claim: in a nested list, heading and quote markers stay as typed, and list switches still apply: verified.
- Claim: each change is one undo step back to the typed marker: verified.
- Claim: a marker counts only when the editor's transformers make that block, and an empty set turns this off: verified.
- Claim: read-only, disabled and shortcuts-off editors, and the read-only view, are unchanged: verified.
- Claim: untouched blocks keep their authored bytes: verified by test.
- Claim:
- [ ]typed in a paragraph makes a task item: verified. - Claim: no public API change: verified with the API surface generator.
- Claim: the tests fail without the plugin: verified, 23 of 29 fail on the base.
- Claim: the plugin tests and the package suite pass: verified, now 29 and 797 tests; the counts in the description are stale.
- Claim (changeset): the list switches, the top-level lift and the one undo step: verified.
- File:
.changeset/in-richtexteditor-a-markdown-block-marker-typed--9chd.md: clear. - File:
packages/richtext/src/ListItemShortcutsPlugin.tsx: first blocking finding. - File:
packages/richtext/src/ListItemShortcutsPlugin.test.tsx: non-blocking note on the new check-list test. - File:
packages/richtext/src/RichTextEditor.doc.mjs: clear. - File:
packages/richtext/src/RichTextEditor.tsx: clear.
…ore it Lexical merges a dirty list only with a same-type list after it, so the plugin marks the list before a switched one, and the two join as the Markdown reads back. A test types 1. after a numbered list and a blank line; it failed on 0fa3431. The check-list test also asserts the items' checked state. Co-authored-by: ernestt <ernestt@users.noreply.github.com>
|
/approve-spec 6bf38a1 |
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet, because the pull request is a draft; it runs once the pull request is marked ready for review.
An engineering owner approved the list-item shortcut behavior for this head, and both fixes from the last round hold: a switched list now joins a list of the same type just before it, with a test that fails without the fix, and the description no longer says that no Lexical check list is created.
Non-blocking: the description's test counts are out of date (the plugin test file now has 30 tests, 25 of which fail without the plugin), and the two comment-style suggestions on the file header and the $takeOutOfList JSDoc are still open.
Claims and files checked
- Claim: on the base, a list marker typed at the start of an item stays as text and no task item can be typed: verified, 25 of the 30 new tests fail on the base.
- Claim: Lexical's shortcuts act only on blocks whose parent is the root, and its list transformer refuses heading parents: verified in the Lexical source.
- Claim: bullet markers switch the item's list to bullets and a number marker to numbers, starting at that number only at the first item: verified by tests.
- Claim: a task marker makes every item a task in the shape of the matching transformer, keeps existing task state, gives the typed item the marker's state, and a bullet or number marker makes every item plain: verified by tests, and
[X]checked separately. - Claim: in an existing check list a task marker changes only its own item: verified; the test now fails without the plugin.
- Claim: a heading or quote marker takes a top-level item out of its list, promotes its nested lists, continues the rest numbered on, and renumbers when a promoted list of the same type absorbs them: verified by tests.
- Claim: in a nested list, heading and quote markers stay as typed and list switches still apply: verified by tests.
- Claim: a switched list joins a list of the same type just before it, as the Markdown reads back: verified; the test fails on the previous head, and the reload matches.
- Claim: each change is one undo step back to the typed marker: verified by test, also after a join.
- Claim: a marker counts only when the editor's transformers make that block, and an empty set turns this off: verified by test and by removing the check.
- Claim: a heading's own marker stays with Lexical's shortcuts: verified by test.
- Claim: no public API change; read-only, disabled and shortcuts-off editors and the read-only view are unchanged: verified with the API surface generator, tests and the editability check.
- Claim: the result is the same nodes the toolbar's block selector makes: verified.
- Claim: untouched blocks keep their authored bytes and a typed task list writes back byte for byte: verified by tests.
- Claim: with
CHECK_LISTahead of the bullet transformer, typed tasks read back as a check list: verified. - Claim: removing the editability or transformer check fails the read-only or empty-transformers test: verified.
- Claim: a nested item whose text starts with a block marker exports unescaped, unchanged here: verified as existing behavior.
- Claim: typecheck, lint, formatting and changeset checks pass: verified.
- Claim: the plugin test and package suite counts: out of date; now 30 tests (25 fail on the base) and 798 in the package (non-blocking).
- Claim (changeset): list switches, the top-level lift with the rest of the list continuing, and one undo step: verified.
- File:
.changeset/in-richtexteditor-a-markdown-block-marker-typed--9chd.md: clear. - File:
packages/richtext/src/ListItemShortcutsPlugin.tsx: clear; two comment-style suggestions still open (non-blocking). - File:
packages/richtext/src/ListItemShortcutsPlugin.test.tsx: clear. - File:
packages/richtext/src/RichTextEditor.doc.mjs: clear. - File:
packages/richtext/src/RichTextEditor.tsx: clear.
There was a problem hiding this comment.
Changes requested: one defect in the task-marker path needs a fix before this can merge.
- Miss: in an editor with the default transformers that shows a Lexical
checklist, typing[ ]or[x]at one item unchecks every other item, because$switchListswitches the list to bullets (ListItemShortcutsPlugin.tsx:170) before it reads each item'sgetChecked(), which Lexical then reports asundefined; read the states before the switch, as theCHECK_LISTbranch already does, and add a test for this case.
The owner-approved behavior otherwise works as described, CI passes on this head, and the fixes from earlier rounds hold. Non-blocking: the description's test counts are out of date (the plugin test file now has 30 tests, 25 of which fail without the plugin, and the package suite has 798), and two comment-style suggestions on the file header and the $takeOutOfList JSDoc are still open.
Claims and files checked
- Claim: on the base, a list marker typed at the start of an item stays as text and no task item can be typed: verified, 25 of the 30 new tests fail on the base, and the same keys in a browser leave escaped text.
- Claim: Lexical's shortcuts act only on blocks whose parent is the root, and its list transformer refuses heading parents: verified in Lexical's code.
- Claim: the plugin runs only when Markdown shortcuts are on: verified in
RichTextEditor.tsx. - Claim: bullet markers switch the item's list to bullets and a number marker to numbers, starting at that number only at the first item: verified by tests.
- Claim: a task marker makes every item a task in the shape of the matching transformer, and the typed item takes the marker's state: verified by tests and in a browser.
- Claim: items that were already tasks keep their state: false for a Lexical check list under the default transformers (the blocking finding); true for RichText task items and with
CHECK_LIST. - Claim: a bullet or number marker makes every item plain: verified by tests.
- Claim: a typed task is stored as an imported one and writes back byte for byte: verified by test.
- Claim: a heading or quote marker takes a top-level item out of its list, keeps the items before it, promotes its nested lists, and continues the rest numbered on, renumbered when a promoted list of the same type absorbs them: verified by tests and in a browser.
- Claim: in a nested list, heading and quote markers stay as typed and list switches still apply: verified by tests.
- Claim: a switched list joins a list of the same type just before it: verified by test.
- Claim: in an existing check list with
CHECK_LIST, a typed marker changes only its own item: verified by test. - Claim: each change is one undo step back to the typed marker: verified by test.
- Claim: a marker counts only when the editor's transformers make that block, and an empty set turns this off: verified by test and by removing the check.
- Claim: removing the editability check or the transformer check fails the read-only or empty-transformers test: verified, each removal fails exactly that test.
- Claim: a heading's own marker stays with Lexical's shortcuts: verified by test.
- Claim: no public API change; read-only, disabled and shortcuts-off editors and the read-only view are unchanged: verified with the API surface generator, tests and the editability check.
- Claim: the result is the same stock list, heading and quote nodes the toolbar makes: verified.
- Claim: with
CHECK_LISTahead of the bullet transformer, typed tasks read back as a check list: verified by importing such Markdown with that order. - Claim: the merge of the base branch kept the new plugin beside the base branch's plugins: verified in the import block.
- Claim: a nested item whose text starts with a block marker exports unescaped: not rechecked; reported as existing behavior this change does not touch.
- Claim: typecheck, lint, formatting and changeset checks pass: verified.
- Claim: the plugin test and package suite counts: out of date; now 30 tests (25 fail on the base) and 798 in the package (non-blocking).
- Claim (changeset): list switches between bullets, numbers and tasks, the top-level lift with the rest of the list continuing, and one undo step: verified, apart from the check-list state loss above.
- File:
.changeset/in-richtexteditor-a-markdown-block-marker-typed--9chd.md: clear. - File:
packages/richtext/src/ListItemShortcutsPlugin.tsx: blocking finding in the task branch; two comment-style suggestions still open (non-blocking). - File:
packages/richtext/src/ListItemShortcutsPlugin.test.tsx: clear; it needs a case for the check-list fix. - File:
packages/richtext/src/RichTextEditor.doc.mjs: clear. - File:
packages/richtext/src/RichTextEditor.tsx: clear.
…tate with the default transformers The plugin reads each item's checked state before it switches a Lexical check list to RichText's task shape, because Lexical reports no checked state once the list is no longer a check list. A test types [ ] in a check list of three items; it failed on 6bf38a1. Co-authored-by: ernestt <ernestt@users.noreply.github.com>
|
/approve-spec 0b237b0 |
There was a problem hiding this comment.
Approved. The list-item Markdown shortcuts match the behavior an engineering owner approved for this head, every fix from earlier rounds is in place with a test, and CI is green.
Optional: when an input method commits a marker together with its space in one update, the list item keeps it as text, while Lexical's own shortcuts convert the same commit in a paragraph (ListItemShortcutsPlugin.tsx:340-347). Two comment-style suggestions on that file are still open: trim the file header to its reasons, and split the long sentence in the $takeOutOfList JSDoc.
Claims and files checked
Claims:
- Changeset: a marker typed at the start of a list item re-marks it and keeps its text: verified by the switch and lift tests in
ListItemShortcutsPlugin.test.tsx. - Changeset:
-,1.or[ ]switches the list between bullets, numbers and tasks: verified by the switch tests. - Changeset:
#or>lifts a top-level item as a heading or quote and the rest of the list continues: verified by the lift tests; a nested item keeps the marker as text (test). - Changeset: each change is one undo step back to the typed marker: verified by the undo test and a task-switch probe.
- Before this change every marker stayed as item text, and
- [ ]in a paragraph left[ ]as text: verified; 26 of the 31 tests fail on the base. - Lexical's own shortcuts never run block transformers inside a list: verified in Lexical's shortcut source.
- The plugin runs only with
hasMarkdownShortcutson; turned off, read-only, disabled andtransformers={[]}editors are unchanged: verified (RichTextEditor.tsx:761-763, tests, a disabled-editor probe). N.sets the start only at the first item: verified (the3.test, and a probe typing0.at a later item).- Tasks take the matching transformer's shape, existing tasks keep their state, the typed item takes the marker's state, and bullets or numbers make every item plain: verified by the switch rows and the check-list tests.
- A lift keeps the items before, promotes nested lists, and numbers the rest on, or merges them into a promoted same-type nested list: verified by the lift tests, the renumbering test and export-reload probes.
- In a nested list
#and>stay as typed while list switches still apply: verified by tests. - A marker counts only when the editor's transformers make that block: verified (
transformerFor; removing that check fails the empty-transformers test). - A heading's own marker stays with Lexical's shortcuts: verified (first test passes on base and head).
- No prop, export, type or node is added: verified (the API generator and CI both report no change).
- Typed tasks write back byte for byte and untouched blocks keep their authored text: verified by tests and eleven export-reload probes.
- With
CHECK_LISTbefore the bullet transformer, typed tasks read back as a check list: verified by a reload probe. - A switched list joins a same-type list just before it: verified by the join test and probes.
- In an existing check list a task marker changes only its own item, and with the default transformers the other items keep their state: verified by tests; the last one fails on the previous revision.
- Removing the editability or transformer check fails the read-only or empty-transformers test: verified by mutation runs.
- A list that begins with a nested list exports at an indent that reads back flat: verified by a probe.
- A nested
- # Innerexports unescaped (noted as found while testing, not changed): the export is verified; RichText reads it back as text and a CommonMark reader as a heading, the same on the base. - The RichText suite, typecheck and lint pass: verified (47 files, 799 tests; CI typecheck and lint green).
- The merge of
mainconflicted only in theRichTextEditor.tsximport block: verified in the merge's combined diff.
Files:
.changeset/in-richtexteditor-a-markdown-block-marker-typed--9chd.md: clear.packages/richtext/src/ListItemShortcutsPlugin.tsx: non-blocking suggestions only (input-method commit, two comment-style points).packages/richtext/src/ListItemShortcutsPlugin.test.tsx: clear.packages/richtext/src/RichTextEditor.tsx: clear.packages/richtext/src/RichTextEditor.doc.mjs: clear.
|
/approve-spec e43a4b3 |
There was a problem hiding this comment.
Changes requested: the list-item Markdown shortcuts work as the engineering owner approved for this revision, and every fix from earlier rounds is in place with a test; one description fix remains.
- Regression: the description still says CI and a 47-file RichText test run are at the current head, but after the merge of
mainCI has not run on this revision (the pull request is a draft) and the RichText suite has 48 files and 812 tests, all passing, so please update those lines and the lint count (32 warnings, on the base too).
Optional: an input method that commits a marker with its space in one update leaves it as text in a list item (ListItemShortcutsPlugin.tsx:341-347), and the Effect note and comment-style suggestions on that file are open.
Claims and files checked
Claims:
- Changeset: a marker typed at the start of a list item re-marks it and keeps its text: verified by the switch and lift tests.
- Changeset:
-,1.or[ ]switches the list between bullets, numbers and tasks: verified by the switch tests. - Changeset:
#or>lifts a top-level item as a heading or quote and the rest of the list continues after it: verified by the lift tests; in a nested list the marker stays as text (test). - Changeset: each change is one undo step back to the typed marker: verified by the undo test.
- Before this change every marker stayed as item text, and
- [ ]in a paragraph left[ ]as text: verified; 26 of the 31 tests fail on the base, and browser frames of the base keep each marker as text. - Lexical's own shortcuts never run block transformers inside a list, and its list transformer refuses a heading: verified in Lexical's shortcut source.
- The plugin runs only with Markdown shortcuts on and checks editability; turned off, read-only, disabled and empty-transformer editors are unchanged: verified (the mount condition, the editable flag, tests).
N.sets the start only at the first item, and*and+switch to bullets: verified (tests, a probe).- Tasks take the matching transformer's shape, existing tasks keep their state, the typed item takes the marker's state, and bullets or numbers make every item plain: verified by the switch rows and the check-list tests.
- A lift keeps the items before, promotes nested lists, and numbers the rest on, or merges them into a promoted same-type nested list: verified by the lift tests, the renumbering test and round-trip probes.
- A marker counts only when the editor's transformers make that block, so a set without quotes leaves
>as text: verified (code, the empty-transformers test, a probe). - A heading's own marker stays with Lexical's shortcuts: verified (the first test passes on the base and the head).
- No prop, export, type or node is added: verified (the API surface generator reports no change).
- Typed tasks write back byte for byte, draw the same checkboxes as imported tasks, and untouched blocks keep their authored text: verified by tests and round-trip probes.
- The result uses the same stock nodes as the toolbar's block selector and adds no chrome: verified for list, heading and quote nodes; task items are the imported task shape.
- With
CHECK_LISTfirst, typed tasks read back as a check list; a switched list joins a same-type list just before it; in an existing check list a task marker changes only its own item, and with the default transformers the other items keep their state: verified by tests, each failing on the revision before its fix. - A list that begins with a nested list reads back flat, and a nested item that starts with a marker exports unescaped: verified by probes; RichText's own import keeps that nested text.
- The first round's red arm and the mutation check: verified (26 of 31 fail on the base; removing the editability or transformer check fails exactly the read-only or empty-transformers test).
- The first merge of
mainconflicted only in the editor's import block: verified in that merge's combined diff. - The plugin's 31 tests pass at the current head: verified.
- "CI and the full run below are at the current head": false; CI skipped every job on this revision.
- The RichText suite "47 files pass at the current head": false; 48 files and 812 tests pass.
- Typecheck, prettier and changeset checks pass, and lint has no errors and no warning on a changed line: verified; lint reports 32 warnings on the base and the head, not 34.
- Repository checks pass: verified for 24 of 27 steps; the three that need git metadata cover scripts, executable bits and fixtures, which this change does not touch.
Files:
.changeset/in-richtexteditor-a-markdown-block-marker-typed--9chd.md: clear.packages/richtext/src/ListItemShortcutsPlugin.tsx: optional suggestions only (input-method commit, Effect note, comment style).packages/richtext/src/ListItemShortcutsPlugin.test.tsx: clear.packages/richtext/src/RichTextEditor.tsx: clear.packages/richtext/src/RichTextEditor.doc.mjs: clear.
There was a problem hiding this comment.
Approved pending CI: CI has not run on this revision because the pull request is a draft; it runs once the pull request is marked ready for review.
The list-item Markdown shortcuts behave as the engineering owner approved for this revision, each fix from earlier rounds is pinned by a test that fails without it, and the description's claims match what runs.
Optional, non-blocking:
- An input method that commits a marker together with its space leaves it as text at the start of a list item, while Lexical converts the same commit in a paragraph (
ListItemShortcutsPlugin.tsx:345). - The new Effect (
:399-402) has no one-line note naming the editor it subscribes to, and the file header and two JSDoc blocks (:5-20,:139-141,:207-212) run past the comment-style limits.
Claims and files checked
Claims:
- Changeset: a marker typed at the start of a list item re-marks it and keeps its text: verified by the switch and lift tests and a real-key browser run.
- Changeset:
-,1.or[ ]switches the list between bullets, numbers and tasks: verified by the switch tests. - Changeset:
#or>lifts a top-level item as a heading or quote and the rest of the list continues: verified by the lift tests; in a nested list the marker stays as text (test). - Changeset: each change is one undo step back to the typed marker: verified by the undo test.
- Before this change every marker stayed as item text, and
- [ ]in a paragraph left[ ]as text: verified; 26 of the 31 tests fail on the base with those exports. - Lexical's shortcuts skip blocks whose parent is not the root, so a list-aware element transformer cannot work, and Lexical's list transformer refuses a heading: verified in Lexical's source.
- Mixed task and plain lists still import from Markdown and keep their mix: consistent; the change does not touch import.
- The plugin runs only with Markdown shortcuts on and checks editability; turned-off, read-only and disabled editors and
RichTextVieware unchanged: verified by the mount condition, the editability wiring and tests. *and+switch to bullets,N.to numbers, andNsets the start only at the first item: verified by tests and a browser run.- Tasks take the matching transformer's shape, earlier tasks keep their state, the typed item takes the marker's, and bullets or numbers make every item plain: verified by the switch and check-list tests.
- A lift keeps the items before it, promotes nested lists and numbers the rest on, or merges them into a promoted nested list of the same type: verified by the lift and renumbering tests.
- A marker counts only when the editor's transformers make that block, so an empty set turns it off: verified by a test and a mutation run.
###before a level-2 heading is Lexical's own re-mark: verified; that test passes on the base and the head.- Each re-mark is its own history step, the same shape Lexical's shortcuts use: verified by the undo test and Lexical's source.
- No prop, export, type or node is added: verified by the API surface generator (no change).
- Typed tasks are the task items the checkbox plugin draws and write back byte for byte, and untouched blocks keep their authored text: verified by tests.
- No visual, theming or layout change, and the result uses stock list, heading and quote nodes: verified; no style is touched.
- A nested item that begins with a marker exports unescaped, found while testing and left unchanged: consistent; the plugin leaves that path as typed.
- The first round's red arm (21 of 26 failing) and the mutation check: verified by re-running both.
- The plugin's 31 tests pass, and the RichText suite has 48 files and 812 tests passing: verified.
- CI last ran on the revision before the merge of
mainand passed, and runs again once the draft moves on: verified; that run has no failed check, and a draft runs CI when it is marked ready. - Typecheck is clean, lint has no errors and its 32 warnings predate the change with none on a changed line, and prettier and the changeset check pass: verified.
- Repository checks pass: verified for 24 of 27 steps; the other three need git metadata and cover scripts, executable bits and fixtures, which this change does not touch.
- The public text carries no internal context: verified by a scan of the title, description, commit messages and added lines.
- The earlier rounds' test counts (47 files, 794 and 796 tests) describe earlier revisions; this revision's 48 files and 812 tests are verified above.
- The earlier rounds' fixes (check-list shape, renumbering, top-level wording, a check-list marker changing only its own item, joining a same-type list before it, keeping a check list's other states): verified; each test fails on the base.
Files:
.changeset/in-richtexteditor-a-markdown-block-marker-typed--9chd.md: clear.packages/richtext/src/ListItemShortcutsPlugin.tsx: optional suggestions only (input-method commit, Effect note, comment style).packages/richtext/src/ListItemShortcutsPlugin.test.tsx: clear.packages/richtext/src/RichTextEditor.tsx: clear.packages/richtext/src/RichTextEditor.doc.mjs: clear.
| // One character typed, just now, in the same text. | ||
| if ( | ||
| !dirtyLeaves.has(key) || | ||
| previous.key !== key || | ||
| previous.offset !== offset - 1 | ||
| ) { | ||
| return null; |
There was a problem hiding this comment.
Non-blocking: an input method that commits a marker together with its space arrives as one update tagged COMPOSITION_END_TAG, so this one-character check (and the isComposing() guard at :321) leaves it as text in a list item. Lexical's own shortcut listener accepts that commit when its last character is a space, so the same 1. converts in a paragraph. Accepting COMPOSITION_END_TAG here when the prefix ends with the space would match it.
| useEffect( | ||
| () => registerListItemShortcuts(editor, transformers), | ||
| [editor, transformers], | ||
| ); |
There was a problem hiding this comment.
Non-blocking (INV3): a new Effect names the external system it synchronizes. Suggested, as TaskCheckboxPlugin.tsx:122 does: // Subscribes to the Lexical editor's updates.
| * (markdownTaskList.ts). Switching to tasks makes every item a task, keeping | ||
| * the state of those that already were, and `item` takes the marker's; | ||
| * switching to bullets or numbers makes every item a plain one. A numbered |
There was a problem hiding this comment.
Non-blocking (INV29): this sentence runs 32 words with three topics. Suggested: "Switching to tasks makes every item a task. Items that were tasks keep their state. item takes the marker's state. Switching to bullets or numbers makes every item plain."
User need
A person writing in
RichTextEditorwith Markdown shortcuts on (the default) changes a block's kind by typing its marker at the start. That works for blocks that sit directly in the document:###before an h2's text makes it an h3. It does nothing in a list. Typed at the start of a list item:1.in a bulleted item stays as text. The item stays bulleted and exports as- 1\. Plan the launch.-in a numbered item stays as text.#or>cannot take an item out of its list, so the item has to be deleted and retyped.- [ ]typed in a paragraph makes a bullet at-, and then[ ]stays as text (- \[ \] Plan the launch).Why: Lexical's
MarkdownShortcutPluginruns element transformers only on a block whose parent is the root, so it never sees a list item. This matters because shortcuts are the keyboard path to block structure in an editor whose toolbar is optional, and task items had no typing path at all.Problem and solution fit
ListItemShortcutsPlugin, rendered byRichTextEditoronly whenhasMarkdownShortcutsis on. When a typed space completes a marker at the start of a list item's text, it re-marks the item in an update of its own and keeps the item's text:-,*,+switch the item's list to bullets, andN.to numbers. A numbered list starts atNwhen the item is its first.[ ],[x],[X]switch the list to tasks, in the shape of the transformer that matched the marker. With RichText's task transformer (the default), every item becomes a bulleted item with task state, asmarkdownTaskList.tsdefines it; with Lexical'sCHECK_LIST, the list becomes a Lexicalchecklist. Items that were already tasks keep their state, and the typed item takes the marker's state. Switching to bullets or numbers makes every item plain. Either way a typed task is stored as an imported one is.#–######or>takes a top-level item out of its list as a heading or a quote. The items before it stay. Lists nested under it move to the top level after it, because a heading cannot hold them. The items after it continue as a list of the same kind, numbered on from where they were.#and>stay as typed. The list switches still apply to the nested list.HISTORY_PUSH_TAG, so it is one undo step and undo returns to the marker as typed. This is the same shape Lexical's shortcut plugin uses.transformersmake the same block from it at the start of a paragraph.transformers={[]}, which turns off all shortcut typing, turns this off too. A custom set without, say, quotes leaves>as text.hasMarkdownShortcutsJSDoc and doc entry;Current specification
spec:AST-061/DEC-7("RichText is a form-field editor"): this changes how existing constructs (lists, task items, headings, quotes) are edited from the keyboard. It adds no document feature.spec:AST-061/DEC-8("the public surface is Lexical's, kept small"): no prop, export, type, or node. The plugin is internal and builds stockListNode,HeadingNode, andQuoteNode.spec:AST-061/FR5andDEC-6: task list items render as checkboxes. A typed task is the same task itemTaskCheckboxPlugindraws on import.spec:AST-062FR1–FR3:spec:AST-061/DEC-2("editing affordances never reflow"): nothing is drawn, so read/edit geometry is unaffected.Public delta and caller burden
hasMarkdownShortcutsis on (defaulttrue).hasMarkdownShortcuts={false}, read-only, and disabled editors are unchanged, and the plugin also checkseditor.isEditable().RichTextViewis unchanged.1.in- Plan the launch:- 1\. Plan the launch→1. Plan the launch[ ]in1. One / 2. Twoat Two:2. \[ \] Two→- [ ] One / - [ ] Two#in- First / - Second / - Thirdat Second:- \# Second→- First,# Second,- Thirdtransformersget re-marking only for the markers their transformers handle. There is no new decision for callers.Design and alternatives
[ ]marking only the typed item (GFM allows mixed task and plain items): rejected for this change, because-and1.switch the whole list and[ ]matches them. Mixed lists still import from Markdown and keep their mix.checklist forCHECK_LIST. So a typed task is stored the same way an imported one is.-or1.before a heading's text. Lexical's list transformer refuses heading parents, and this change keeps to list items.Evidence
packages/richtext/src/ListItemShortcutsPlugin.test.tsxmountsRichTextEditorand types one character per update, as a keyboard does. It covers:- [ ]typed in a paragraph making a task item;TaskCheckboxPlugindraws as[false, true], with an export that round-trips byte for byte;*emphasis*,&, and__Outro__as authored;Unchanged paths (pass before and after):
###before an h2's text is Lexical's own re-mark and undo;#in a nested item stays text;hasMarkdownShortcuts={false}andtransformers={[]}change nothing;Red arm, from the first round, with the plugin and its wiring removed (the file had 26 tests then; it has 31 now):
Mutation check: dropping the editability check or the transformers check fails the read-only and
transformers={[]}tests.Found while testing, not changed here: a nested list item whose text begins with a block marker exports unescaped (
- # Inner), so it reads back as a heading. The nested-item test asserts the editor's structure rather than that export.Scope
Testing
pnpm exec vitest run packages/richtext/src/ListItemShortcutsPlugin.test.tsx: 31 pass.pnpm exec vitest run packages/richtext/src: 48 files and 812 tests pass after the latest merge ofmain. CI last ran at 0b237b0, before that merge, and passed; it runs again at this head once review approves.pnpm -F @astryxdesign/richtext typecheck: clean, after building core and richtext.pnpm -F @astryxdesign/richtext lint: 0 errors. The 32 warnings were already there, and none are on changed lines. Strict lint on the changed files adds none.pnpm exec prettier --checkon the changed files,pnpm check:changesets, andpnpm check:repo: pass.Maintainer round (merge of main)
One merge of
mainon top (1cd4ace, no rebase); the commits above are unchanged. The only conflict was the import block inRichTextEditor.tsx, resolved by keepingListItemShortcutsPluginandmain'sDisabledLinkGuardPlugin,SelectAllScopePluginandImageSourceCaretPlugin. The merge commit was built outside a worktree, so the pre-commit hook's merge checks were run by hand on every file that differs frommain:node --checkonRichTextEditor.doc.mjs,eslintonRichTextEditor.tsx,ListItemShortcutsPlugin.tsxand its test (0 errors), andprettier --checkon those files and the changeset (clean). The RichText suite passes: 47 test files, 794 tests.Maintainer round 2 (CodeBunny's review)
One commit (d23bb4e), red-first; the commits above are unchanged.
CHECK_LISTintransformers, typing[ ]or[x]at the start of a list item makes a Lexicalchecklist, whichCHECK_LISTwrites as[ ]and[x]. The tasks also read back as a check list whenCHECK_LISTcomes before Lexical's bullet transformer intransformers, as it does in the test ([CHECK_LIST, ...TRANSFORMERS]). The test failed on 1cd4ace (getMarkdown()lost the tasks) and passes now.$takeOutOfListJSDoc says so, and a numbered test pins it:##atTwoin1. One / 2. Two / 1. Two a / 3. Threegives1. One, the heading, then1. Two a / 2. Three.hasMarkdownShortcutsdoc entry and its JSDoc say#and>take a top-level item out of its list; in a nested list they stay as typed. The file header is shorter, and a comment that restated the plugin's name is gone.Maintainer round 3
One commit, red-first. In a list that is already a check list, a typed
[x]or[ ]changes only its own item; before, it unchecked every other item. Only a list that is not yet a check list is converted, carrying over RichText's task state. A new test types[x]at two items and expects both checked. The test title and the description no longer claim a reload without naming the transformer order.Maintainer round 4
One commit, red-first. A list switched by a marker now joins a list of the same type just before it, as the Markdown reads back: in
1. Draft / 2. Review, a blank line,- Ship, typing1.atShipgives one list ending3. Ship. Lexical merges a dirty list only with a same-type list after it, so the plugin marks the list before it. A new test pins it. The two-item check-list test now also asserts achecklist with both items checked, so it fails if the markers stay as text. The description no longer says nochecklist is created.Maintainer round 5
One commit, red-first. With the default transformers, typing
[ ]or[x]at one item of an existing Lexicalchecklist (for example one loaded from a saved value) no longer unchecks the others. The plugin reads each item's checked state before it switches the list to RichText's task shape, because Lexical reports no checked state once the list is no longer a check list. A new test ([x] Alpha / [ ] Beta / [x] Gamma, typing[ ]at Beta) failed on 6bf38a1 and passes now.