diff --git a/.claude/skills/mass-bug-fixing/SKILL.md b/.claude/skills/mass-bug-fixing/SKILL.md index 165c99148..19dd9873d 100644 --- a/.claude/skills/mass-bug-fixing/SKILL.md +++ b/.claude/skills/mass-bug-fixing/SKILL.md @@ -3,7 +3,8 @@ name: mass-bug-fixing description: | Process multiple bugs from a Fizzy board (or GitHub issues) in parallel. Uses worktree agents to reproduce, fix, test, create PRs, and comment on cards. -invocation: user +invocation: auto_and_user +auto_trigger: when the user asks to fix more than one bug at a time (e.g., provides multiple bug URLs, card numbers, or issue references) user_invocation: /mass-bug-fixing --- diff --git a/.github/workflows/benchmarks.yml b/.github/workflows/benchmarks.yml index bde42f6c2..1c6f716fc 100644 --- a/.github/workflows/benchmarks.yml +++ b/.github/workflows/benchmarks.yml @@ -28,7 +28,7 @@ jobs: - name: Set up Node.js uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0 with: - node-version: "20" + node-version: "22" cache: "yarn" - name: Install JavaScript dependencies diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1d234e729..f17616fcf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,7 +20,7 @@ jobs: persist-credentials: false - name: Set up Ruby - uses: ruby/setup-ruby@319994f95fa847cf3fb3cd3dbe89f6dcde9f178f # v1.295.0 + uses: ruby/setup-ruby@3ff19f5e2baf30647122352b96108b1fbe250c64 # v1.299.0 with: ruby-version: .ruby-version bundler-cache: true @@ -28,7 +28,7 @@ jobs: - name: Set up Node.js uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0 with: - node-version: '20' + node-version: '22' cache: 'yarn' - name: Install JavaScript dependencies @@ -73,7 +73,7 @@ jobs: - name: Set up Node.js uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0 with: - node-version: '20' + node-version: '22' cache: 'yarn' - name: Install JavaScript dependencies @@ -87,6 +87,19 @@ jobs: runs-on: ubuntu-latest permissions: contents: read + + strategy: + fail-fast: false + matrix: + include: + - name: "Rails 8.1 (no adapter)" + use_legacy_rails: "true" + - name: "Rails main (with adapter)" + use_legacy_rails: "" + + env: + USE_RAILS_WITHOUT_ACTION_TEXT_ADAPTER: ${{ matrix.use_legacy_rails }} + steps: - name: Install packages run: sudo apt-get update && sudo apt-get install --no-install-recommends -y build-essential git libyaml-dev pkg-config google-chrome-stable mupdf build-essential ffmpeg groff libreoffice-writer libreoffice-impress libreoffice-calc mupdf-tools libvips zip @@ -97,15 +110,16 @@ jobs: persist-credentials: false - name: Set up Ruby - uses: ruby/setup-ruby@319994f95fa847cf3fb3cd3dbe89f6dcde9f178f # v1.295.0 + uses: ruby/setup-ruby@3ff19f5e2baf30647122352b96108b1fbe250c64 # v1.299.0 with: ruby-version: .ruby-version bundler-cache: true + cache-version: ${{ matrix.name }} - name: Set up Node.js uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0 with: - node-version: '20' + node-version: '22' cache: 'yarn' - name: Install JavaScript dependencies @@ -125,7 +139,7 @@ jobs: uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 if: failure() with: - name: screenshots + name: screenshots-${{ matrix.name }} path: ${{ github.workspace }}/test/dummy/tmp/screenshots if-no-files-found: ignore @@ -147,15 +161,31 @@ jobs: - name: Set up Node.js uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0 with: - node-version: '20' + node-version: '22' cache: 'yarn' - name: Install JavaScript dependencies run: yarn install --frozen-lockfile - - name: Install Playwright browsers + - name: Get Playwright version + id: playwright-version + run: echo "version=$(npx playwright --version)" >> "$GITHUB_OUTPUT" + + - name: Cache Playwright browsers + id: playwright-cache + uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 + with: + path: ~/.cache/ms-playwright + key: playwright-${{ matrix.browser }}-${{ steps.playwright-version.outputs.version }} + + - name: Install Playwright browser + if: steps.playwright-cache.outputs.cache-hit != 'true' run: npx playwright install --with-deps ${{ matrix.browser }} + - name: Install Playwright system deps + if: steps.playwright-cache.outputs.cache-hit == 'true' + run: npx playwright install-deps ${{ matrix.browser }} + - name: Run Playwright tests run: yarn test:browser:${{ matrix.browser }} diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 2645b0803..e8fa2f4f4 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -30,7 +30,7 @@ jobs: persist-credentials: false - name: Setup Ruby - uses: ruby/setup-ruby@319994f95fa847cf3fb3cd3dbe89f6dcde9f178f # v1.295.0 + uses: ruby/setup-ruby@3ff19f5e2baf30647122352b96108b1fbe250c64 # v1.299.0 with: ruby-version: '3.3' bundler-cache: true @@ -38,7 +38,7 @@ jobs: - name: Setup Pages id: pages - uses: actions/configure-pages@983d7736d9b0ae728b81ab479565c72886d7745b # v5.0.0 + uses: actions/configure-pages@45bfe0192ca1faeb007ade9deae92b16b8254a0d # v6.0.0 - name: Build with Jekyll run: bundle exec jekyll build --baseurl "${STEPS_PAGES_OUTPUTS_BASE_PATH}" @@ -63,4 +63,4 @@ jobs: steps: - name: Deploy to GitHub Pages id: deployment - uses: actions/deploy-pages@d6db90164ac5ed86f2b6aed7e0febac5b3c0c03e # v4.0.5 + uses: actions/deploy-pages@cd2ce8fcbc39b97be8ca5fce6e763baed58fa128 # v5.0.0 diff --git a/.gitignore b/.gitignore index 4152d107a..19a901281 100644 --- a/.gitignore +++ b/.gitignore @@ -20,6 +20,8 @@ /test/browser/playwright-report/ /test/browser/blob-report/ +Gemfile.lock + # Jekyll docs /docs/_site/ /docs/.jekyll-cache/ diff --git a/AGENTS.md b/AGENTS.md index 065ad9405..c489565e8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -49,12 +49,14 @@ When changing how Lexxy formats or serializes content (new inline styles, node t ## Fixing Bugs +**Important:** Some bugs filed against Lexxy are actually bugs in the host application (e.g., BC5 modal behavior, server-side preview rendering, Turbo interactions). When a bug's root cause is in a 37signals app rather than in Lexxy itself, use the `/bugs-fix` skill from `basecamp/coworker` to fix it in the appropriate app repo (e.g., `~/Work/basecamp/bc3`). The workflow below applies only to bugs whose root cause is in Lexxy. + Follow this mandatory workflow. Every step must complete before moving to the next. -1. **Classify** — Determine if the bug is a **core editing bug** (JS behavior: typing, cursor, selection, formatting, paste, toolbar, nodes, extensions) or a **system-level bug** (e.g., Action Text, uploads, Trix conversion, persistence, prompt/SGID resolution, Turbo). +1. **Classify** — Determine if the bug is a **core editing bug** (JS behavior: typing, cursor, selection, formatting, paste, toolbar, nodes, extensions), a **system-level bug** (e.g., Action Text, uploads, Trix conversion, persistence, prompt/SGID resolution, Turbo), or a **host app bug** (e.g., BC5 modal/composer behavior, server-side preview rendering, Turbo frame interactions). Host app bugs should be fixed in the app repo using `/bugs-fix` from `basecamp/coworker`, not here. 2. **Reproduce** — Use `/bugs-reproducer` to write a failing test. This skill is ONLY for reproduction — do not investigate fixes or modify source code during this step. Core editing bugs go to Playwright (`test/browser/tests/`), system-level bugs go to Capybara (`test/system/`). Write the test, run it, and **confirm it fails before touching any source code.** A test that hasn't been seen failing proves nothing — it could be passing for the wrong reason. The failing test is your evidence that the bug is real. Do NOT skip this step even if the root cause seems obvious. If you have a justified reason to skip (e.g., the bug is purely visual and untestable), state the justification explicitly before proceeding. 3. **Fix** — Now investigate the root cause and work on the fix until the reproduction test passes. 4. **Test** — Ensure a regression test covers the bug. If you reproduced with Playwright or Capybara in step 2, you already have it. If you used Selenium as a fallback, write a proper test now. Then run the full test suite (`yarn test:browser` for Playwright, `bin/rails test:all` for Capybara) to discard regressions. **Always run `yarn lint` after making changes and fix any lint errors before committing.** -5. **PR & Copilot review** — Push the branch, create a PR, and request a GitHub Copilot review (`gh api repos///pulls//requested_reviewers -f "reviewers[]=copilot-pull-request-reviewer[bot]"`). When the review arrives, reply to each comment on GitHub. For actionable feedback (real bugs, internal API misuse, flaky test patterns), write a test that validates the issue first, then fix it. Decline cosmetic or low-value suggestions with a brief rationale. +5. **PR & Copilot review** — Push the branch, create a **draft** PR (`gh pr create --draft`), and request a GitHub Copilot review (`gh api repos///pulls//requested_reviewers -f "reviewers[]=copilot-pull-request-reviewer[bot]"`). When the review arrives, reply to each comment on GitHub. For actionable feedback (real bugs, internal API misuse, flaky test patterns), write a test that validates the issue first, then fix it. Decline cosmetic or low-value suggestions with a brief rationale. 6. **Validate** — Run `/manually-validate-fix ` to validate the fix against both production (bug reproduces) and local (bug fixed). This step is mandatory after every PR — it catches cases where the automated test passes but the actual user-facing behavior is still broken. 7. **Learn** — If the bug revealed a **family of bugs** (an architectural pattern or subsystem that produces recurring issues), update the "Common Bug Patterns" section in `/bugs-reproducer` (`.claude/skills/bugs-reproducer.md`). Each entry should describe a class of bugs, not an individual fix. Good: "Decorator node navigation" (a whole category). Bad: "Backtick shortcut only works one direction" (a single bug). Don't add notes about specific bugs — that's what the tests and commit messages are for. diff --git a/AUDIT-block-editing-standalone.md b/AUDIT-block-editing-standalone.md new file mode 100644 index 000000000..8c9739428 --- /dev/null +++ b/AUDIT-block-editing-standalone.md @@ -0,0 +1,321 @@ +# `block-editing-standalone` branch audit vs basecamp/main + +> Generated 2026-04-16. Compared HEAD on `block-editing-standalone` against +> `origin/main` (= basecamp/lexxy) at `3d3b62eb`. +> +> **Branch posture:** +> - Fork point: `3735c0ad` ("Version 0.9.3.beta") +> - HEAD: 125 commits ahead of fork point +> - basecamp/main: 116 commits past fork point on its own line of work +> - `headwayio/main` and local `main` were both pinned at the fork point +> when this audit was written; local `main` has since been retracked to +> follow `origin/main` directly. +> +> Two huge net-new files: `block_selection_extension.js` (4499 lines), +> `block_actions_menu.js` (667 lines), plus `block_drag_and_drop.js` +> (2279 lines) and a refactored `attachment_controls.js` / preview +> pipeline. Five new Playwright test files added in this audit session +> (24 tests, all passing). + +--- + +## Test work completed in this session + +| File | Tests | Plan section | +|---|---|---| +| `test/browser/tests/block_editing/wrapped_block_origin.test.js` | 5 | P1.1 | +| `test/browser/tests/block_editing/single_undo_correctness.test.js` | 4 | P1.3 | +| `test/browser/tests/block_editing/table_block_select.test.js` | 5 | P1.4 | +| `test/browser/tests/block_editing/hr_block_select.test.js` | 4 | P1.5 | +| `test/browser/tests/block_editing/attachment_block_select.test.js` | 6 | P1.2 | + +Block-editing suite: 79 → **103 tests** passing. + +Two contract clarifications surfaced during implementation: + +1. **The Cmd+/ menu's `table` rule disables every Turn into command.** The + underlying multi-block conversion (commit fc4d137f) supports wrapping a + table in a list/quote, but the menu restriction is + `table: { commands: new Set(), color: true }` — so the wrap path is only + reachable via a multi-block selection that anchors focus on a + non-restricted block. Either widen the menu rule (allow `LISTS` + + `insertQuoteBlock` like the `decorator` rule), or document the + discrepancy as intentional. + +2. **`setValue` always loads wrapped blocks as user-origin.** The fallback + in `#isUserWrapped` means there's no way to test a movement-wrapped item + without actually executing arrow movement to drift one in. Tests in + `wrapped_block_origin.test.js` go through the drift sequence end-to-end. + +P2 and P3 tests were not implemented — see `PLAN-block-editing-regression-tests.md` for the rest. + +--- + +## 1. Things basecamp added after the fork point — pick what you want + +(Not "you're behind." This branch developed in parallel with basecamp's +own line of work. These are upstream items worth considering on their own +merits, not because of any rebase pressure.) + +### 1A — XSS in `CustomActionTextAttachmentNode` (HIGH severity, security) + +`src/nodes/custom_action_text_attachment_node.js:77` + +```js +createDOM() { + const figure = createElement(this.tagName, { "content-type": ..., "data-lexxy-decorator": true }) + figure.insertAdjacentHTML("beforeend", this.innerHtml) // unsanitized + ... +} +``` + +basecamp commit `2ef7d8bc` wraps the call in `sanitize(...)`. `this.innerHtml` originates from `parseAttachmentContent(attachment.getAttribute("content"))` in `importDOM`, which does no sanitization. So `` survives import and executes when the node renders. `CustomActionTextAttachmentNode` is registered at `src/elements/editor.js:338` and instantiated by mention/prompt insertion at `src/elements/prompt.js:459`. + +**Fix:** import `sanitize` from `src/helpers/sanitization_helper.js` and wrap the `insertAdjacentHTML` call. Cherry-pick `2ef7d8bc` if the surrounding context still applies. Worth doing regardless of any rebase strategy. + +### 1B — `link_opener_extension.js` (Cmd/Ctrl+click to open links) — MEDIUM + +basecamp commit `20c95582` adds an extension that watches the modifier key and conditionally adds `target="_blank" rel="noopener noreferrer"` to anchors so plain Cmd+click opens links. Searching `src/` for `link-opener|LinkOpener|linkOpener` returns zero matches in our fork. + +User-visible: Cmd+click on a link does nothing in our fork (or moves the caret). + +**Fix:** cherry-pick the upstream extension. It depends on `registerEventListener` from basecamp's `listener_helper.js` (1D below) so either restore that helper or rewrite the extension to use raw listeners. + +### 1C — `clearFormatting` command + button — MEDIUM + +basecamp commit `3b683785` adds a "Clear formatting" toolbar button and `dispatchClearFormatting` to the dispatcher. Our `src/editor/command_dispatcher.js` has neither in its `COMMANDS` array nor a method. Users on our fork have no built-in way to strip inline styling from a selection. + +### 1D — `listener_helper.js` (`registerEventListener` + `ListenerBin`) — MEDIUM + +basecamp introduced a memory-safe listener registration helper: + +```js +export function registerEventListener(element, type, listener, options) { + element.addEventListener(type, listener, options) + const elementRef = new WeakRef(element) + const listenerRef = new WeakRef(listener) + return () => { + const listener = listenerRef.deref() + if (listener) elementRef.deref()?.removeEventListener(type, listener, options) + } +} + +export class ListenerBin { ... } +``` + +Then converted every element/extension to use it (basecamp commits `3a9f25b2` through `b9457a32`). Our fork: + +- Doesn't have the helper (it was added on basecamp's side after our fork point). +- Has **97 raw `addEventListener` calls** vs **63 raw `removeEventListener` calls** across `src/`. The 34-call gap suggests at least *some* listeners are never removed. +- Risky callers: `src/editor/block_drag_and_drop.js` (17 add — needs eyeballing); `src/elements/block_actions_menu.js` (8 add, 8 remove — balanced); `src/extensions/block_selection_extension.js` (7 add — most go through `cleanupFns`). + +**Fix:** import the helper and migrate, or audit the 34-call gap to confirm none are leaks. Without `WeakRef` semantics, even paired add/remove can leak if an exception in between skips removal. + +### 1E — Markdown `---` shorthand for HR — MEDIUM + +basecamp has `src/editor/markdown/horizontal_divider_transformer.js`: + +```js +export const HORIZONTAL_DIVIDER = { + dependencies: [HorizontalDividerNode], + regExpStart: /^-{3,}\s?$/, + replace: (parentNode, ...) => { parentNode.replace(new HorizontalDividerNode()); ... } +} +``` + +We don't have it. We added a *different* transformer (`src/editor/markdown/list_heading_shortcut.js`) for `# `, `## `, `### `, `#### `, `> ` inside list items — additive, not a replacement. Users on our fork who type `---` and Enter get a literal `---` paragraph. + +**Fix:** restore the transformer (basecamp commit `6f3344cc`). + +### 1F — Lexical 0.41 → 0.42 dependency bump + +Our `package.json` pins `^0.41.0`; basecamp uses `^0.42.0` (commit `f78352e7`, 16 lexical packages). Pinning to 0.41 is a defensible choice if we've validated against it; this is informational. Lexical 0.42 reportedly requires Node 22+ (Iterator helpers). + +### 1G — Rails 8.2 `ActionText::Editor` adapter (#778) + +basecamp commit `b9457a32` switches the engine to use Rails 8.2's `ActionText::Editor` adapter API when available, falling back to the prepend/monkey-patch on older Rails. Our `lib/lexxy/engine.rb` is still prepend-only. On Rails 8.2, our gem still works (prepend wins) but uses the deprecated path. Will eventually break. + +### 1H — Smaller upstream changes + +| Change | basecamp commit | Notes | +|---|---|---| +| Unwrap link dialog form | `9a4b622f` | UX polish on the link toolbar dropdown | +| Toolbar focus fix | `ae6c2757` | Prevents toolbar buttons from stealing focus | +| Link-change unwraps autolink | `3d3b62eb` | Edge case in `dropdown/link.js` | +| Prompt filtering tweaks | `7cef7351` | Empty-string-instead-of-null for link URL | + +--- + +## 2. Suspected regressions and bugs in our own code + +### 2A — Inconsistent `"history-push"` string literal vs `HISTORY_PUSH_TAG` constant — LOW (latent) + +`src/extensions/block_selection_extension.js:1143, 1709` use the string literal `"history-push"`. The other two callsites (`1407`, `4316`) use the imported `HISTORY_PUSH_TAG` constant. The constant is already imported on line 21. + +```js +// Line 1143 +}, { tag: "history-push" }) + +// Line 4316 +}, { tag: HISTORY_PUSH_TAG }) +``` + +Lexical 0.41's `HISTORY_PUSH_TAG` happens to equal `"history-push"`, so this works today. If Lexical ever changes the value (which has happened — `HISTORY_MERGE_TAG` was renamed/reassigned previously), the two literal callsites silently produce the wrong undo behavior at exactly the locations the rest of the file works hardest to keep right. + +**Fix:** replace both literals with `HISTORY_PUSH_TAG`. One-line change. + +### 2B — Six `try { } catch (_) { /* ignore */ }` blocks — MEDIUM + +`src/extensions/block_selection_extension.js`: + +```js +// 1707 +try { this.#resyncWrappedKeys() } catch (_) { /* nodes may have been removed */ } +// 1714, 1723 +try { this.editor.update(...) } catch (_) { /* nodes may have been unwrapped/removed */ } +// 3231, 3244 +try { /* lookup */ } catch (e) { /* ignore */ } +// 1501 +try { insertAfterNode.insertAfter(clone) } catch (_) { /* fallback at root */ } +``` + +Each says "node may have been removed" but catches every error type. The 1707 callsite is in the hot `#moveSelectedBlocks` path — selection state corruption from a swallowed exception would be hard to attribute. + +**Fix:** narrow the catches. For deleted-node cases, check `node.getParent() === null` or `$getNodeByKey(key) === null` *before* the operation. Keep the insert-fallback catch (1501) but log to `console.warn`. + +### 2C — `#matchesKeySet` is O(N) per call, called O(M) times — MEDIUM (perf) + +`src/extensions/block_selection_extension.js:3242` + +```js +#matchesKeySet(node, keySet) { + for (const key of keySet) { + try { + const tracked = $getNodeByKey(key) + if (tracked && tracked.is(node)) return true + } catch (e) { /* ignore */ } + } + return false +} +``` + +Called from `#isUserWrapped` (line 3214) which is called per block during sync. On a 200-block doc with 50 wrapped items, that's 10 000 lookups + try/catch instantiations per sync. + +**Fix:** check `key === node.getKey()` directly first to avoid the inner $getNodeByKey. Drop the catch (`$getNodeByKey` returns null, doesn't throw, in current Lexical). Or store node refs / use a `WeakMap`. + +### 2D — Excessive `requestAnimationFrame` and double-RAF patterns — LOW (perf) + +19 `requestAnimationFrame`/`setTimeout` calls in `block_selection_extension.js`. Several are double-RAF stacks: + +```js +requestAnimationFrame(() => { + this.#syncSelectionClasses() + this.#syncWrappedBlockAttributes() + requestAnimationFrame(() => { + this.#dragAndDrop?.repositionHandle() + this.#syncBulletOffsets() + ... + }) +}) +``` + +The pattern is documented (first RAF waits for Lexical reconciliation, second for layout) and probably necessary, but each user action defers up to 4 distinct frames of work. Recommend a manual perf pass on a doc with 500+ blocks and 50+ wrapped items before shipping. + +### 2E — Per-call `#getDocumentOrderBlockKeys()` walks the entire document — LOW (perf) + +`src/extensions/block_selection_extension.js:455`. Called from many sites. Each call does a recursive tree walk. Per-action this is `O(N)` once and fine; the risk is multi-block selections that loop through selected keys and call helpers per-key, accidentally producing `O(N×M)` walks. **Fix:** memoize per-`editor.update` boundary or pass the result as a param. + +### 2F — `data-block-movement-wrapped` DOM attribute as origin source-of-truth — LOW (smell) + +`#isWrappedBlock` falls back to reading `data-block-movement-wrapped` from the DOM: + +```js +try { + const el = this.editor.getElementByKey(key) + if (el?.hasAttribute("data-block-movement-wrapped")) return true +} catch (e) { /* ignore */ } +``` + +A DOM attribute as source-of-truth for origin means a Turbo cache restore or server re-render could clear it, silently flipping movement-wrapped to user-wrapped. The `#isUserWrapped` doc comment acknowledges this with the "any unclassified wrapped block is user-wrapped" fallback, but the DOM attribute path adds a third source of truth that's harder to reason about. + +--- + +## 3. Dead / orphaned code + +### 3A — `lexxy_extension.js` lost 4 lines vs basecamp + +Diff vs `origin/main` shows a removed `get allowedElements()` getter. Confirm no subclass relied on it (`grep -rn "allowedElements" src/`). + +### 3B — `provisional_paragraph_extension.js` divergence + +Our fork added `$addUpdateTag(HISTORY_MERGE_TAG)` inside `$markAllProvisionalParagraphsDirty`. The doc comment is excellent — explains why this is needed for the "two undo presses" bug. basecamp doesn't have this, but basecamp also doesn't have the block-actions menu. The change is correct and well-justified for our fork. + +### 3C — `action_text_attachment_upload_node.js` is +49 lines vs basecamp + +Confirm it doesn't conflict with basecamp's "keep cursor location when upload completes" patch (commit `b407c1b6`). + +--- + +## 4. Style/pattern divergences from basecamp + +### 4A — Raw `addEventListener`/`removeEventListener` everywhere — see 1D. + +### 4B — `#scrollY` capture + `queueMicrotask(() => window.scrollTo(...))` + +We have a one-off scroll-preservation pattern at multiple callsites. basecamp introduced a reusable `withPreservedScroll(handler)` wrapper in `command_dispatcher.js`. Cleaner to consolidate. + +### 4C — Private method count in `block_selection_extension.js` + +180 method definitions. The class is doing the work of 4–5 collaborators. See the separate refactor map (in conversation) for the proposed extraction order: `BlockSelectionState` → `WrappedOriginRegistry` → `HighlightInheritance` → `BlockActionsMenuController` → `WrappedBlockIndenter` → `BlockMovementEngine`. + +### 4D — String tags vs constants (covered in 2A). + +--- + +## 5. Test coverage gaps still open after this session + +P1 items in the regression-test plan are now covered. Remaining work — see +`PLAN-block-editing-regression-tests.md`: + +- **P2.1** Inline formatting in block-select multi-block (4 tests) +- **P2.2** Cmd+A escalation (3 tests) +- **P2.3** Cmd+D on wrapped blocks (2 tests) +- **P2.4** Color application on multi-block (5 tests) +- **P2.5** Mixed-selection Shift+Tab (1 test) +- **P3.1** Scroll preservation (7 tests) +- **P3.2** Block actions menu positioning/anchoring (3 tests) +- **P3.3** Keyboard navigation edge cases (3 tests) +- **Capybara round-trip** (9 tests in `test/system/`) + +Audit-driven tests not in the plan that would also be valuable: +- A regression test that asserts `` cannot execute when imported (locks down 1A even after the fix lands). +- A test that types `---` followed by Enter and asserts a horizontal divider appears (will lock in the fix for 1E). +- A leak-detection test that creates and disposes 100 editors and asserts the listener count stays bounded (catches 1D regression). + +--- + +## 6. Suggested triage / merge order + +If this branch is being prepared for upstream contribution or for a release: + +1. **Fix 1A (XSS) immediately.** One-line patch. Currently a security hole. +2. **Fix 2A** (string-tag literals) — trivial and removes a foot-gun. +3. **Pick up the small basecamp commits you want** (link-opener, clearFormatting, markdown HR shorthand) — each is small, additive, low-risk. +4. **Land the P2/P3 tests** (the rest of the regression plan). +5. **Decide on Lexical 0.42** and Rails 8.2 adapter strategy (1F, 1G). +6. **Refactor 4C / 2B / 2C** as part of a cleanup pass once the surface is stable. + +--- + +## Files inspected for this audit + +- `src/extensions/block_selection_extension.js` (4499 lines) +- `src/elements/block_actions_menu.js` (667 lines) +- `src/extensions/provisional_paragraph_extension.js` +- `src/editor/command_dispatcher.js` +- `src/nodes/custom_action_text_attachment_node.js` +- `src/nodes/wrapped_table_node.js` +- `src/editor/markdown/list_heading_shortcut.js` +- `src/helpers/sanitization_helper.js` +- `package.json` +- `test/browser/tests/block_editing/*.js` (existing tests for context) diff --git a/Gemfile b/Gemfile index 097daa3ad..b3100c388 100644 --- a/Gemfile +++ b/Gemfile @@ -3,19 +3,29 @@ source "https://rubygems.org" # Specify your gem's dependencies in actiontext-lexical.gemspec. gemspec -gem "puma" +if ENV["USE_RAILS_WITHOUT_ACTION_TEXT_ADAPTER"] == "true" + gem "rails", "~> 8.1.0" +else + gem "rails", github: "rails/rails", branch: "main" +end -gem "sqlite3" +gem "puma", "~> 7.0" -gem "propshaft" +gem "sqlite3", "~> 2.0" -gem "rubocop-rails-omakase", require: false -gem "importmap-rails" -gem "image_processing" +gem "propshaft", "~> 1.0" -gem "turbo-rails" -gem "stimulus-rails" +gem "rubocop-rails-omakase", "~> 1.0", require: false +gem "importmap-rails", "~> 2.0" +gem "image_processing", "~> 1.0" + +gem "turbo-rails", "~> 2.0" +gem "stimulus-rails", "~> 1.0" gem "rack-cors" gem "foreman" + +gem "capybara", "~> 3.0" +gem "selenium-webdriver", "~> 4.0" +gem "cuprite", "~> 0.17" diff --git a/Gemfile.lock b/Gemfile.lock deleted file mode 100644 index 57a34f86e..000000000 --- a/Gemfile.lock +++ /dev/null @@ -1,335 +0,0 @@ -PATH - remote: . - specs: - lexxy (0.9.3.beta) - rails (>= 8.0.2) - -GEM - remote: https://rubygems.org/ - specs: - action_text-trix (2.1.16) - railties - actioncable (8.1.2) - actionpack (= 8.1.2) - activesupport (= 8.1.2) - nio4r (~> 2.0) - websocket-driver (>= 0.6.1) - zeitwerk (~> 2.6) - actionmailbox (8.1.2) - actionpack (= 8.1.2) - activejob (= 8.1.2) - activerecord (= 8.1.2) - activestorage (= 8.1.2) - activesupport (= 8.1.2) - mail (>= 2.8.0) - actionmailer (8.1.2) - actionpack (= 8.1.2) - actionview (= 8.1.2) - activejob (= 8.1.2) - activesupport (= 8.1.2) - mail (>= 2.8.0) - rails-dom-testing (~> 2.2) - actionpack (8.1.2) - actionview (= 8.1.2) - activesupport (= 8.1.2) - nokogiri (>= 1.8.5) - rack (>= 2.2.4) - rack-session (>= 1.0.1) - rack-test (>= 0.6.3) - rails-dom-testing (~> 2.2) - rails-html-sanitizer (~> 1.6) - useragent (~> 0.16) - actiontext (8.1.2) - action_text-trix (~> 2.1.15) - actionpack (= 8.1.2) - activerecord (= 8.1.2) - activestorage (= 8.1.2) - activesupport (= 8.1.2) - globalid (>= 0.6.0) - nokogiri (>= 1.8.5) - actionview (8.1.2) - activesupport (= 8.1.2) - builder (~> 3.1) - erubi (~> 1.11) - rails-dom-testing (~> 2.2) - rails-html-sanitizer (~> 1.6) - activejob (8.1.2) - activesupport (= 8.1.2) - globalid (>= 0.3.6) - activemodel (8.1.2) - activesupport (= 8.1.2) - activerecord (8.1.2) - activemodel (= 8.1.2) - activesupport (= 8.1.2) - timeout (>= 0.4.0) - activestorage (8.1.2) - actionpack (= 8.1.2) - activejob (= 8.1.2) - activerecord (= 8.1.2) - activesupport (= 8.1.2) - marcel (~> 1.0) - activesupport (8.1.2) - base64 - bigdecimal - concurrent-ruby (~> 1.0, >= 1.3.1) - connection_pool (>= 2.2.5) - drb - i18n (>= 1.6, < 2) - json - logger (>= 1.4.2) - minitest (>= 5.1) - securerandom (>= 0.3) - tzinfo (~> 2.0, >= 2.0.5) - uri (>= 0.13.1) - addressable (2.8.7) - public_suffix (>= 2.0.2, < 7.0) - ast (2.4.3) - base64 (0.3.0) - bigdecimal (4.0.1) - builder (3.3.0) - capybara (3.40.0) - addressable - matrix - mini_mime (>= 0.1.3) - nokogiri (~> 1.11) - rack (>= 1.6.0) - rack-test (>= 0.6.3) - regexp_parser (>= 1.5, < 3.0) - xpath (~> 3.2) - concurrent-ruby (1.3.6) - connection_pool (3.0.2) - crass (1.0.6) - cuprite (0.17) - capybara (~> 3.0) - ferrum (~> 0.17.0) - date (3.5.1) - drb (2.2.3) - erb (6.0.2) - erubi (1.13.1) - ferrum (0.17.1) - addressable (~> 2.5) - base64 (~> 0.2) - concurrent-ruby (~> 1.1) - webrick (~> 1.7) - websocket-driver (~> 0.7) - ffi (1.17.2-aarch64-linux-gnu) - ffi (1.17.2-arm64-darwin) - ffi (1.17.2-x86_64-linux-gnu) - foreman (0.90.0) - thor (~> 1.4) - globalid (1.3.0) - activesupport (>= 6.1) - i18n (1.14.8) - concurrent-ruby (~> 1.0) - image_processing (1.14.0) - mini_magick (>= 4.9.5, < 6) - ruby-vips (>= 2.0.17, < 3) - importmap-rails (2.2.3) - actionpack (>= 6.0.0) - activesupport (>= 6.0.0) - railties (>= 6.0.0) - io-console (0.8.2) - irb (1.17.0) - pp (>= 0.6.0) - prism (>= 1.3.0) - rdoc (>= 4.0.0) - reline (>= 0.4.2) - json (2.18.1) - language_server-protocol (3.17.0.5) - lint_roller (1.1.0) - logger (1.7.0) - loofah (2.25.0) - crass (~> 1.0.2) - nokogiri (>= 1.12.0) - mail (2.9.0) - logger - mini_mime (>= 0.1.1) - net-imap - net-pop - net-smtp - marcel (1.1.0) - matrix (0.4.3) - mini_magick (5.3.1) - logger - mini_mime (1.1.5) - minitest (6.0.2) - drb (~> 2.0) - prism (~> 1.5) - net-imap (0.6.2) - date - net-protocol - net-pop (0.1.2) - net-protocol - net-protocol (0.2.2) - timeout - net-smtp (0.5.1) - net-protocol - nio4r (2.7.5) - nokogiri (1.19.1-aarch64-linux-gnu) - racc (~> 1.4) - nokogiri (1.19.1-arm64-darwin) - racc (~> 1.4) - nokogiri (1.19.1-x86_64-linux-gnu) - racc (~> 1.4) - parallel (1.27.0) - parser (3.3.9.0) - ast (~> 2.4.1) - racc - pp (0.6.3) - prettyprint - prettyprint (0.2.0) - prism (1.9.0) - propshaft (1.2.1) - actionpack (>= 7.0.0) - activesupport (>= 7.0.0) - rack - psych (5.3.1) - date - stringio - public_suffix (6.0.2) - puma (7.2.0) - nio4r (~> 2.0) - racc (1.8.1) - rack (3.2.5) - rack-cors (3.0.0) - logger - rack (>= 3.0.14) - rack-session (2.1.1) - base64 (>= 0.1.0) - rack (>= 3.0.0) - rack-test (2.2.0) - rack (>= 1.3) - rackup (2.3.1) - rack (>= 3) - rails (8.1.2) - actioncable (= 8.1.2) - actionmailbox (= 8.1.2) - actionmailer (= 8.1.2) - actionpack (= 8.1.2) - actiontext (= 8.1.2) - actionview (= 8.1.2) - activejob (= 8.1.2) - activemodel (= 8.1.2) - activerecord (= 8.1.2) - activestorage (= 8.1.2) - activesupport (= 8.1.2) - bundler (>= 1.15.0) - railties (= 8.1.2) - rails-dom-testing (2.3.0) - activesupport (>= 5.0.0) - minitest - nokogiri (>= 1.6) - rails-html-sanitizer (1.7.0) - loofah (~> 2.25) - nokogiri (>= 1.15.7, != 1.16.7, != 1.16.6, != 1.16.5, != 1.16.4, != 1.16.3, != 1.16.2, != 1.16.1, != 1.16.0.rc1, != 1.16.0) - railties (8.1.2) - actionpack (= 8.1.2) - activesupport (= 8.1.2) - irb (~> 1.13) - rackup (>= 1.0.0) - rake (>= 12.2) - thor (~> 1.0, >= 1.2.2) - tsort (>= 0.2) - zeitwerk (~> 2.6) - rainbow (3.1.1) - rake (13.3.1) - rdoc (7.2.0) - erb - psych (>= 4.0.0) - tsort - regexp_parser (2.11.2) - reline (0.6.3) - io-console (~> 0.5) - rexml (3.4.4) - rubocop (1.80.1) - json (~> 2.3) - language_server-protocol (~> 3.17.0.2) - lint_roller (~> 1.1.0) - parallel (~> 1.10) - parser (>= 3.3.0.2) - rainbow (>= 2.2.2, < 4.0) - regexp_parser (>= 2.9.3, < 3.0) - rubocop-ast (>= 1.46.0, < 2.0) - ruby-progressbar (~> 1.7) - unicode-display_width (>= 2.4.0, < 4.0) - rubocop-ast (1.46.0) - parser (>= 3.3.7.2) - prism (~> 1.4) - rubocop-performance (1.25.0) - lint_roller (~> 1.1) - rubocop (>= 1.75.0, < 2.0) - rubocop-ast (>= 1.38.0, < 2.0) - rubocop-rails (2.33.3) - activesupport (>= 4.2.0) - lint_roller (~> 1.1) - rack (>= 1.1) - rubocop (>= 1.75.0, < 2.0) - rubocop-ast (>= 1.44.0, < 2.0) - rubocop-rails-omakase (1.1.0) - rubocop (>= 1.72) - rubocop-performance (>= 1.24) - rubocop-rails (>= 2.30) - ruby-progressbar (1.13.0) - ruby-vips (2.2.5) - ffi (~> 1.12) - logger - rubyzip (3.2.2) - securerandom (0.4.1) - selenium-webdriver (4.41.0) - base64 (~> 0.2) - logger (~> 1.4) - rexml (~> 3.2, >= 3.2.5) - rubyzip (>= 1.2.2, < 4.0) - websocket (~> 1.0) - sqlite3 (2.9.0-aarch64-linux-gnu) - sqlite3 (2.9.0-arm64-darwin) - sqlite3 (2.9.0-x86_64-linux-gnu) - stimulus-rails (1.3.4) - railties (>= 6.0.0) - stringio (3.2.0) - thor (1.5.0) - timeout (0.6.0) - tsort (0.2.0) - turbo-rails (2.0.23) - actionpack (>= 7.1.0) - railties (>= 7.1.0) - tzinfo (2.0.6) - concurrent-ruby (~> 1.0) - unicode-display_width (3.1.5) - unicode-emoji (~> 4.0, >= 4.0.4) - unicode-emoji (4.0.4) - uri (1.1.1) - useragent (0.16.11) - webrick (1.9.1) - websocket (1.2.11) - websocket-driver (0.8.0) - base64 - websocket-extensions (>= 0.1.0) - websocket-extensions (0.1.5) - xpath (3.2.0) - nokogiri (~> 1.8) - zeitwerk (2.7.5) - -PLATFORMS - aarch64-linux - arm64-darwin - x86_64-linux - -DEPENDENCIES - capybara - cuprite - foreman - image_processing - importmap-rails - lexxy! - propshaft - puma - rack-cors - rubocop-rails-omakase - selenium-webdriver - sqlite3 - stimulus-rails - turbo-rails - -BUNDLED WITH - 2.6.5 diff --git a/PLAN-block-editing-regression-tests.md b/PLAN-block-editing-regression-tests.md new file mode 100644 index 000000000..6fd6cc5e8 --- /dev/null +++ b/PLAN-block-editing-regression-tests.md @@ -0,0 +1,396 @@ +# Block-editing regression test plan + +> Filled coverage gaps for the `block-editing-standalone` branch so critical +> paths don't silently regress. Generated 2026-04-16 after a session that +> fixed several bugs (wrapped-block outdent, multi-block Turn into, undo +> double-press, scroll jumps, wrapper restrictions, Remove Quote/Bullet, +> Turn-into-list-wraps-non-paragraph). This plan picks up where that work +> stopped. + +## How to approach this file + +- Each section = one test file or one logical block of tests. +- Priority buckets are ordered: do P1 before P2 before P3. +- Most tests go in `test/browser/tests/block_editing/*.test.js` (Playwright). +- Round-trip tests go in `test/system/*.rb` (Capybara) per AGENTS.md. +- All Playwright tests should use the existing helpers: + - `../../test_helper.js` for the `test` object with `editor`, `page` fixtures + - `../../helpers/html.js` → `normalizeHtml` + - `../../helpers/active_storage_mock.js` → `mockActiveStorageUploads` for + any test that uploads attachments + - Shared `assertBlockHtml` / `stripDynamicAttrs` helpers already exist in + `block_selection.test.js` and `wrapped_block_outdent.test.js` — copy or + import them. +- Keyboard shortcut constant pattern (already in `block_actions_menu.test.js`): + ```js + const modifier = process.platform === "darwin" ? "Meta" : "Control" + ``` +- Rebuild + Rails restart between sessions: + - `rollup -wc` watch should be running (auto-restarts on src/ changes). + - If dev skeleton is used (`/Users/jon/Code/lexxy_test_skeleton` on port + 3003), kill and re-start Rails after any src/ change so the asset + fingerprint refreshes. Playwright tests use `test/dummy/` via Vite on + 5173 — they DO NOT need the skeleton app. + +## Current state + +79 passing tests as of commit fc4d137f. Gaps enumerated below. + +--- + +## P1 — core capabilities with zero or near-zero coverage + +### P1.1 — Origin-aware wrapping (user vs movement) + +File: `test/browser/tests/block_editing/wrapped_block_origin.test.js` (new) + +Why: commit 070621a2 introduced the two-origin system. Turn-into creates +**user-wrapped** items that persist through arrow movement; Cmd+Shift+↓ into a +list creates **movement-wrapped** items that auto-unwrap on exit. No test +currently locks this contract in. + +Tests to add: + +1. **User-wrapped stays wrapped across movement** + ``` +
  • Parent

