Skip to content

fix(editor): test and support Tiptap 3.32 - #4401

Open
emdash-autofix[bot] wants to merge 6 commits into
mainfrom
emdash-bot/issue-4400
Open

emdash-autofix[bot] wants to merge 6 commits into
mainfrom
emdash-bot/issue-4400

Conversation

@emdash-autofix

@emdash-autofix emdash-autofix Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?

Closes #4400.

Pins the Tiptap 3.x editor catalog entries to exactly 3.32.0, keeping extension-code-block-lowlight at 3.20.0. Published admin/core packages will use that tested version rather than a floating caret range; integrations sharing Tiptap dependencies should align to 3.32.0 to avoid duplicate copies. Updates y-tiptap to ^3.0.7 (locked to 3.0.10) to satisfy the collaboration/drag-handle peer requirement. The lockfile consolidates ProseMirror without unrelated dependency upgrades.

  • Restores EmDash's responsive table view instead of Tiptap's newly registered non-resizable view, including read-only tables.
  • Moves syntax-highlighting DOM assertions to a React-mounted editor, checks EmDash-owned select-all image decorations, and scopes link buttons to the permanent toolbar to avoid ambiguous selectors.
  • Accepts upstream's intentional whitespace link exit: a link started at an empty caret ends at the first typed space. In the visual editor, typing a space at the end of an existing link also ends it. This is covered by the updated test and called out in the changeset, alongside the exact-pin decision.

Type of change

  • Bug fix
  • Feature (requires an accepted feature plan or RFC)
  • Refactor (no behavior change)
  • Translation
  • Documentation
  • Performance improvement
  • Tests
  • Chore (dependencies, CI, tooling)

Your connection to this change

Prepared by the project autofix bot through auto-intake for this reproduced report. Human maintainer approval is required before merge.

  • I use EmDash and ran into the problem this PR addresses
  • I maintain a tool, dependency, or service that EmDash uses, and this change concerns that integration
  • A Maintainer approved this specific change before I opened the PR

Checklist

  • I have read CONTRIBUTING.md
  • pnpm typecheck passes
  • pnpm lint passes — quick lint passes with zero diagnostics; full type-aware lint deferred to CI
  • pnpm test passes (targeted tests for this change)
  • pnpm format has been run
  • I have added/updated tests for my changes
  • User-visible strings are wrapped for translation — no new strings or catalog changes
  • I have added and reviewed the user-facing changeset
  • New features link to their merged design PR — not applicable
  • I have included screenshots below

AI assistance disclosure

  • This PR includes AI-generated code or text — model/tool: cloudflare-ai-gateway:gpt-6.1-sol via the EmDash bot (Pi Durable)

Screenshots / test output

Passed on the final source:

  • Root pnpm build with NODE_OPTIONS=--max-old-space-size=8192.
  • Workspace pnpm typecheck, pnpm lint:quick (zero diagnostics), pnpm format, and frozen-lockfile install.
  • pnpm --filter @emdash-cms/admin exec vitest run tests/editor --maxWorkers=1 (978 tests across 49 files).
  • pnpm --filter emdash exec vitest run tests/unit/components tests/unit/converters/custom-block-round-trip.test.ts --maxWorkers=1 (includes core menu, image-toolbar, code-block, table and conversion coverage).

A parallel editor run hit an intersection-observer timing assertion; the complete serial run passed. Full type-aware lint and remaining suites are left to CI.

Table reproduction harness on 3.32, same container and saved 128px/160px column widths:

Before After
Table shrinks to 288px Table fills its container
Arabic RTL table
Before After
Arabic table before Arabic table fills its container after

The EmDash bot took this issue on automatically when it was opened; no maintainer has assigned it yet.

Pin the editor catalog to Tiptap 3.32.0, retain lowlight 3.20.0, satisfy the y-tiptap peer, and consolidate ProseMirror. Restore responsive table rendering and fix newer-Tiptap floating toolbar DOM/event compatibility in admin and core. Update highlighting, selection, link, table and keyboard regression coverage. Verified root build, workspace typecheck, zero-diagnostic quick lint, format, frozen install, the full serial admin editor suite and core component/conversion tests. Added changeset and before/after Chromium screenshots; independent follow-up review cleared all findings.

EmDash-Bot-Case: 1198246700-issue-6025800409
EmDash-Bot-Candidate: cand-1-122fcc0877
@emdash-autofix emdash-autofix Bot added the bot:fix Assign this issue or pull request to the EmDash bot to fix label Oct 25, 2026
@emdash-autofix

