githubnext/chopinPublic

Let's compose rich plans together

Let Tab leave the editor outside lists #292

MergedMaggieAppletonmerged 3 commits intomainfromrefs/pull/292/head
  1. MA

    Tab now leaves prose without modifying the shared document, nests lists where valid, and indents code with Escape then Tab as the exit. Table navigation remains delegated to Lexical.

    Integration preserves all product patches. The table browser test used End on macOS, which left the caret midword; it now uses Meta+ArrowRight on Darwin and End elsewhere. The same assertions pass with the native shortcut.

    Local checks, types, 17 Tab unit tests and all nine editing browser tests pass, including tables and collaborative list behavior. Published on top of #278; final CI is running.

    <details><summary>Original proposal and screenshot</summary>

    ## Why

    Tab was a keyboard trap that edited the shared document. MDXEditor's `listsPlugin` installs Lexical's `TabIndentationPlugin` for every block (pulled in via `dialectPlugins` in `plan-editor.tsx`), so once focus reached the editor:

    - Tab at the start of a heading or paragraph indented it (one more level per press). MDX cannot express that indent, so the source never shows it, but collaborators see it live. - Tab or Shift+Tab anywhere else inserted a literal tab character, which does reach the source (`A paragrap\th mid text`, `- item\t two`). - In a list, Tab only nested from the very start of an item, and on a first item it wrapped the item in an empty one that only collaborators saw.

    ## What changed

    A new `TabKeyPlugin` (`packages/editor/src/widgets/tab-key.tsx`) handles Tab before Lexical's own indentation and after `@lexical/table`'s:

    - **List item:** Tab nests and Shift+Tab un-nests, from anywhere in the item, but only when that is a real change. Every selected item needs an item before it to nest under (and must be below the lists plugin's depth limit). Un-nesting needs an item that is already nested. Otherwise focus moves on. - **Code block:** Tab inserts a tab. Shift+Tab removes one leading tab, or up to four leading spaces, from the caret's line, and does nothing if the line has none. Escape then Tab leaves the block (as in CodeMirror). Escape no longer blurs the editor in a code block, because Chrome forgets the focus position when an element blurs and Tab would start again from the top of the page. While the caret is in a code block, its header shows a quiet hint in `text-xs text-text-tertiary`: "Esc then Tab to leave". - **Table cell:** unchanged. Lexical's table handler moves between cells, and Tab in the last cell moves on to the block after the table. - **Everything else** (headings, paragraphs, callouts, quotes, or a selection spanning a list and prose): the browser moves focus. The document is never changed.

    Keyboard path, before: Tab into the editor, then every Tab or Shift+Tab stays in the editor and changes the document. Keyboard path, after: Tab into the editor, then Tab moves on to the next control and Shift+Tab goes back. Only list nesting and code indentation, where they apply, keep the key.

    ## Screenshots

    Code block while editing, with the hint in its header:

    ![Code block hint](https://github.com/user-attachments/assets/1c1d8204-7a8d-4286-9a9b-5e7d421a954e)

    ## Testing

    - Reproduced on a local fake-GitHub server before the fix (heading padded `calc(3 * …)`, `\t` written into the source). After the fix: the first-item Tab leaves with identical DOM; code Tab writes `\tb();`, Shift+Tab removes it, and Escape then Tab moves focus to the next control; a list-to-paragraph selection leaves unchanged. - `bun test packages/editor`: 465 pass. `tab-key.test.ts` covers `tabAction`, `leadingIndent`, `$tabContext` (first item, nested item, selection including the first item, code, list-into-paragraph), and `$untab` on tab nodes and imported spaces. - `bun run ci`, `bun run types`: pass. - New e2e in `e2e/editing.e2e.ts`: - A heading leaves without changing the source. - A list item nests and un-nests. - Tab on a first item leaves with no DOM change and no change on a second participant's view. - Code: Tab, Shift+Tab, the hint, and Escape then Tab. - Table: cell to cell, then out of the last cell. - A list-to-paragraph selection leaves unchanged.

    CI e2e: 338 passed.

    🤖 Generated with [Claude Code](https://claude.com/claude-code)

    </details>

  2. MA
    Author

    **Review:** Changes needed

    Checked in a real browser (keyboard only, Chromium, 2 users) against main. Tab and Shift+Tab now behave as described for headings, paragraphs, quotes and tables. List nesting from mid-item, code Tab, and no-op Tab on prose all work. `bun test tab-key.test.ts` and `bun run ci` pass. One real bug and two recommendations.

    **Verified OK** - Shell to editor to beyond: Tab from the editor's last prose goes to the next control ("Show source in chat", then the decision card controls); Shift+Tab goes back. Source and DOM unchanged after Tab/Shift+Tab/Tab-mashing in headings, paragraphs and quotes. The second user sees no `padding-inline-start` and no `\t` (on main, mid-paragraph Tab wrote `\t` into the source). - `- one / two`, Tab at end of `two` gives ` - two` in the source. Shift+Tab restores it. Ordered lists the same. - Table cells: Tab/Shift+Tab move between cells. Tab from the last cell moves to the paragraph after the table, and the next Tab leaves. Shift+Tab in the first cell leaves. - Code block: Tab inserts `\t`, Enter then Tab inserts a second, source stays a valid fence. Shift+Tab leaves. - Slash menu: Tab still selects the item (`slash.tsx` uses `KEY_DOWN_COMMAND`, which runs before `KEY_TAB_COMMAND`). - Priority: Lexical table's Tab handler is `COMMAND_PRIORITY_HIGH` and collapsed-selection only, so it still wins in cells. This plugin at LOW runs before `registerTabIndentation` at EDITOR. A non-collapsed selection inside a cell falls to this plugin and now leaves instead of indenting, which is fine. - Undo history: Tab on prose adds no history entry. List nest/un-nest/undo/redo is identical to main.

    **1. Phantom indentation survives for list items with no valid target (`packages/editor/src/widgets/tab-key.tsx:77-80`)** Tab on the first item of a list, or on an item already nested one level below its previous sibling, still dispatches `INDENT_CONTENT_COMMAND`. Lexical wraps it in empty `ul > li > ul` levels. Reproduced: `- one / two / three`, caret in `one`, Tab x3 gives the source still `- one\n- two\n- three`, but the DOM (and the second user's view) is `ul>li>ul>li>ul>li>ul>li "one"`, four levels deep, and Shift+Tab selecting `two..three` nests the whole list the same way. That is exactly the class of bug this PR sets out to fix (collaborators see an edit the MDX cannot express). Before, it needed the caret at the item start. Now it works from anywhere in the item, so Tab-mashing is easy. Suggested fix: make `tabAction` take what the list can do. For "list", indent only if every selected item has a previous sibling `ListItemNode`, whose own nesting is at most the same depth; outdent only if the item's list has a parent `ListItemNode`; otherwise `"leave"`. This also fixes #2 for the first item and top-level Shift+Tab. Add a `$tabPlace` unit case for first item and an e2e: Tab on a first item must not change `li li` count.

    **2. Accessibility: lists and code still trap Tab (WCAG 2.1.2 No Keyboard Trap)** - List: Tab on a non-first item nests, Shift+Tab at top level keeps focus in the editor (verified: two Shift+Tab on a top-level item stay in the textbox). Fix #1 resolves top-level Shift+Tab and Tab on a first item. Remaining forward trap on non-first items is the usual editor trade-off. - Code: Tab types a tab and only Shift+Tab leaves, so there is no forward exit and the direction is non-obvious. Keyboard-only users can only go backwards. - Escape already works as the standard escape: Lexical's `KEY_ESCAPE_COMMAND` blurs the editor. I verified `Escape` in a list and in code, then `Tab`, lands on the next control after the editor. It is undiscoverable, though. 2.1.2 passes if the exit method is documented to the user. Recommendation: keep Tab-types-tab in code and Tab-nests-in-list (editor convention), make Escape the documented exit (WAI-ARIA APG, same as CodeMirror/Monaco's "Tab moves focus" toggle), drop the asymmetric Shift+Tab-leaves-code rule (Shift+Tab in code should outdent the line, or insert nothing, consistent with CodeMirror), and expose the hint, for example in the code block's language-menu tooltip or a one-line `aria-description`/live hint on the editor ("Press Escape, then Tab to leave"). If the Shift+Tab leave rule stays, add an e2e for it and document it, since it is the only forward-less exit.

    **3. Tests** Coverage is meaningful for prose and a simple list. Missing: first-item/phantom case (see 1), code-block Tab and Shift+Tab, a table cell, a spanning selection (`$tabPlace` unit test exists, but no browser check), and a collaborator-sees-nothing assertion. Also note in e2e that clicking then immediately pressing Tab can race Lexical's selection sync, which makes a bare click+Tab test flaky. The new tests press after typing, so they are fine.

    No style, token or unrelated-change issues. Conventions (tabs, `let`, comments) are fine.

  3. MA
    Author

    Addressed in ad0ab8f9. CI is green (e2e: 338 passed).

    1. **Phantom indentation:** Tab indents only when every selected item has a previous list item to nest under and is below the lists plugin's depth limit (7). Shift+Tab outdents only when every selected item's list is itself nested. Otherwise both return `leave`. Unit tests cover the first item, a nested item, and a selection that includes the first item. The new e2e checks that Tab on a first item leaves the editor with identical DOM, and that a second participant sees no nested item after a later barrier edit. 2. **Code:** Tab inserts a tab. Shift+Tab removes one leading tab, or up to four leading spaces, from the caret's line, and otherwise does nothing (`leadingIndent` and `$untab` are unit-tested). One deviation: Lexical's Escape `blur()` made Chrome lose the focus position, so the following Tab jumped to the top of the page (the sidebar). Escape inside a code block now arms the next Tab to leave instead, as CodeMirror does; Escape elsewhere is unchanged. Esc then Tab now reaches the next control (the block's language menu). The header hint "Esc then Tab to leave" (`text-xs text-text-tertiary`) shows only while the caret is in that code block and the editor has focus. 3. **Browser tests added:** code (Tab, Shift+Tab, hint, Escape then Tab), table (cell to cell, then out of the last cell into the next block), and a list-to-paragraph selection (Tab leaves, DOM and source unchanged).

  4. MaggieAppletonreferenced this in Replace the native link prompt with an inline link popover #298last month on GitHub

  5. lavaman131referenced 7345fa769812last month on GitHub

  6. MaggieAppletonhead-ref-force-pushed 51f600b77aeelast month on GitHub

  7. MaggieAppletonhead-ref-force-pushed 3fa78b898cablast month on GitHub

  8. MaggieAppletonhead-ref-force-pushed ff748b2868aalast month on GitHub

  9. MaggieAppletonhead-ref-force-pushed 8550c3eea6balast month on GitHub

  10. MaggieAppletonhead-ref-force-pushed 24b1a89d493dlast month on GitHub

  11. MaggieAppletonadded label maintenance:workinglast month on GitHub

  12. MA
    Author

    <!-- chopin:pr-maintenance:v1 --> **PR maintenance: working**

    rebase in progress.

    Inspected head: 24b1a89d493dda855d5bc391f51aac821712c0a1

    Base head: 0df386224b433ec28c82ea78c0fe4652293d5255

    [Coordinator run](https://github.com/githubnext/chopin/actions/runs/37649768953) · [CI run](https://github.com/githubnext/chopin/actions/runs/37632353524) <!-- chopin:pr-maintenance-auth:eyJwYXlsb2FkIjp7InNjaGVtYVZlcnNpb24iOjEsInJlcG9zaXRvcnkiOiJnaXRodWJuZXh0L2Nob3BpbiIsInJldmlzaW9uIjowLCJwcnMiOnsiMjkyIjp7ImJvZHkiOiI8IS0tIGNob3Bpbjpwci1tYWludGVuYW5jZTp2MSAtLT5cbioqUFIgbWFpbnRlbmFuY2U6IHdvcmtpbmcqKlxuXG5yZWJhc2UgaW4gcHJvZ3Jlc3MuXG5cbkluc3BlY3RlZCBoZWFkOiAyNGIxYTg5ZDQ5M2RkYTg1NWQ1YmMzOTFmNTFhYWM4MjE3MTJjMGExXG5cbkJhc2UgaGVhZDogMGRmMzg2MjI0YjQzM2VjMjhjODJlYTc4YzBmZTQ2NTIyOTNkNTI1NVxuXG5bQ29vcmRpbmF0b3IgcnVuXShodHRwczovL2dpdGh1Yi5jb20vZ2l0aHVibmV4dC9jaG9waW4vYWN0aW9ucy9ydW5zLzM3NjQ5NzY4OTUzKSDCtyBbQ0kgcnVuXShodHRwczovL2dpdGh1Yi5jb20vZ2l0aHVibmV4dC9jaG9waW4vYWN0aW9ucy9ydW5zLzM3NjMyMzUzNTI0KSIsImJsb2NrZXJJZCI6bnVsbH19fSwic2lnbmF0dXJlIjoiMzI5MzIwZDlhMTk2ZWJkMjA0MTQwZmNmY2UyYWM2MTQwODJkZmU1YWFkYzhjNGRmOGM3Nzg2ZTZmMDI4OWQ2NSJ9 -->

  13. MaggieAppletonreferenced this in fix: let the PR babysitter test and report inside its sandbox #333last month on GitHub

  14. <!-- chopin:pr-maintenance:v1 --> **PR maintenance: working**

    retrying in progress.

    Inspected head: 24b1a89d493dda855d5bc391f51aac821712c0a1

    Base head: dced9b21e5f7e82701e10dc2a468076a957ca26f

    [Coordinator run](https://github.com/githubnext/chopin/actions/runs/37650852920) · [CI run](https://github.com/githubnext/chopin/actions/runs/37632353524) <!-- chopin:pr-maintenance-auth:eyJwYXlsb2FkIjp7InNjaGVtYVZlcnNpb24iOjEsInJlcG9zaXRvcnkiOiJnaXRodWJuZXh0L2Nob3BpbiIsInJldmlzaW9uIjowLCJwcnMiOnsiMjkyIjp7ImJvZHkiOiI8IS0tIGNob3Bpbjpwci1tYWludGVuYW5jZTp2MSAtLT5cbioqUFIgbWFpbnRlbmFuY2U6IHdvcmtpbmcqKlxuXG5yZXRyeWluZyBpbiBwcm9ncmVzcy5cblxuSW5zcGVjdGVkIGhlYWQ6IDI0YjFhODlkNDkzZGRhODU1ZDViYzM5MWY1MWFhYzgyMTcxMmMwYTFcblxuQmFzZSBoZWFkOiBkY2VkOWIyMWU1ZjdlODI3MDFlMTBkYzJhNDY4MDc2YTk1N2NhMjZmXG5cbltDb29yZGluYXRvciBydW5dKGh0dHBzOi8vZ2l0aHViLmNvbS9naXRodWJuZXh0L2Nob3Bpbi9hY3Rpb25zL3J1bnMvMzc2NTA4NTI5MjApIMK3IFtDSSBydW5dKGh0dHBzOi8vZ2l0aHViLmNvbS9naXRodWJuZXh0L2Nob3Bpbi9hY3Rpb25zL3J1bnMvMzc2MzIzNTM1MjQpIiwiYmxvY2tlcklkIjpudWxsfX19LCJzaWduYXR1cmUiOiI3ZDVmNjk2ZDFiMzE0MTIwNWYyOWQyNDhiYzg1ZjI3OGE3NDNiYzgwNDBjOWRkMGRjZGE5M2ZjOWM2OGRmNjkyIn0= -->

  15. MaggieAppletonhead-ref-force-pushed d935d99c51f4last month on GitHub

  16. MaggieAppletonhead-ref-force-pushed be4a889e0ef9last month on GitHub

  17. MaggieAppletonmerged 935d6611d61flast month on GitHub

  18. MaggieAppletonclosedlast month on GitHub

  19. MaggieAppletonhead-ref-deletedlast month on GitHub

  20. MaggieAppletonreferenced this in Quiet rendered block chrome and steady diagram previews #370last month on GitHub