User heading

+ ``` + - Click "User heading", Turn into Bullet list via menu (wraps it). + - Cmd+Shift+↑ to move it — should nest under "Parent" as a wrapped li. + - Cmd+Shift+↓ twice to move it back out past the list. + - Assert: heading still wrapped in a new root-level `
    `. Not unwrapped. + +2. **Movement-wrapped unwraps on exit** + ``` +

    Drift heading

    • Target
    + ``` + - Select the heading in block-select mode, Cmd+Shift+↓ → heading drifts into + the list (becomes movement-wrapped). + - Cmd+Shift+↓ again to exit past the list. + - Assert: heading is now a **standalone** `

    ` below the list, not a + wrapped li. + +3. **Origin survives depth changes** + - User-wrapped item: Cmd+Shift+↑ to nest deeper, then Cmd+Shift+↓ many times + to exit. It should still be wrapped when it pops out. + - Movement-wrapped item: same journey → unwrapped when it pops out. + +4. **Shift+Tab at root unwraps regardless of origin** + - User-wrapped item at root-level list + Shift+Tab → unwraps. + - Movement-wrapped item at root-level list + Shift+Tab → unwraps. + - (Only arrow movement's boundary exit differentiates origins.) + +### P1.2 — Attachment-specific block-select tests + +File: `test/browser/tests/block_editing/attachment_block_select.test.js` (new) + +Needs: `mockActiveStorageUploads(page)` at the start of each test (see +`test/browser/tests/attachments/attachments.test.js` for the pattern). + +1. **Turn into Bullet on a standalone attachment wraps it** + - Upload a PNG, then Escape to block-select, focus the attachment, + Cmd+/ → Turn into → Bullet list. + - Assert: attachment is now inside `
    `. + - Editor value contains `action-text-attachment` unchanged (sgid + preserved). + +2. **Shift+Tab on a wrapped attachment at root unwraps to standalone** + - Set up the wrapped state from test 1, Shift+Tab. + - Assert: attachment back at root, no surrounding list. + +3. **Remove Bullet on a wrapped attachment unwraps to root** + - Wrapped attachment in a bullet list → menu → Remove Bullet. + - Assert: attachment at root. + +4. **Remove Quote on an attachment wrapped in a blockquote unwraps to root** + - `
    ` → menu → Remove Quote. + - Assert: attachment at root. + +5. **Nested: attachment in blockquote in list, Remove Bullet → root** + - `
    ` + - Remove Bullet (or Remove Quote — either should work per commit d9654ab3). + - Assert: attachment at root, no list, no blockquote. + +6. **Cmd+Shift+↓ movement of a wrapped attachment preserves wrapping** + - Create wrapped attachment next to another block. + - Cmd+Shift+↓ past the sibling → wrapped attachment in a new list below. + +### P1.3 — Single-press undo correctness after Turn into + +File: add to `test/browser/tests/block_editing/block_actions_menu.test.js` +(existing file) in a new `test.describe` block, or a separate file. + +Why: commit 55632e18 switched HISTORY_MERGE_TAG → HISTORY_PUSH_TAG and tagged +the provisional-paragraph follow-up update with HISTORY_MERGE_TAG. Both +bugs caused "takes two undos" behavior. No test locks this in. + +1. **One Cmd+Z after multi-block Turn into Bullet reverts to exactly the + pre-Turn-into state** + - Set up `

    A

    B

    After

    ` + - Select both, Turn into Bullet, Cmd+Z + - Assert: original HTML (not empty, not partial). + +2. **One Cmd+Z after Remove Quote reverts to the blockquote shape** +3. **One Cmd+Z after Remove Bullet reverts to the wrapped-list shape** +4. **One Cmd+Z after multi-block group move reverts to pre-move shape** + - `

    A

    B

    C

    `, select A+B, Cmd+Shift+↓, Cmd+Z → original. + +Optional: register an `editor.registerUpdateListener` at the start of each +test to count the history-push commits; assert that each user action +produces exactly one such entry. (See in-session browser diagnostics used +around commit b29532f0 for the technique.) + +### P1.4 — Tables as wrappable blocks + +File: `test/browser/tests/block_editing/table_block_select.test.js` (new) + +Why: fc4d137f narrowed the table skip guard and enabled Turn into Bullet/Quote +on tables. No test. + +1. **Turn into Bullet on a standalone table wraps it; cells preserved** + - `
    ` with known cells. + - Focus it, Cmd+/ → Turn into → Bullet list. + - Assert: `
    • ……
    `. + - Assert: every `` from the original is still present with its content. + +2. **Turn into Quote on a standalone table wraps it; cells preserved** + +3. **Turn into Text / Heading on a standalone table is a no-op (restriction)** + - The menu should disable these options (existing test covers the menu + state). Also assert that if somehow dispatched, the table remains + intact (regression guard for the skip branch). + +4. **Shift+Tab on a wrapped table at root unwraps to standalone table** + - `
    • …table…
    ` + Shift+Tab + - Assert: standalone table, cells preserved. + +5. **Cmd+Shift+↓ moves a wrapped table past its sibling** + +### P1.5 — HR-specific tests + +File: add a describe to `test/browser/tests/block_editing/block_actions_menu.test.js`, +or a new `hr_block_select.test.js`. + +1. **Turn into Bullet on a standalone `
    ` wraps it** + - `

    Before


    After

    `, focus HR, Turn into Bullet. + - Assert: `

    ` between the paragraphs. + +2. **Cmd+Shift+↓ movement of a wrapped HR** + - Wrapped HR in a list with siblings; move it down → order updates. + +3. **Remove Bullet on a wrapped HR unwraps to root HR** + +4. **Arrow-key navigation skips over HR decorator correctly in block-select** + - Block-select focused above HR, ArrowDown → focus moves to HR. + - ArrowDown again → focus moves past HR to next block. + +--- + +## P2 — medium priority capabilities + +### P2.1 — Inline formatting in block-select multi-block + +File: `test/browser/tests/block_editing/inline_format_block_select.test.js` (new) + +1. **Cmd+B across a multi-block selection bolds every text node in every + block** + - `

    alpha

    bravo

    `, select both, Cmd+B + - Both paragraphs' text is `` or has `format: 1` internally. + +2. **Cmd+I / Cmd+U / Cmd+Shift+X (strikethrough) across multi-block** + +3. **Cmd+B on a wrapped heading block applies to the heading text** + +4. **Cmd+Shift+H (last-used highlight) across multi-block** + - Pre-condition: a color has been used recently (seed via localStorage or + first apply a color via the menu). + - Then select multi-block and press Cmd+Shift+H → all selected blocks get + the last color. + +### P2.2 — Cmd+A escalation + +File: add to `block_api.test.js` or a new `select_all.test.js` + +1. **First Cmd+A selects current block's text (edit mode)** +2. **Second consecutive Cmd+A enters block-select with all blocks selected** +3. **Cmd+A while already in block-select selects all blocks** + +### P2.3 — Cmd+D duplicates wrapped blocks with children + +1. **Cmd+D on a wrapped `

    ` in a list duplicates the wrapped li** + - Result: two adjacent `
  • ` items. + +2. **Cmd+D on a wrapped li that has a structural-wrapper of children + duplicates the subtree** + - `
    • parent

      • child
    ` + - Cmd+D on the parent → new parent with its own children clone. + +### P2.4 — Color application on multi-block + +File: `test/browser/tests/block_editing/color_block_select.test.js` (new) + +1. **Apply a text color via menu across a multi-block selection** + - All selected blocks' text inherits the color. + +2. **Apply a background color (highlight) across multi-block** + +3. **Color propagates from parent list item to its selected children** — + the `#applyOrRestoreParentHighlight` path. + +4. **Remove color via the Remove-all-coloring button clears all selected** + +5. **Color preserved through Cmd+Shift+↓ movement** + +### P2.5 — Mixed-selection Shift+Tab + +Add one more test to `wrapped_block_outdent.test.js`: + +1. **Shift+Tab on a root list containing plain + wrapped items mixed** + - `
    • plain
    • W

    • plain2
    ` + - Select all three, Shift+Tab + - Expected: each item lands in the right shape. Wrapped items extract to + standalone blocks; plain items collect into the naturally-formed list + segment before/after (per `#extractWrappedItemsInPlace` semantics). + +--- + +## P3 — lower priority / harder to test + +### P3.1 — Scroll preservation + +File: `test/browser/tests/block_editing/scroll_preservation.test.js` (new) + +Pattern: set up a document with ≥40 filler paragraphs above and below the +target block so `scrollY` has room to vary. Scroll to the target, capture +`scrollY`, perform action, assert `scrollY` is within a small delta (±10px +for tolerance). + +Actions to cover: +1. Turn into heading (existing scroll-preserving path) +2. Turn into Bullet wrapping a heading +3. Remove Quote +4. Remove Bullet +5. Cmd+Z after any of the above +6. Cmd+Z after multi-block group move +7. Cmd+Shift+↓ when the moved block stays on-screen (no scroll change) + +### P3.2 — Block actions menu positioning/anchoring + +1. **Menu stays anchored to its block as the page scrolls** — open Cmd+/, + scroll the page by a small amount, assert menu's computed top/left updated + to follow its anchor element. +2. **Menu closes on click-outside** — covered by one existing test; add a + variant with the menu fully open on a submenu. +3. **ArrowDown skips hidden menu items** — Remove Quote hidden, navigation + from Color should jump to Duplicate (not to Remove Quote). Already fixed + in commit c47a28ed but not tested. + +### P3.3 — Keyboard navigation edge cases + +1. **Enter at end of a wrapped heading in block-select mode** — places + cursor at end of heading in edit mode. +2. **Escape from edit mode inside a wrapped table** — re-enters block-select + with the table's wrapper li focused. +3. **Arrow keys escaping a wrapped code block** — existing + `tables_extension.js` registers `COMMAND_PRIORITY_CRITICAL` handlers for + this; one regression test is worth having. + +--- + +## Capybara round-trip tests (AGENTS.md requirement) + +File: `test/system/block_editing_roundtrip_test.rb` (new) + +These tests ensure the Action Text save → render → re-edit cycle preserves +the new wrapped-block shapes. Every change to how Lexxy serializes content +on this branch is load-bearing for these. + +Pattern (see `test/system/code_highlighting_test.rb` for a reference): +```ruby +test "wrapped heading in bullet list survives the full round-trip" do + post = Post.create!(body: %q(
    • Wrapped heading

    )) + + visit edit_post_path(post) + # Assert editor value reflects the wrapped shape + assert_editor_html %q(
    • Wrapped heading

    ) + + # Submit form (no changes) and reload + click_on "Update Post" + visit post_path(post) + # Assert show page renders the wrapped shape via Action Text + assert_selector "ul > li > h2", text: "Wrapped heading" + + # Re-edit — assert the editor still loads it wrapped + visit edit_post_path(post) + assert_editor_html %q(
    • Wrapped heading

    ) +end +``` + +Shapes to cover: + +1. **Wrapped `

    ` in `
      `** +2. **Wrapped `
      ` in `
        `** +3. **Wrapped `
        ` in `
          `** +4. **Wrapped `
          ` in `
            `** +5. **Wrapped attachment in `
              `** — use `posts(:with_attachment)` fixture + or seed via `Post.create!` with a pre-uploaded blob. +6. **Blockquote wrapping a decorator** — `

              ` +7. **Nested structural wrapper with children** — + `
                • child
              ` + — assert the structural-wrapper `
            • ` + survives. +8. **Wrapped table in list** — `
              ` +9. **Movement-wrapped origin does NOT persist** — a heading that was + movement-wrapped and saved should round-trip as user-wrapped (loaded + state has no origin marker, fallback is user-wrapped — see + `#isUserWrapped` comment). Edge-case test; may be skippable if the + invariant holds by construction. + +If any shape fails round-trip, the fix is usually one of: +- `engine.rb` ActionText sanitizer allowlists (tags/attributes) +- `src/config/dom_purify.js` client allowlist +- `app/views/active_storage/blobs/_blob.html.erb` server rendering +- `src/elements/editor.js`'s `#parseHtmlIntoLexicalNodes` for import parity + +--- + +## Suggested execution order + +If doing this in one fresh session: + +1. P1.1 (origin-aware) — 5 tests, ~1 hour. Pure Playwright, no mocking. +2. P1.3 (undo correctness) — 4 tests, ~45 min. Pure Playwright. +3. P1.4 (tables) — 5 tests, ~1 hour. Pure Playwright. +4. P1.5 (HR) — 4 tests, ~45 min. Pure Playwright. +5. P1.2 (attachments) — 6 tests, ~1.5 hours. Needs + `mockActiveStorageUploads` + attachment-specific setup. +6. P2.5 (mixed-selection Shift+Tab) — 1 test, 15 min. +7. P2.1 (inline formatting) — 4 tests, ~45 min. +8. P2.2 (Cmd+A escalation) — 3 tests, ~30 min. +9. P2.3 (Cmd+D on wrapped) — 2 tests, ~30 min. +10. P2.4 (color) — 5 tests, ~1 hour. +11. P3.1 (scroll preservation) — 7 tests, ~1.5 hours. +12. P3.2, P3.3 — 30 min each. +13. Round-trip Capybara suite — 9 tests, ~2 hours. + +Total: roughly a full day of focused work. + +## Success criteria + +After this plan is complete: +- `yarn test:browser` passes with all new tests green. +- `bin/rails test:system` passes with the new round-trip tests. +- Total Playwright block-editing suite goes from 79 → ~130+ tests. +- Any commit touching `block_selection_extension.js`, `block_actions_menu.js`, + `provisional_paragraph_extension.js`, `command_dispatcher.js`, or the + `nodes/wrapped_table_node.js` gets caught by at least one test if it + breaks a documented capability. + +## Out of scope + +- Performance benchmarks (`yarn benchmark:browser`) — tracked separately + per AGENTS.md. +- Visual regression tests (block highlight CSS, group styling) — needs a + snapshot tool not currently set up. +- Drag-and-drop tests beyond what exists — 20 tests already cover this + well; gaps exist but they're lower priority. diff --git a/PLAN-group-block-movement.md b/PLAN-group-block-movement.md new file mode 100644 index 000000000..273c4ce7e --- /dev/null +++ b/PLAN-group-block-movement.md @@ -0,0 +1,595 @@ +# Block Editing: Architecture & Implementation + +> Notion-style block selection, movement, drag-and-drop, and formatting for Lexxy. +> Branch: `block-editing-standalone` — 98 files changed, 17,161 insertions, 387 deletions against `origin/main`. + +## Overview + +This branch adds a complete block editing system: multi-block selection with keyboard navigation, drag-and-drop reordering, block-level formatting (turn-into, highlight colors), and Cmd+Shift+Up/Down movement through arbitrarily nested list structures. The system is implemented primarily as an extension (`BlockSelectionExtension`, coordinator in `src/extensions/block_selection_extension.js`) with supporting modules under `src/editor/block_selection/` and changes to core lexxy infrastructure where required. + +The design goal is Notion-style block semantics: every visible element (paragraph, heading, list item, table, code block, HR) is an individually selectable, movable block. Selected blocks highlight with a subtle fill, can be dragged as a group, and move through nested list hierarchies one step at a time. + +--- + +## File inventory + +### New files (extension layer) + +| File | Lines | Purpose | +| ------------------------------------------------------------------------------------- | ------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `src/extensions/block_selection_extension.js` | 4,525 | Coordinator: selection state, keyboard navigation, block movement, formatting, highlight propagation — delegates drag-and-drop and a few independent concerns to the modules below | +| `src/editor/block_selection/drag_and_drop/index.js` | 1,973 | Drag-and-drop coordinator: drag handles, drop indicators, drag ghosts, hover detection | +| `src/editor/block_selection/drag_and_drop/autoscroll.js` | 158 | `AutoScroll` class (edge-proximity scroll while dragging) | +| `src/editor/block_selection/drag_and_drop/ghost.js` | 103 | `DragGhost` class (translucent clone of the dragged block) | +| `src/editor/block_selection/drag_and_drop/drop_indicator.js` | 57 | `DropIndicator` DOM lifecycle | +| `src/editor/block_selection/drag_and_drop/geometry.js` | 59 | Snap-point / nesting-depth geometry helpers | +| `src/editor/block_selection/wrapped_origin.js` | 172 | `WrappedOriginTracker` class (user-vs-movement wrap origin) | +| `src/editor/block_selection/selection_history.js` | 57 | `SelectionHistory` class (snapshot/restore for undo/redo) | +| `src/editor/block_selection/highlight_css.js` | 47 | `extract/merge/removeHighlightFromCSS` utilities for inline-style manipulation | +| `src/editor/block_selection/bullet_color_sync.js` | 60 | `registerBulletMarkerColorSync` (keep bullet markers colored to match text) | +| `src/elements/block_actions_menu.js` | 671 | Floating context menu: turn-into, highlight colors, delete | +| `src/elements/attachment_controls.js` | 186 | Floating on-hover controls for attachments (open / collapse / edit / caption-toggle / delete); formerly `node_delete_button.js` | +| `src/elements/attachment_icons.js` | 41 | Shared SVG icon strings for attachment floating controls and preview modal chrome | +| `src/elements/preview/dialog_builder.js` | 214 | Shared preview-modal DOM builder used by the editor element and the standalone show-page script | +| `src/elements/preview/playback_sync.js` | 53 | `attachPlaybackSync` + `installPauseOthers` — media playback coordination between inline players and the modal | +| `src/preview/content_preview.js` | 52 | Rollup entry for `lexxy-content-preview.js` — the show-page script host apps import on pages that render ActionText content; thin wrapper around the shared preview modules | +| `src/editor/block_helpers.js` | 38 | Shared CSS-class constants, `$isStructuralWrapper()`, and `getNodeKeyFromElement()` helpers | +| `src/nodes/wrapped_table_node.js` | 78 | TableNode subclass for tables inside list items; provisional escape item tracking | +| `src/editor/markdown/list_heading_shortcut.js` | 102 | Markdown shortcuts (`# `, `## `, `> `) inside list items → wrapped blocks | +| `lib/lexxy/attachment_helper.rb` | 110 | Helpers used by `_blob*.html.erb`: `lexxy_attachment_actions` (preview/download buttons), `lexxy_attachment_preview_caption`, `lexxy_attachment_file_caption`, `lexxy_inline_svg` (reads from `app/assets/images/lexxy/`) | +| `app/views/active_storage/blobs/_blob_{audio,file,image,inline_image,video}.html.erb` | ~30 ea | Per-type partials. `_blob.html.erb` is now a dispatcher that routes to the right partial based on `blob.video?` / `blob.audio?` / content type. Keeps each type's markup small and independently overridable | +| `app/assets/images/lexxy/{preview,download}.svg` | — | SVG assets for the per-attachment action buttons, read via `lexxy_inline_svg` | +| `test/browser/tests/block_editing/*.test.js` | — | 14 Playwright test files covering selection, drag-and-drop, movement, actions menu, wrapped blocks, undo correctness (see Test coverage below) | + +### Modified core files + +| File | Delta | What changed | Could it live in the extension? | +| ------------------------------------------ | ------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `src/editor/command_dispatcher.js` | +187 | Unified `COMMANDS` map (merged the old `BLOCK_FORMAT_COMMANDS`); scroll preservation for every format command plus undo/redo (wraps handlers so `window.scrollY` is restored after Lexical's `scrollIntoViewIfNeeded`); `SELECT_ALL_COMMAND` escalation to block mode; Tab handling for both code blocks and lists. | **No.** These are command routing changes that affect all editors, not just block-select mode. Scroll preservation prevents page jumps during any heading/list/quote/code conversion. | +| `src/editor/contents.js` | +73 | Wrapped block creation: `#applyHeadingFormat`, `#applyCodeBlockFormat`, `#applyQuoteBlockFormat` now wrap content inside list items instead of replacing them. Uses `getListItemNode` from `src/helpers/lexical_helper.js` plus a new `#wrapListItemInBlock` helper. | **No.** Determines how block formats interact with list structure — fundamental data model behavior. | +| `src/elements/editor.js` | +37 | `BlockSelectionExtension` registration, `block-handles` attribute, `selectAllBlocks()` public API, `#applyCodeSettings()`, extension lifecycle init/dispose. | **Partially.** The `block-handles` attribute and `selectAllBlocks()` API must be on the editor element. Extension registration is standard. `#applyCodeSettings()` for `--lexxy-code-tab-size` CSS variable could arguably stay in the extension. | +| `src/extensions/tables_extension.js` | +227 | Arrow-key escape from wrapped tables/code in lists; provisional ListItemNode creation/cleanup; `$handleWrappedBlockEscapeInList()`, `$isAtVisualEdge()`. | **Mostly yes.** Table escape logic is closely tied to the wrapped-block concept introduced by block editing. The escape handlers register at `COMMAND_PRIORITY_CRITICAL` which works fine from an extension. | +| `src/elements/table/table_controller.js` | +30 | Enter key in wrapped tables creates rows (not bailing for wrapper lists); `#isListNestedInCell()` distinguishes wrapper-list from cell-nested-list. | **No.** Enter key behavior inside tables is core table UX. The distinction between "table wrapped in a list item" vs "list nested in a table cell" must be handled in the controller. | +| `src/nodes/early_escape_code_node.js` | +12 | Code block exit creates sibling ListItemNode (not paragraph) when inside a list item. | **No.** Node-level behavior must be in the node class. | +| `src/elements/code_language_picker.js` | +99 | Copy button, hover-based visibility, `#monitorForCodeBlockHover()`. | **Could be.** The copy button and hover monitoring are independent of block selection. These are code block UX improvements that happened alongside this branch but aren't architecturally dependent on it. | +| `src/elements/toolbar.js` | +38 | Clears toolbar pressed states during block-select mode. | **Could be.** The extension could listen for mode changes and clear toolbar state, but it's simpler in toolbar.js since the toolbar already updates on selection changes. | +| `src/elements/toolbar_dropdown.js` | +32 | Deferred initialization via `queueMicrotask`. | **Yes.** This is a lifecycle bug fix for dynamic toolbar creation, not specific to block editing. | +| `src/elements/dropdown/highlight.js` | +9 | Saves last-used color via `saveLastUsedColor()` (from `src/helpers/storage_helper.js`). | **Could be.** Cross-component color tracking, but touching extension UI code. | +| `src/elements/dropdown/link.js` | +4 | Renamed `connectedCallback` → `initialize()`. | **Yes.** Consistency refactor, not block-editing specific. | +| `src/config/lexxy.js` | +6 | Added `markdown: true` to default config. | **Yes.** Enables list-heading shortcuts but is a config default, not structural. | +| `src/helpers/lexical_helper.js` | +6 | Added `getListItemNode()` utility. | **Could be.** Small helper, but useful beyond block editing. | +| `src/extensions/highlight_extension.js` | +27 | Mark padding sync (`data-pad-start`/`data-pad-end` on `` elements). Corresponding CSS rules added to `lexxy-content.css`. | **Yes.** Visual polish for highlights, independent of block editing. | +| `app/assets/stylesheets/lexxy-editor.css` | +2,005 | All block selection visual styling. | **No.** Editor-level CSS must ship with the editor, not be injected by an extension. | +| `app/assets/stylesheets/lexxy-content.css` | +431 | Custom bullet rendering (radial-gradient markers), list margin/padding restructuring, code block spacing, attachment icon sizing. | **Partially.** The list bullet redesign (replacing browser markers with `::before` pseudo-elements) was necessary to enable block selection's left-gutter highlighting. The code block and attachment changes are independent improvements. | + +### Changes that could potentially be kept in the extension + +Based on the analysis above, candidates for moving back to the extension (or splitting into separate PRs): + +1. **`toolbar_dropdown.js` lifecycle fix** — Generic bug fix, not block-editing specific. Could be a separate PR. +2. **`link.js` rename** — Consistency refactor, separate PR. +3. **`config/lexxy.js` markdown default** — Config change for list shortcuts, could ship independently. +4. **`code_language_picker.js` copy button + hover** — Code block UX improvements, independent feature. +5. **`highlight_extension.js` mark padding** — Visual polish, independent feature. CSS rules now added to `lexxy-content.css` (complete on this branch). +6. **`editor.js` `#applyCodeSettings()`** — The CSS variable for code tab-size could be set by the extension's `initializeEditor()` hook instead. + +--- + +## Architecture + +### Extension subsystems + +The `BlockSelectionExtension` (4,525 lines in `src/extensions/block_selection_extension.js`) has 13 interconnected subsystems. Independent concerns (drag-and-drop, wrapped-origin tracking, selection history, highlight CSS parsing, bullet color sync) live as sibling modules under `src/editor/block_selection/` — see the inventory above. + +#### 1. Mode management + +Dual-mode system: `"edit"` (normal text editing) and `"block-select"` (block-level operations). Escape toggles between them. Entering block-select adds `block-selection-active` to the editor root (hides caret, disables text selection via CSS). Exiting removes it and commits any pending highlight color changes. + +#### 2. Selection state + +- `#selectedBlockKeys` (Set) — currently selected node keys +- `#anchorKey` — first block selected (range anchor for Shift+Arrow) +- `#focusKey` — last block in selection (current focus) +- `#selectBlock(key, extend)` — select/toggle a single block +- `#selectRange(from, to)` — select contiguous range +- `#getNavigableBlockKeys()` — all top-level selectable blocks (excludes ListNode containers) +- Selection history maintained in parallel undo/redo stacks, synced with Lexical's history commands + +#### 3. DOM synchronization + +`#syncSelectionClasses()` diffs `#selectedBlockKeys` against `#previousSelectedKeys` and applies/removes `block--selected` and `block--focused` CSS classes. Diff-based for performance. + +`#syncSelectionGroupClasses()` identifies contiguous runs of selected items and applies `block--select-first`, `block--select-mid`, `block--select-last` for flattened-edge group styling. + +`#syncLeafInsets()` writes per-block `--leaf-top-inset` / `--leaf-bottom-inset` CSS variables computed by `#findAdjacentBlocks()` and `#computeLeafReach()`. Each selected leaf's `::after` reaches halfway to its nearest visible neighbor (skipping `.hidden` provisional separator paragraphs). The target visible gap is resolved per-pair by `#targetGapBetween()`: 2px for plain-text LIs sharing the same parent list (tight Notion rhythm — Tango↔Uniform at the outer OL), 4px otherwise (mixed-list rhythm — Papa↔Quebec inside a wrapper's inner ul). Cross-wrapper pairs default to 4px but switch to 2px when the wrapper's outer-level owner is itself in a parent-takeover state, so the unified parent-takeover highlight lands 2px above the next plain-text sibling. Both sides of any adjacent selected pair run through the same classifier, guaranteeing the two reaches agree on the target. + +`#syncParentSelectionHeight()` sets `--parent-selection-height` on parent items so the parent's `::after` extends to cover all nested children as a unified highlight. The bottom uses the last child's computed `bottomReach` so the parent-takeover highlight ends exactly where that last child's individual highlight would have — heights stay stable as the selection grows from leaf → group → parent. + +#### 4. Keyboard handling + +Global `keydown` listener active only in block-select mode. Keybindings: + +| Key | Action | +| ------------------ | ------------------------------------------ | +| Arrow Up/Down | Navigate selection (Shift to extend range) | +| Cmd+Shift+Up/Down | Move selected blocks | +| Enter | Enter edit mode on focused block | +| Escape | Exit block-select → blur editor | +| Tab / Shift+Tab | Indent / outdent | +| Backspace / Delete | Remove selected blocks | +| Cmd+A | Select all blocks | +| Cmd+D | Duplicate selected blocks | +| Cmd+B/I/U | Bold / italic / underline across selection | +| Cmd+Shift+X | Strikethrough | +| Cmd+Shift+H | Apply last-used highlight color | +| Cmd+/ | Open block actions menu | + +#### 5. Block movement (single item) + +`#moveSingleBlock` → `#moveListItem` → dispatches to: + +- `#nestListItemUnderSibling` — nest under adjacent sibling (depth-first traversal) +- `#promoteListItem` — promote one nesting level +- `#promoteWrappedBlockThroughRoot` — wrapped blocks skip root list level and exit as standalone elements + +#### 6. Block movement (atomic groups) + +`#moveGroupAtomically` handles multi-block moves. Flow: + +``` +Cmd+Shift+Up/Down with group: + ├─ Different parents? + │ ├─ Any outside list or in root list → #moveRootLevelGroup (swap/enter) + │ └─ Different depths in same hierarchy → #normalizeGroupDepth + ├─ Has sibling in direction → #nestGroupUnderSibling + └─ No sibling (boundary): + ├─ Group IS entire root-level list → swap list as unit / enter adjacent list + ├─ Root-level list → #exitGroupFromList + └─ Nested list → #promoteGroupOneLevel +``` + +Key concepts: + +- **Root keys**: `#filterToRootKeys` identifies the top-level items in the selection; children travel with their root via structural wrappers +- **Structural wrappers**: ListItemNodes containing only nested ListNodes — they carry child content when a parent item moves +- **Cursor approach**: `#exitGroupFromList` creates a temporary ParagraphNode as a stable reference point, places extracted items relative to it, then removes it (or keeps it as a separator to prevent Lexical's adjacent-list merge). When a retained separator is itself a decorator paragraph, subsequent group moves skip over it when computing the target index. +- **Batch exit**: consecutive regular items are collected into a single standalone list to prevent merge-induced infinite loops + +#### 7. Drag-and-drop + +`BlockDragAndDrop` (`src/editor/block_selection/drag_and_drop/index.js`, 1,973 lines plus 4 sibling modules totaling ~380 more lines) manages: + +- Drag handle element with 6-dot grip icon, positioned on hover +- Add-block button (plus icon) +- Drop indicator: depth-aware positioning, gap resolution, center-aligned on the target edge; OL drop targets render `#.` rather than a bullet circle +- Drag ghost (translucent clone of dragged block, width matches source) +- Auto-scroll when dragging near container edges +- Multi-block drag (Shift+click, Cmd+click for range/toggle selection) +- Post-drop cleanup: removes empty ListItemNodes left behind by Lexical's normalization after an unwrap + +#### 8. Block actions menu + +`BlockActionsMenu` (`src/elements/block_actions_menu.js`, 658 lines) — floating context menu opened via Cmd+/ or right-click: + +- **Turn into**: paragraph, H2, H3, H4, bullets, numbers, quote, code +- **Color**: 9 highlight colors × 2 styles (text color, background color), plus last-used quick access +- **Delete**: remove selected blocks +- Full keyboard navigation (arrow keys, Enter, Escape) + +#### 9. Highlight color management + +Sophisticated color propagation system for nested list highlights: + +- `#savedHighlightStyles` preserves original colors before parent propagation +- When a parent item gets a highlight, children inherit it +- When entering edit mode, new blocks don't inherit parent highlight +- `#applyOrRestoreParentHighlight` resolves whether to keep inherited color or revert to saved original +- CSS string parsing/merging handles complex `style=""` attribute manipulation + +#### 10. Block type conversion + +`#convertBlockType(command)` converts selected blocks between types. When blocks are inside list items, uses `#wrapListItemContent` to create wrapped blocks (heading-in-list, quote-in-list) rather than replacing the list item. + +#### 11. Indent / outdent + +Tab/Shift+Tab in block-select mode calls `#handleIndentOutdent`. For wrapped blocks (headings/quotes/code inside list items), uses special `#indentWrappedBlock` / `#outdentWrappedBlock` that manipulate the structural wrapper nesting. When a root-level item can't outdent further, `#flattenChildrenOneLevel` promotes children to siblings. + +Shift+Tab on mixed root-level lists (wrapped blocks + regular bullets) extracts each wrapped item in place by splitting the list around it. Regular bullets stay in the naturally-formed list segments, preserving interleaved document order. `#exitGroupFromList` is still used for Cmd+Shift+Up boundary exit, but no longer for root-level outdent. + +#### 12. Wrapped block escape + +Tables and code blocks inside list items need special arrow-key escape. Registered in `tables_extension.js` at `COMMAND_PRIORITY_CRITICAL`: + +- Arrow Up at top of table/code → creates provisional sibling ListItemNode above +- Arrow Down at bottom → creates provisional sibling below +- Provisional items auto-remove on selection change if left empty +- Backspace in a provisional returns focus to the adjacent table/code + +#### 13. Bootstrap deferral + +Every block-editing feature that doesn't affect initial render is deferred until the user actually touches the editor. On construction the extension wires a pair of one-shot listeners — `mouseenter` on the editor element and `focusin` on the root — and only on the first fire does it register its `registerCommand`s, keyboard handlers, and instantiate `BlockDragAndDrop`. The only work that stays in the bootstrap path is the bullet-marker node transform, because it needs to run during initial content reconciliation to style pre-existing lists. + +Impact: removes 14 `registerCommand` calls, 1 `registerNodeTransform`, and several `addEventListener` calls per editor from the bootstrap path. This is load-bearing for the `bootstrap-many-editors` benchmark and why the per-editor cost stays close to the pre-branch baseline. + +### Lexical list structure + +``` +ListNode (ul/ol) + ListItemNode "Parent text" ← content item (selectable block) + ListItemNode [structural wrapper] ← has class "lexxy-nested-listitem" + ListNode (ul/ol) + ListItemNode "Child text" ← content item + ListItemNode [heading wrapper] ← wrapped block (contains H2) +``` + +- `$isStructuralWrapper(node)` — ListItemNode whose only children are ListNodes +- **Wrapped blocks** — ListItemNodes containing non-text block content (h1-h6, blockquote, code, table, HR, attachments). Detected by `#isWrappedBlock` via content heuristic, tracked key set, and DOM attribute fallback. +- When selecting a parent, its structural wrapper's children are automatically included in the selection + +### CSS architecture + +Selection highlighting uses `::after` pseudo-elements at `z-index: -1` as the primary mechanism, with per-block-type overrides: + +| Block type | Technique | Why | +| ----------- | ------------------------------------------------ | ------------------------------------------------------------------------------ | +| Most blocks | `::after` pseudo-element | Standard approach, paints behind content | +| Code blocks | `z-index: auto` + outline + box-shadow tint | Code bg is opaque; breaking the stacking context lets `::after` paint below it | +| Tables | Direct background on wrapper + cells | Cell backgrounds are opaque; `::after` would be hidden | +| Blockquotes | `::before` bar at z-index: 1 over `::after` fill | Vertical bar must render above the highlight fill | +| HR | Compact `::after` with `min-height: unset` | Thin element needs minimal highlight | + +**Contiguous group styling**: Adjacent selected items get `block--select-first/mid/last` classes for flattened-edge visual bands. Mixed lists (containing wrapped blocks) skip mid-styling because 4px gaps are too wide for flat-edge merging. + +**Parent fill**: When a parent item and its children are all selected, the parent's `::after` extends to cover the entire subtree via `--parent-selection-height` CSS variable. Children's individual `::after` elements are hidden to avoid double-painting. + +**Layout-shift-free selection**: Selection CSS must never toggle flow properties (margin, padding, height). Only `::after` geometry and background/color respond to `.selected`. Wrapper list items (`lexxy-nested-listitem`) establish a block formatting context via `display: flow-root` so nested wrapped-block margins stay trapped inside the wrapper rather than collapsing up through empty ancestors and inflating the outer list. + +**Halfway-reach invariant for isolated leaves**: Every selected `li` / `figure.attachment` / `.attachment-gallery` gets `--leaf-top-inset` and `--leaf-bottom-inset` pre-computed as halfway-to-neighbor. The target visible gap is pair-classified: 2px for plain-text LIs in the same parent list (tight rhythm), 4px for mixed-list and cross-wrapper pairs, except a cross-wrapper pair whose outer-level owner is itself parent-taken-over is re-classified as owner↔outer-sibling and pulls tight to 2px. Solo selections reach to the midpoint on each side. `.hidden` provisional paragraphs (Lexical's decorator separators) are skipped during adjacency walks so inter-decorator pairs reach each other directly instead of landing on invisible separators. + +**List bullet redesign**: Browser default markers were replaced with `::before` pseudo-elements using radial-gradient bullets and CSS counter numbers. This was necessary because block selection's left-gutter highlight extends beyond the bullet position, and browser markers can't be styled to integrate with the highlight fill. + +**Code block hover controls**: The copy button and language picker are hidden while block-select mode is active or a drag is in progress, so they don't paint on top of the block highlight or interfere with the drop target. + +### Lexical reconciler caveats + +- **Adjacent-list merge**: Lexical silently merges adjacent `ListNode`s of the same type during DOM reconciliation. Any operation that places two same-type lists next to each other will have them merged. Prevent by: (a) batching items into a single list, (b) keeping a ParagraphNode separator between lists. +- **Copy-on-write keys**: Lexical may change node keys during `editor.update()`. `#resyncWrappedKeys` handles some cases but stale keys can persist after group operations that create/destroy nodes. +- **CSS `:has()` nesting**: Browsers silently ignore nested `:has()` selectors. `ul:has(> li:has(> h2))` is dropped — must flatten to `ul:has(> li > h2)`. +- **Provisional paragraphs between decorators**: Lexical inserts empty `` modal with header (icon, caption, filename, file size) + content area +- Content-type detection: image zoom, video player, audio player, PDF iframe, generic download fallback +- Two-line header: shows custom caption as title with filename + size as subtitle (or filename as title with size below) +- Playback time sync: opening modal from an inline player carries over `currentTime`; closing syncs it back. If the page media was playing, modal continues from the same spot. +- Pause-others: playing any media element pauses all other video/audio on the page (`installPauseOthers()` from `playback_sync.js`) +- Turbo-compatible via `turbo:load` listener + +**Host app integration** — minimal: + +```ruby +# config/importmap.rb +pin "lexxy", to: "lexxy.js" +pin "lexxy-content-preview", to: "lexxy-content-preview.js" # only for show-page modal +``` + +```javascript +// app/javascript/application.js +import "lexxy"; +import "lexxy-content-preview"; // optional; opts into show-page modal & preview button +``` + +The `previewModal: true` flag for the editor modal is now the default — no `configure()` call needed. Apps that want to opt out can set `previewModal: false`. + +If the app has a custom `app/views/active_storage/blobs/_blob.html.erb`, delete it (and delete any per-type partials) to use Lexxy's. The Lexxy gem ships: + +- `lib/lexxy/attachment_helper.rb` (`lexxy_attachment_actions` — preview/download button row, `lexxy_attachment_preview_caption`, `lexxy_attachment_file_caption`, `lexxy_inline_svg` — reads from `app/assets/images/lexxy/`) + +The helper module is auto-included into `ActionView::Base` by `Lexxy::Engine`, so the partials just call its methods. Icon labels are just `extension.upcase` inline in the partials — no separate Ruby icon-label map. + +### Attachment state persistence + +Two new data attributes on `action-text-attachment` elements persist editor state to the rendered page: + +| Attribute | Effect in editor | Effect on show page | +| --------------------- | ----------------------------------------------------------------------- | ------------------------------------------------------------- | +| `data-collapsed` | Hides preview, shows file card | Hides media, renders as file card with icon + filename + size | +| `data-caption-hidden` | Hides caption on images; shows filename instead of custom name on files | Same behavior via CSS + JS | + +**Pipeline for persistence:** + +1. Editor → Lexical node properties (`collapsed`, `captionHidden`) +2. `exportDOM()` → data attributes on `action-text-attachment` element +3. DOMPurify client-side allowlist (so attributes survive value serialization) +4. `ActionText::Attachment::ATTRIBUTES` (so attributes survive server-side re-serialization) +5. `ActionText::ContentHelper.allowed_attributes` (so attributes survive render-time sanitization) +6. CSS attribute selectors + JS on the show page apply the visual state + +### Editor attachment controls (``) + +Custom element at `src/elements/attachment_controls.js` (formerly `node_delete_button.js` — renamed since it grew well beyond the delete button). Floating controls appear on hover and selection. All buttons have native browser tooltips via `title` plus matching `aria-label`. Button order: + +| Button | Preview attachments | File attachments | Tooltip | +| --------------------------- | --------------------------------------------------------- | ------------------------------------------------ | ------------------------------------- | +| Open (eye) | Opens modal | Opens modal | "Open" | +| Collapse (chevron) | Toggles preview ↔ card | — (already a card) | "Collapse preview" / "Expand preview" | +| Edit (pencil) | Focuses caption editor (textarea or click-to-rename name) | Click-to-edit inline filename | "Edit name" | +| Caption toggle (text lines) | Show/hide caption below media | Toggle between custom name and original filename | "Hide caption" / "Show caption" | +| Delete (trash) | Remove attachment | Remove attachment | "Remove" | + +**Naming note**: We use **"Open"** for the eye icon to disambiguate from the inline "preview" (which the collapse button toggles). Internally the code/event names still use "preview" (`#openPreview`, `lexxy:preview-attachment`, `PreviewModal`, etc.) — only user-visible strings changed. + +**Audio preview in editor**: Audio attachments render as a file card with an inline `