emdash-autofix Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor Author

EmDash bot: Ready for human review

Plan

Completed the maintainer's wording-only requests: rewrote the PR description to match the remaining diff, added visual-editor existing-link space behavior to the changeset, and corrected the helper comment. No implementation or tests chang…

  • Inspect editor integrations and upgrade Tiptap to reproduce the failures
  • Fix compatibility issues and update regression coverage
  • Run editor checks, capture rendering evidence, and review the change
  • Address review: inline editor keyboard compatibility, collaboration peer, and release notes
  • Fix remounted menu anchoring with browser coverage and finish review documentation
  • Name the editable textbox and verify table accessibility checks
  • Classify aggregate Tests failure and reproduce scheduling geometry check
  • Keep the editorProps explanation directly above its memo

Status

  • Required checks passed, the review bot approved the current head, and the head merges cleanly.

Next action: A maintainer will review and approve the pull request.

Details
Candidate cand-6-129d976203 (tree d5f2f26bab84)
Published head 3304ab96e8af
Reviewer's latest review of head approved
Reporter confirmation not yet
Human approval required before merge

Required checks on the current head:

  • Typecheck: succeeded
  • Tests: succeeded
  • Smoke Tests: succeeded
  • Browser Tests: succeeded
  • Format: succeeded
  • CLAssistant: succeeded
  • Measure: succeeded

This status is generated from recorded evidence. It is not a review and contains no instructions.

@changeset-bot

changeset-bot Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 3304ab9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 12 packages
Name Type
@emdash-cms/admin Patch
emdash Patch
@emdash-cms/cloudflare Patch
@emdash-cms/plugin-test Patch
@emdash-cms/sandbox-workerd Patch
@emdash-cms/auth Patch
@emdash-cms/blocks Patch
create-emdash Patch
@emdash-cms/gutenberg-to-portable-text Patch
@emdash-cms/x402 Patch
@emdash-cms/auth-atproto Patch
@emdash-cms/plugin-embeds Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

🚀 Deploying Preview to Cloudflare 🚀

Preview URL: https://emdash-bot-issue-4400.try.emdashcms.com, https://emdash-bot-issue-4400-emdash-playground.emdash-cms.workers.dev (commit 3304ab9)

This URL reflects your latest Preview deployment

Preview Deployments by commit

Status Deployment URL Commit Updated (UTC) See this deployment's details
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://eb64094a.try.emdashcms.com, https://eb64094a-emdash-playground.emdash-cms.workers.dev 3304ab9 2026-10-27T18:42:08.828Z Visit the dashboard ↗
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://578b67c0.try.emdashcms.com, https://578b67c0-emdash-playground.emdash-cms.workers.dev c6d7985 2026-10-27T01:52:14.216Z Visit the dashboard ↗
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://1e05ceca.try.emdashcms.com, https://1e05ceca-emdash-playground.emdash-cms.workers.dev b61976d 2026-10-25T18:06:29.162Z Visit the dashboard ↗
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://551b0f62.try.emdashcms.com, https://551b0f62-emdash-playground.emdash-cms.workers.dev f5a0b04 2026-10-25T18:03:46.150Z Visit the dashboard ↗
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://9fe505a7.try.emdashcms.com, https://9fe505a7-emdash-playground.emdash-cms.workers.dev 8da5483 2026-10-25T17:30:53.054Z Visit the dashboard ↗
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://7fe7d26f.try.emdashcms.com, https://7fe7d26f-emdash-playground.emdash-cms.workers.dev 1ce5e56 2026-10-25T17:13:24.231Z Visit the dashboard ↗

@emdashbot emdashbot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The approach is sound. Moving the catalog to the Tiptap version users actually install (3.32.0) and fixing each failure from #4400 addresses the CI/install mismatch. Keeping EmDash's own table view, running the highlighting tests in a React-mounted editor, and making the toolbars wrapper-free all fit the existing architecture. One reviewer found a remaining wrapper-dependent positioning path in the core inline editor that needs fixing. There are also two convention and documentation gaps, and two cleanup suggestions. Two decisions are left to a maintainer: the link-authoring behavior change, which the other reviewer accepted as a Tiptap change, and the overlap with #4396, which touches the same toolbar code. Both reviews were static, so nothing was built or run.


Reviewed by anthropic/claude-opus-5-5 and openai/gpt-6.1-sol.

}) => {
elements.floating.style.maxWidth = `${Math.max(0, availableWidth)}px`;
},
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[needs fixing] The wrapper-free migration also needs to update ToolbarMenu in inline-editor-ui.tsx:392–405. That effect still gets its positioning reference with closest('[role="toolbar"]')?.parentElement. With the new DOM structure that parent is .emdash-inline-ui, not the positioned toolbar. This is reachable when closing the link field: the formatting controls remount while the toolbar is already attached, so both Turn into and More formatting capture the UI root's rectangle and open away from their toolbar. Use the toolbar element itself as the position reference, and add browser geometry coverage for opening these menus after leaving the link field. The existing jsdom tests cannot detect this regression. (Only one reviewer raised this; the other reported no bugs but did not examine this path in detail.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Confirmed in Chromium: after closing the link field, the menu was about 106px away from its expected vertical anchor. ToolbarMenu now references the toolbar itself. Opening a menu also finishes the toolbar’s entrance animation so the anchor stays stable. Added browser geometry coverage for both remounted menus; it fails before the fix and passes afterward, along with the existing entrance-positioning test.

Comment thread .changeset/tall-hats-tease.md Outdated

Pins the editor's Tiptap dependencies to exactly 3.32.0 so installations use the version covered by the editor tests. Code-block highlighting keeps its existing lowlight extension version. Sites or plugins that share Tiptap dependencies with EmDash should align them to 3.32.0 to avoid installing multiple copies.

Links applied at an empty caret now end when you type a space, following Tiptap's updated behavior. Text typed after that space is not part of the link.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[needs fixing] The changed link-authoring behavior is explained only in the release note. .changeset/README.md requires useful upgrade explanations to appear in the canonical feature documentation too, but docs/src/content/docs/guides/working-with-content.mdx still gives no guidance on this. Add it under Write rich text, including the practical way to create a multi-word link: type the label first, select the whole label, then apply the link. Editors can then get the previous outcome after upgrading. Note that the other reviewer judged the changeset acceptable as written and treated the behavior change as an accepted Tiptap change. It does weaken the "add a link at an empty caret, then type" flow, because only the first word is now linked.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added guidance under Write rich text explaining the space boundary and the multi-word workflow: type the label, select it, then apply the link. The docs build passes.

import { useEffect } from "react";
import { describe, expect, it, vi } from "vitest";

import { CodeBlockExtension } from "../../src/components/editor/CodeBlockNode";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[needs fixing] This new internal import omits the .js extension required by the root AGENTS.md ESM import convention.

Suggested change
import { CodeBlockExtension } from "../../src/components/editor/CodeBlockNode";
import { CodeBlockExtension } from "../../src/components/editor/CodeBlockNode.js";

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed the new import to use CodeBlockNode.js.

Comment on lines 4930 to 4936
elements.floating.style.maxWidth = `${Math.max(0, availableWidth)}px`;
// The surface scrolls itself, so no wrapper clips its rounded ring.
const surface = elements.floating.firstElementChild;
const surface = elements.floating;
if (surface instanceof HTMLElement) {
surface.style.maxWidth = "100%";
surface.style.overflowX = "auto";
}
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[suggestion] elements.floating is always an HTMLElement, so the surface alias and the instanceof guard don't check anything any more. The same leftover pattern appears in playBubbleEntrance (const surface = element;, line 4809) and in ImageBubbleMenu.playEntrance (const toolbar = menu;, line 5506), where the instanceof check sits after menu?.isConnected. Simplifying them makes it clear that the floating element itself is now the surface:

Suggested change
elements.floating.style.maxWidth = `${Math.max(0, availableWidth)}px`;
// The surface scrolls itself, so no wrapper clips its rounded ring.
const surface = elements.floating.firstElementChild;
const surface = elements.floating;
if (surface instanceof HTMLElement) {
surface.style.maxWidth = "100%";
surface.style.overflowX = "auto";
}
},
elements.floating.style.maxWidth = `${Math.max(0, availableWidth)}px`;
// The surface scrolls itself, so no wrapper clips its rounded ring.
elements.floating.style.overflowX = "auto";
},

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Simplified the sizing and animation paths to operate directly on the floating surface, removing the leftover aliases and redundant guards.

Comment on lines +583 to +590
if (event.nativeEvent.isComposing || event.nativeEvent.defaultPrevented) return;
const toolbar = event.currentTarget;
const controls = [...toolbar.querySelectorAll(TOOLBAR_CONTROL_SELECTOR)];
const backToEditor =
event.key === "Escape" ||
(event.key === "Tab" && event.shiftKey && document.activeElement === controls[0]);
event.nativeEvent.key === "Escape" ||
(event.nativeEvent.key === "Tab" &&
event.nativeEvent.shiftKey &&
document.activeElement === controls[0]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[suggestion] These handlers now read key, shiftKey and defaultPrevented from event.nativeEvent, but they still call event.preventDefault() on the React event. Nested inputs, such as the link field at inline-bubble-menu.tsx:415 and the alt field at inline-image.tsx:362, keep using event.key. Nothing in the code explains why the toolbar handlers differ, so a later cleanup is likely to switch them back to event.key and bring back the 3.32 failure. Add a one-line comment at the start of handleToolbarKeyDown and moveToolbarFocus (admin, line 6626) explaining why the key has to come from nativeEvent when the handler is attached to Tiptap's BubbleMenu.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added a short comment to both shared toolbar keyboard handlers explaining that BubbleMenu’s event wrapper does not forward native keyboard property getters.

@pkg-pr-new

pkg-pr-new Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

@emdash-cms/admin

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/admin@4401

@emdash-cms/auth

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/auth@4401

@emdash-cms/auth-atproto

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/auth-atproto@4401

@emdash-cms/blocks

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/blocks@4401

@emdash-cms/cloudflare

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/cloudflare@4401

@emdash-cms/contentful-to-portable-text

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/contentful-to-portable-text@4401

emdash

npm i https://pkg.pr.new/emdash-cms/emdash@4401

create-emdash

npm i https://pkg.pr.new/emdash-cms/emdash/create-emdash@4401

@emdash-cms/gutenberg-to-portable-text

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/gutenberg-to-portable-text@4401

@emdash-cms/plugin-cli

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-cli@4401

@emdash-cms/plugin-test

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-test@4401

@emdash-cms/plugin-types

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-types@4401

@emdash-cms/registry-client

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/registry-client@4401

@emdash-cms/registry-lexicons

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/registry-lexicons@4401

@emdash-cms/registry-loader

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/registry-loader@4401

@emdash-cms/registry-moderation

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/registry-moderation@4401

@emdash-cms/registry-verification

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/registry-verification@4401

upgrade-emdash

npm i https://pkg.pr.new/emdash-cms/emdash/upgrade-emdash@4401

@emdash-cms/sandbox-workerd

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/sandbox-workerd@4401

@emdash-cms/x402

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/x402@4401

@emdash-cms/plugin-ai-moderation

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-ai-moderation@4401

@emdash-cms/plugin-ai-search

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-ai-search@4401

@emdash-cms/plugin-atproto

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-atproto@4401

@emdash-cms/plugin-audit-log

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-audit-log@4401

@emdash-cms/plugin-color

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-color@4401

@emdash-cms/plugin-embeds

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-embeds@4401

@emdash-cms/plugin-field-kit

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-field-kit@4401

@emdash-cms/plugin-forms

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-forms@4401

@emdash-cms/plugin-webhook-notifier

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-webhook-notifier@4401

commit: 3304ab9

Address review by anchoring remounted inline menus to the actual toolbar and finishing its entrance before positioning. Added Chromium regression coverage that fails before the fix and passes after, retaining existing entrance geometry coverage. Documented multi-word link authoring, corrected the ESM import, simplified floating-surface aliases, and explained native keyboard-event property access. Verified root/docs builds, workspace typecheck, zero-diagnostic quick lint, formatting, 88 focused admin tests, eight core menu tests and two Playwright geometry tests. Added before/after screenshots and replied to all review threads; independent review found no significant issues.

EmDash-Bot-Case: 1198246700-issue-6025800409
EmDash-Bot-Candidate: cand-2-7a8f737a3c
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
docs c6d7985 Oct 27 2026, 01:51 AM

@emdashbot emdashbot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The approach is sound. The catalog now uses the Tiptap version users actually install (3.32.0), which fixes the CI/install mismatch behind #4400. EmDash's own table view, the React-mounted highlighting tests and the wrapper-free toolbars all fit the existing architecture. Both reviewers approved with no new findings, and all five earlier findings are resolved at this head. The core ToolbarMenu now anchors to the toolbar element itself, and new browser geometry coverage exercises it. The link-authoring docs, the .js import extension, the leftover aliases and the nativeEvent comments are also addressed. Two decisions remain for a maintainer: whether to accept the Tiptap link-exit behavior, and how to reconcile this PR with #4396, which touches the same toolbar code. Both reviews were static, so nothing was built or run.


Reviewed by anthropic/claude-opus-5-5 and openai/gpt-6.1-sol.

Fix the deterministic Table E2E accessibility failure introduced by Tiptap 3.32's textbox role: name the editable elements in admin and inline editors, forward admin field labels, and supply a localized admin fallback. Add admin accessible-name regressions and an inline browser assertion; update the changeset. Verified root build, workspace typecheck, quick lint, format, 129 admin component tests, all Chromium Table E2E tests (11 passed, one skipped), and final-source admin naming/inline browser checks. Firefox's CI console warning passed on retry and remains classified as flaky; its browser executable is unavailable locally. Independent follow-up review cleared the changes.

EmDash-Bot-Case: 1198246700-issue-6025800409
EmDash-Bot-Candidate: cand-3-06923df5cd

@emdashbot emdashbot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The approach is sound. Moving the catalog and lockfile to Tiptap 3.32.0 means CI tests the version a fresh install gets, which fixes the mismatch behind #4400. The table-view restoration, the wrapper-free toolbars and the React-mounted highlighting tests fit the existing architecture. Both reviewers approved. All five findings from the earlier review remain resolved at this head: toolbar menu anchoring with geometry coverage, the link-authoring docs, the .js import, the leftover aliases and the nativeEvent comments. The new accessible textbox names are covered by tests. One minor suggestion remains, about comment placement. Two decisions are still for a maintainer: whether to accept Tiptap's new behavior where a link ends at the first typed space, and how to reconcile this PR with #4396, which touches the same toolbar code. Both reviews were static, so nothing was built or run.


Reviewed by anthropic/claude-opus-5-5 and openai/gpt-6.1-sol.

// Stable editorProps reference — a new object every render would cause
// compareOptions to call setOptions → updateState → plugin teardown →
// Suggestion onExit → setSlashMenuState → re-render → infinite loop.
const editorLabel = t`Rich text editor`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[suggestion] The new editorLabel declaration sits between the "Stable editorProps reference…" comment and the useMemo that comment describes, so the comment now looks like it explains editorLabel. Move the declaration above the comment, so the comment stays directly above const editorProps = React.useMemo(.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Moved editorLabel above the comment so the explanation is directly above the editorProps memo. No runtime code changed.

Address the final review nit by moving the editorProps comment below the editorLabel declaration, directly beside the memo it describes. This is comment placement only; executable code and behavior are unchanged. Ran pnpm format and git diff --check. Independent review skipped for this trivial comment-only relocation.

EmDash-Bot-Case: 1198246700-issue-6025800409
EmDash-Bot-Candidate: cand-4-bd109fedb5

@emdashbot emdashbot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The approach is sound. Pinning Tiptap 3.32.0 in the catalog and lockfile means CI tests the version a fresh install gets, which fixes the mismatch behind #4400. Both reviewers approved with no new findings. All earlier findings remain resolved at this head: the editorLabel comment placement, the nativeEvent handler comments, the leftover surface/toolbar aliases, the .js import, the link-authoring docs, and ToolbarMenu anchoring with geometry coverage. Two decisions are still for a maintainer: whether to accept Tiptap's new behavior where a link ends at the first typed space, and how to reconcile this PR with #4396, which changes the same toolbar code. Both reviews were static, so nothing was built or run.


Reviewed by anthropic/claude-opus-5-5 and openai/gpt-6.1-sol.

@emdash-autofix emdash-autofix Bot added triage/awaiting-reporter Fix pushed, waiting on reporter to verify and removed triage/awaiting-reporter Fix pushed, waiting on reporter to verify labels Oct 25, 2026

@danielmlr danielmlr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

#4396 has merged and fixes the toolbar keys a different way. The two PRs edit the same lines.

  1. Rebase onto main and drop these toolbar changes: the nativeEvent reads, the firstElementChild removals, and the ToolbarMenu anchor change in inline-editor-ui.tsx, which isn't needed once the toolbar sits inside the menu element again. #4396 puts the key handlers on EmDash's own element, so they get normal React events. Here they stay on Tiptap's menu element and read event.nativeEvent. A later handler on that element that reads event.key or event.shiftKey would throw "Illegal invocation" again.
  2. Keep the rest as it is: the move to 3.32.0, the table addNodeView override, the textbox names, the tests for the table, textbox names and link behaviour, and the link note in the changeset and docs.
  3. Rewrite the changeset's first sentence so it no longer claims the toolbar keyboard fix, since #4396 ships that.

~ 🤖 Claude Code (Opus 5.5)

@emdashbot emdashbot Bot added the review/needs-review No maintainer or bot review yet label Oct 27, 2026
@github-actions github-actions Bot removed the review/needs-review No maintainer or bot review yet label Oct 27, 2026
@emdash-autofix emdash-autofix Bot removed the triage/awaiting-reporter Fix pushed, waiting on reporter to verify label Oct 27, 2026
@danielmlr

Copy link
Copy Markdown
Contributor

@emdash-autofix #4396 is merged. It fixes the floating toolbar keys by putting EmDash's own element inside each BubbleMenu.

  1. Rebase onto main and drop these toolbar changes: the nativeEvent reads, the firstElementChild removals, and the ToolbarMenu anchor change in inline-editor-ui.tsx, which isn't needed once the toolbar sits inside the menu element again.
  2. Keep the rest as it is: the move to 3.32.0, the table addNodeView override, the textbox names, the tests for the table, textbox names and link behaviour, and the link note in the changeset and docs.
  3. Rewrite the changeset's first sentence so it no longer claims the toolbar keyboard fix, since fix(editor): keep floating toolbar events on React-owned surfaces #4396 ships that.

@emdash-autofix

Copy link
Copy Markdown
Contributor Author

Merged current main, including #4396, and resolved the conflicts in favor of its explicit React toolbar surfaces. Removed this PR’s native-event workarounds, toolbar anchor/animation changes, and superseded toolbar tests. The changeset now claims only the table/accessibility fixes and dependency/link behavior updates. The Tiptap upgrade, responsive table view, textbox names, relevant tests, and link documentation remain. Focused checks pass; publishing the merge update next.

Merge default branch 0409cd2 and resolve toolbar conflicts using main's #4396 implementation unchanged. Drop superseded native-event, surface, anchor, and toolbar-test changes; restore the original synchronous image geometry assertion. Keep the 3.32 catalog/lockfile update, responsive table override, textbox names, relevant regression tests, and link changeset/docs; remove the changeset's duplicate toolbar-fix claim. Verified frozen install, root/docs builds, workspace typecheck, quick lint, formatting, all 982 admin editor tests, 349 focused tests, core component/conversion coverage, main's nine menu tests, inline browser checks, and RTL/forced-colors table accessibility. Independent follow-up review cleared the merged scope.

EmDash-Bot-Case: 1198246700-issue-6025800409
EmDash-Bot-Candidate: cand-5-8de02d1625
@emdash-autofix

Copy link
Copy Markdown
Contributor Author

Done in the submitted merge update: incorporated main at 0409cd220949, kept #4396’s toolbar implementation unchanged, and dropped the native-event workarounds, surface changes, and ToolbarMenu anchor change. The dependency update, table override, textbox names, relevant tests, and link documentation remain. The changeset’s first sentence now covers responsive tables and textbox accessibility only. Focused checks pass; publication of the update is pending.

@emdashbot emdashbot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pinning and testing Tiptap 3.32.0 is the right approach to the install/CI mismatch in #4400. The table-view override, React-mounted highlighting tests, accessible names and link-authoring docs fit that goal. The reviewers disagree on the toolbar compatibility work. Claude concluded it is no longer in this diff and was probably taken over by #4396, so its earlier toolbar findings no longer apply. GPT found that the head still has the old keyboard handlers and wrapper-based sizing, which break under 3.32. I kept both of GPT's findings as needs_fixing because the previously approved fixes may be missing at this head, and Claude's explanation does not show that the regressions are handled elsewhere. A maintainer should either confirm that #4396 lands first and covers them, or restore the fixes here. The PR description also still describes toolbar fixes and an Escape test that are not in the diff. Whether to accept Tiptap's behavior where a link ends at the first typed space is still a maintainer decision. Both reviews were static; nothing was built or run.


Reviewed by anthropic/claude-opus-5-5 and openai/gpt-6.1-sol.

Comment thread pnpm-workspace.yaml
"@tiptap/pm": 3.32.0
"@tiptap/react": 3.32.0
"@tiptap/starter-kit": 3.32.0
"@tiptap/suggestion": 3.32.0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[needs fixing] The upgrade to @tiptap/react 3.32.0 is present, but the native-keyboard-event compatibility fix described in the PR is absent from this head. moveToolbarFocus in packages/admin/src/components/PortableTextEditor.tsx:6646 still reads event.key. handleToolbarKeyDown in packages/core/src/components/inline-editor-ui.tsx:583–610 still reads event.defaultPrevented, event.key and event.shiftKey from BubbleMenu's wrapped event. Per GPT, these getter accesses throw on 3.32, so keyboard navigation in the selection and image toolbars stays broken. Restore the native-event reads in both editors and the Escape regression coverage. Claude disagreed: it found this work is no longer in the diff and believes #4396 took it over, so it raised no finding. If #4396 covers it, merge order matters, because this pin would otherwise expose the regression.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

#4396 is already merged into this head via main 0409cd220949. These handlers are no longer props on BubbleMenu: admin attaches onKeyDown to its own child <div ref={surfaceRef}> (around lines 4991–4999), and core does the same in inline-bubble-menu.tsx around lines 628–634. They therefore receive real React keyboard events, not Tiptap’s synthetic native-event wrapper. The image handlers likewise sit on the explicit child div.

I retained that implementation as @danielmlr explicitly requested rather than restoring the superseded workaround. Verification after the merge included main’s nine core menu tests (Tab and arrow navigation), admin toolbar/image tests, and browser checks; all passed. No merge-order dependency remains.

Comment thread pnpm-workspace.yaml
"@tiptap/pm": 3.32.0
"@tiptap/react": 3.32.0
"@tiptap/starter-kit": 3.32.0
"@tiptap/suggestion": 3.32.0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[needs fixing] The wrapper-free floating-toolbar adaptation is also missing at this head. With 3.32 the BubbleMenu child is the positioned surface itself, but the admin sizing callback (PortableTextEditor.tsx:4933–4938) first sets elements.floating.style.maxWidth to the available pixel width and then overwrites it through surfaceRef.current with 100%. This loses the collision-aware width constraint on narrow viewports. The entrance animations also still target the old wrapper-based structure. Restore the wrapper-free sizing and animation changes in both editors, along with the ToolbarMenu anchoring fix and browser geometry coverage. Claude disagreed for the same reason as above: it treats this work as moved to #4396.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The two references are distinct DOM elements at this head. elements.floating is Tiptap’s positioned menu container; surfaceRef.current is EmDash’s explicit child <div> introduced by #4396. Thus the pixel max-width remains on the outer container, while 100% and horizontal scrolling apply to its child. Animations target that child, and ToolbarMenu’s parent anchor again points to the positioned container.

This follows the maintainer’s instruction to keep #4396 and drop our wrapper-free adaptation. The existing entrance-positioning browser test and focused toolbar tests passed after merging main. The latest submitted PR description explicitly records this reconciliation and removal of the extra toolbar tests; it no longer claims those toolbar fixes as part of this diff.

@emdash-autofix emdash-autofix Bot added triage/awaiting-reporter Fix pushed, waiting on reporter to verify and removed triage/awaiting-reporter Fix pushed, waiting on reporter to verify labels Oct 27, 2026

@danielmlr danielmlr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The toolbar fix from #4396 works on Tiptap 3.32, so this PR doesn't need any toolbar changes.

  1. Rewrite the PR description to match the diff. It still describes toolbar fixes, an Escape test, toolbar screenshots and the overlap with #4396, which are no longer part of this PR.
  2. Add to the changeset that in the visual editor, typing a space at the end of an existing link now ends the link. The admin editor already doesn't extend a link once you move the caret to its end, so the docs paragraph can stay as it is.
  3. Update the comment on setSelectedTextLink in PortableTextEditor.tsx. The text typed next now gets the link only up to the first space.

Keep everything else as it is.

@emdashbot emdashbot Bot added the review/needs-review No maintainer or bot review yet label Oct 27, 2026
@github-actions github-actions Bot removed the review/needs-review No maintainer or bot review yet label Oct 27, 2026
@danielmlr

Copy link
Copy Markdown
Contributor

@emdash-autofix The toolbar fix from #4396 works on Tiptap 3.32, so this PR doesn't need any toolbar changes.

  1. Rewrite the PR description to match the diff. It still describes toolbar fixes, an Escape test, toolbar screenshots and the overlap with fix(editor): keep floating toolbar events on React-owned surfaces #4396, which are no longer part of this PR.
  2. Add to the changeset that in the visual editor, typing a space at the end of an existing link now ends the link. The admin editor already doesn't extend a link once you move the caret to its end, so the docs paragraph can stay as it is.
  3. Update the comment on setSelectedTextLink in PortableTextEditor.tsx. The text typed next now gets the link only up to the first space.

Keep everything else as it is.

@emdash-autofix emdash-autofix Bot removed the triage/awaiting-reporter Fix pushed, waiting on reporter to verify label Oct 27, 2026
@emdash-autofix

Copy link
Copy Markdown
Contributor Author

Updated all three items: the PR description now covers only the remaining dependency, table, textbox-name, test, and link changes, with table screenshots only; the changeset now calls out space-at-end behavior for existing links in the visual editor; and setSelectedTextLink’s comment now says typing is linked only up to the first space. Everything else, including the docs paragraph, is unchanged. Formatting and diff checks pass, and independent review found no significant issues.

Address the maintainer's remaining wording requests only: describe the current scoped diff without obsolete toolbar claims or screenshots, add the visual-editor space-at-end-of-existing-link behavior to the changeset, and clarify that setSelectedTextLink links newly typed text only up to its first space. Implementation, tests, and the authoring-guide paragraph are unchanged. Ran pnpm format and git diff --check; independent review found no significant issues. The preceding implementation head c6d7985 passed required CI.

EmDash-Bot-Case: 1198246700-issue-6025800409
EmDash-Bot-Candidate: cand-6-129d976203

@emdashbot emdashbot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pinning and testing Tiptap 3.32.0 is the right approach to the fresh-install/CI mismatch in #4400. Both reviewers approved and neither raised a new finding at this head. The table-view override, React-mounted highlighting tests, accessible names and link-authoring docs fit that goal. The earlier toolbar concerns (native keyboard events, wrapper-free sizing, ToolbarMenu anchoring) are resolved: #4396 is merged through main, so the handlers sit on EmDash's own child element and the pixel max-width stays on the separate positioned container. The earlier suggestions (.js import extension, comment placement, link docs) were also addressed. Two items remain that are not code defects. The PR description still mentions toolbar fixes and an Escape test that are not in this diff, and the author's reply says it was updated. A maintainer still has to decide whether to accept Tiptap's behavior of ending a link at the first typed space. Both reviews were static; nothing was built or run.


Reviewed by anthropic/claude-opus-5-5 and openai/gpt-6.1-sol.

@emdashbot emdashbot Bot added the review/needs-review No maintainer or bot review yet label Oct 27, 2026
@emdash-autofix

Copy link
Copy Markdown
Contributor Author

The source changes are approved. There is a remaining description-publication discrepancy: candidate 6 supplied a replacement body with no toolbar-fix claims, Escape-test claims, overlap discussion, or toolbar screenshots, but this review still reports the old text. I cannot read or directly edit the live PR body with the available tools.

@danielmlr, please check the published body. If it is stale, the intended summary is:

Pins and tests Tiptap 3.32.0, retaining lowlight 3.20.0; restores the responsive table view; names the admin and visual-editor textboxes; updates highlighting, selection, and link tests; and documents the upstream space-at-link-end behavior. The exact pin keeps fresh installs aligned with CI. Integrations sharing Tiptap should align their versions to avoid duplicate copies.

Only the table before/after screenshots should remain. The complete replacement body was supplied with cand-6-129d976203; no further source changes are needed.

@github-actions github-actions Bot removed the review/needs-review No maintainer or bot review yet label Oct 27, 2026
@danielmlr

Copy link
Copy Markdown
Contributor

@emdash-autofix I checked the published body. I updated it after your comment, and it now matches your summary: no toolbar-fix or Escape-test claims, nothing about the overlap with #4396, and only the table before/after screenshots (English and Arabic). Nothing else is open from my side, so this PR now just needs a second maintainer's approval.

@emdash-autofix

Copy link
Copy Markdown
Contributor Author

Thanks for checking and updating the published description. No further changes from me; leaving the PR ready for the second maintainer’s approval.

@emdash-autofix emdash-autofix Bot added the triage/awaiting-reporter Fix pushed, waiting on reporter to verify label Oct 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/admin area/core area/docs bot:fix Assign this issue or pull request to the EmDash bot to fix cla: signed size/XL triage/awaiting-reporter Fix pushed, waiting on reporter to verify

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fresh installs get tiptap 3.32, where admin editor tests fail, but CI only tests 3.20

1 participant