fix: stop plan bubble menu transaction loop (#3083)

This commit is contained in:
Carlos Florêncio
2026-08-27 12:05:22 +00:00
committed by GitHub
parent 60140b83c6
commit 5c5b3c6d90
5 changed files with 404 additions and 7 deletions
@@ -1,4 +1,5 @@
import { act, cleanup, fireEvent, render, screen } from "@testing-library/react";
import { useEffect, useRef } from "react";
import type { Editor } from "@tiptap/core";
import { afterEach, describe, expect, it, vi } from "vitest";
@@ -14,6 +15,13 @@ const mocks = vi.hoisted(() => ({
keyboardOpen: false,
viewportBottom: 800,
},
bubbleMenu: {
optionUpdateTransactions: 0,
renderCount: 0,
testId: "plan-selection-bubble",
visibleValue: "true",
hiddenValue: "false",
},
}));
vi.mock("@/hooks/use-responsive-breakpoint", () => ({
@@ -49,16 +57,32 @@ vi.mock("react-i18next", () => ({
vi.mock("@tiptap/react/menus", () => ({
BubbleMenu: ({
editor,
options,
shouldShow,
children,
}: {
editor: Editor;
options?: object;
shouldShow?: (props: { editor: Editor; state: Editor["state"] }) => boolean;
children: React.ReactNode;
}) => {
mocks.bubbleMenu.renderCount += 1;
const previousOptions = useRef(options);
useEffect(() => {
if (previousOptions.current === options) return;
previousOptions.current = options;
mocks.bubbleMenu.optionUpdateTransactions += 1;
if (mocks.bubbleMenu.optionUpdateTransactions < 3) {
(editor as FakeEditor).emit("transaction");
}
}, [editor, options]);
const isVisible = shouldShow?.({ editor, state: editor.state }) ?? true;
return (
<div data-testid="plan-selection-bubble" data-visible={isVisible ? "true" : "false"}>
<div
data-testid={mocks.bubbleMenu.testId}
data-visible={isVisible ? mocks.bubbleMenu.visibleValue : mocks.bubbleMenu.hiddenValue}
>
{isVisible ? children : null}
</div>
);
@@ -68,11 +92,14 @@ vi.mock("@tiptap/react/menus", () => ({
import { PlanBubbleMenu } from "./plan-bubble-menu";
const PLAN_TOOLBAR_LABEL = "editors:planFormattingToolbar";
const VISIBILITY_ATTRIBUTE = "data-visible";
type FakeEditor = Editor & {
emit: (event: string) => void;
setSelection: (from: number, to: number, text?: string) => void;
setFocused: (focused: boolean) => void;
setActiveMark: (name: string, active: boolean) => void;
setCodeBlock: (active: boolean) => void;
chainMock: {
focus: ReturnType<typeof vi.fn>;
toggleBold: ReturnType<typeof vi.fn>;
@@ -95,6 +122,8 @@ function createEditor({
} = {}): FakeEditor {
const handlers = new Map<string, Set<() => void>>();
const selection = { from, to };
let codeBlockState = codeBlock;
const activeMarks = new Set<string>();
const chainMock = {
focus: vi.fn(),
toggleBold: vi.fn(),
@@ -109,7 +138,9 @@ function createEditor({
selection,
doc: { textBetween: vi.fn(() => text) },
},
isActive: vi.fn((name: string) => name === "codeBlock" && codeBlock),
isActive: vi.fn((name: string) =>
name === "codeBlock" ? codeBlockState : activeMarks.has(name),
),
getAttributes: vi.fn(() => ({})),
chain: vi.fn(() => chainMock),
commands: { focus: vi.fn() },
@@ -137,6 +168,16 @@ function createEditor({
editor.setFocused = (nextFocused) => {
editor.isFocused = nextFocused;
};
editor.setActiveMark = (name, active) => {
if (active) {
activeMarks.add(name);
} else {
activeMarks.delete(name);
}
};
editor.setCodeBlock = (active) => {
codeBlockState = active;
};
editor.chainMock = chainMock;
return editor;
}
@@ -150,6 +191,8 @@ afterEach(() => {
mocks.viewport.keyboardOpen = false;
mocks.viewport.bottomOffset = 0;
mocks.viewport.viewportBottom = 800;
mocks.bubbleMenu.optionUpdateTransactions = 0;
mocks.bubbleMenu.renderCount = 0;
});
describe("PlanBubbleMenu responsive presentation", () => {
@@ -159,7 +202,7 @@ describe("PlanBubbleMenu responsive presentation", () => {
render(<PlanBubbleMenu editor={editor} onComment={vi.fn()} mobileBottomOffset="3.25rem" />);
expect(screen.getByRole("toolbar", { name: PLAN_TOOLBAR_LABEL })).toBeTruthy();
expect(screen.queryByTestId("plan-selection-bubble")).toBeNull();
expect(screen.queryByTestId(mocks.bubbleMenu.testId)).toBeNull();
});
});
@@ -214,6 +257,31 @@ describe("PlanBubbleMenu mobile selection behavior", () => {
});
describe("PlanBubbleMenu responsive layout", () => {
it("@covers AC-UI-RESPONSIVE-PLAN-FORMATTING-001.1: deduplicates unchanged metadata transactions", () => {
mocks.breakpoint.isFinePointer = true;
mocks.breakpoint.isMobile = false;
mocks.breakpoint.usesDesktopWorkbench = true;
const editor = createEditor({ focused: false });
render(<PlanBubbleMenu editor={editor} />);
act(() => {
editor.setFocused(true);
editor.emit("focus");
});
const renderCountAfterFocus = mocks.bubbleMenu.renderCount;
const optionUpdatesAfterFocus = mocks.bubbleMenu.optionUpdateTransactions;
act(() => {
editor.emit("transaction");
});
expect(mocks.bubbleMenu.renderCount).toBe(renderCountAfterFocus);
expect(mocks.bubbleMenu.optionUpdateTransactions).toBe(optionUpdatesAfterFocus);
expect(optionUpdatesAfterFocus).toBe(0);
});
it("keeps the dock visible while the link input owns focus", () => {
const editor = createEditor();
@@ -278,10 +346,14 @@ describe("PlanBubbleMenu responsive layout", () => {
render(<PlanBubbleMenu editor={editor} onComment={vi.fn()} />);
expect(screen.getByTestId("plan-selection-bubble").getAttribute("data-visible")).toBe("true");
expect(screen.getByTestId(mocks.bubbleMenu.testId).getAttribute(VISIBILITY_ATTRIBUTE)).toBe(
mocks.bubbleMenu.visibleValue,
);
expect(screen.queryByRole("toolbar", { name: PLAN_TOOLBAR_LABEL })).toBeNull();
});
});
describe("PlanBubbleMenu reactive editor state", () => {
it("updates mobile visibility from editor focus and transaction events", () => {
const editor = createEditor({ focused: false });
render(<PlanBubbleMenu editor={editor} />);
@@ -300,6 +372,45 @@ describe("PlanBubbleMenu responsive layout", () => {
});
expect(screen.queryByRole("toolbar", { name: PLAN_TOOLBAR_LABEL })).toBeNull();
});
it("updates active mark controls from transaction events", () => {
const editor = createEditor();
render(<PlanBubbleMenu editor={editor} />);
const bold = screen.getByRole("button", { name: "editors:boldCmdB" });
expect(bold.getAttribute("aria-pressed")).toBe(mocks.bubbleMenu.hiddenValue);
act(() => {
editor.setActiveMark("bold", true);
editor.emit("transaction");
});
expect(bold.getAttribute("aria-pressed")).toBe(mocks.bubbleMenu.visibleValue);
act(() => {
editor.setActiveMark("bold", false);
editor.emit("transaction");
});
expect(bold.getAttribute("aria-pressed")).toBe(mocks.bubbleMenu.hiddenValue);
});
it("updates mobile toolbar visibility from code-block transactions", () => {
const editor = createEditor();
render(<PlanBubbleMenu editor={editor} />);
expect(screen.getByRole("toolbar", { name: PLAN_TOOLBAR_LABEL })).toBeTruthy();
act(() => {
editor.setCodeBlock(true);
editor.emit("transaction");
});
expect(screen.queryByRole("toolbar", { name: PLAN_TOOLBAR_LABEL })).toBeNull();
act(() => {
editor.setCodeBlock(false);
editor.emit("transaction");
});
expect(screen.getByRole("toolbar", { name: PLAN_TOOLBAR_LABEL })).toBeTruthy();
});
});
describe("PlanBubbleMenu code-block suppression", () => {
@@ -319,6 +430,32 @@ describe("PlanBubbleMenu code-block suppression", () => {
render(<PlanBubbleMenu editor={editor} onComment={vi.fn()} />);
expect(screen.getByTestId("plan-selection-bubble").getAttribute("data-visible")).toBe("false");
expect(screen.getByTestId(mocks.bubbleMenu.testId).getAttribute(VISIBILITY_ATTRIBUTE)).toBe(
mocks.bubbleMenu.hiddenValue,
);
});
it("updates desktop selection bubble visibility from code-block transactions", () => {
mocks.breakpoint.isFinePointer = true;
mocks.breakpoint.isMobile = false;
mocks.breakpoint.usesDesktopWorkbench = true;
const editor = createEditor();
render(<PlanBubbleMenu editor={editor} />);
const bubble = screen.getByTestId(mocks.bubbleMenu.testId);
expect(bubble.getAttribute(VISIBILITY_ATTRIBUTE)).toBe(mocks.bubbleMenu.visibleValue);
act(() => {
editor.setCodeBlock(true);
editor.emit("transaction");
});
expect(bubble.getAttribute(VISIBILITY_ATTRIBUTE)).toBe(mocks.bubbleMenu.hiddenValue);
act(() => {
editor.setCodeBlock(false);
editor.emit("transaction");
});
expect(bubble.getAttribute(VISIBILITY_ATTRIBUTE)).toBe(mocks.bubbleMenu.visibleValue);
});
});
@@ -60,6 +60,26 @@ type EditorSnapshot = {
isLink: boolean;
};
const PLAN_BUBBLE_MENU_OPTIONS = { placement: "top" } as const;
const EDITOR_SNAPSHOT_KEYS = {
isFocused: "isFocused",
isCodeBlock: "isCodeBlock",
hasTextSelection: "hasTextSelection",
hasCommentSelection: "hasCommentSelection",
isBold: "isBold",
isItalic: "isItalic",
isUnderline: "isUnderline",
isStrike: "isStrike",
isCode: "isCode",
isHighlight: "isHighlight",
isLink: "isLink",
} as const satisfies { [K in keyof EditorSnapshot]: K };
const EDITOR_SNAPSHOT_KEY_VALUES = Object.values(EDITOR_SNAPSHOT_KEYS) as Array<
keyof EditorSnapshot
>;
function readEditorSnapshot(editor: Editor): EditorSnapshot {
const { from, to } = editor.state.selection;
const selectedText = editor.state.doc.textBetween(from, to, " ").trim();
@@ -79,11 +99,20 @@ function readEditorSnapshot(editor: Editor): EditorSnapshot {
};
}
function areEditorSnapshotsEqual(first: EditorSnapshot, second: EditorSnapshot): boolean {
return EDITOR_SNAPSHOT_KEY_VALUES.every((key) => first[key] === second[key]);
}
function useEditorSnapshot(editor: Editor): EditorSnapshot {
const [snapshot, setSnapshot] = useState(() => readEditorSnapshot(editor));
useEffect(() => {
const update = () => setSnapshot(readEditorSnapshot(editor));
const update = () => {
setSnapshot((current) => {
const next = readEditorSnapshot(editor);
return areEditorSnapshotsEqual(current, next) ? current : next;
});
};
editor.on("transaction", update);
editor.on("focus", update);
editor.on("blur", update);
@@ -558,7 +587,11 @@ export function PlanBubbleMenu({
}
return (
<BubbleMenu editor={editor} options={{ placement: "top" }} shouldShow={shouldShowBubbleMenu}>
<BubbleMenu
editor={editor}
options={PLAN_BUBBLE_MENU_OPTIONS}
shouldShow={shouldShowBubbleMenu}
>
{content}
</BubbleMenu>
);
@@ -0,0 +1,111 @@
---
created: 2026-08-27
status: done
requirements:
- REQ-UI-RESPONSIVE-PLAN-FORMATTING-001
system_design:
- ../../specs/ui/system-design/responsive-plan-formatting.md
legacy_specs: []
---
# Implementation Plan: Stop the Plan Bubble Menu Transaction Loop
## Overview
Stop the desktop Plan editor from running a continuous React and Tiptap
transaction loop. Implement one focused TDD work order because the fault and
its regression boundary are in one component.
The trace shows approximately 44,800 React scheduler callbacks in 3.56
seconds. The DOM stays at 4,862 elements. The source-mapped stack shows a
Tiptap BubbleMenu option update followed by an editor transaction and another
PlanBubbleMenu render.
## Scope
### In scope
- Keep the desktop BubbleMenu option identity stable across unchanged renders.
- Publish a new editor snapshot only when its derived values change.
- Add a focused regression that models the real BubbleMenu option-update
transaction.
- Preserve the desktop selection bubble and the mobile docked formatting
strip.
### Out of scope
- Change Plan formatting commands, layout, touch geometry, or viewport
behavior.
- Change Tiptap, ProseMirror, task persistence, or WebSocket code.
- Add performance telemetry or a general React render monitor.
- Change the public plugin editor contract.
## Technical approach
### Stable BubbleMenu inputs
Define the placement options once in
`apps/web/components/editors/tiptap/plan-bubble-menu.tsx`. Pass that stable
object to the desktop BubbleMenu.
### Deduplicated editor snapshots
Compare the next `EditorSnapshot` with the current snapshot. Return the
current state when all derived values are equal. Publish the next snapshot
only after focus, selection, context, or active marks change.
### Regression boundary
Extend `plan-bubble-menu.test.tsx` with a controlled BubbleMenu fake. The fake
models Tiptap's option dependency and its metadata transaction. The test fails
on the current code because the transaction repeats after each render.
## Tests
| Acceptance criterion | Evidence |
| --- | --- |
| `AC-UI-RESPONSIVE-PLAN-FORMATTING-001.1` | The focused Vitest regression proves that the desktop BubbleMenu settles after one option update. Focus, selection, active-mark, and desktop/mobile code-block transitions remain reactive. |
| `AC-UI-RESPONSIVE-PLAN-FORMATTING-001.2` through `.8` | The existing mobile component tests and `mobile-plan-formatting-toolbar.spec.ts` prove that the docked presentation keeps its current behavior. |
## E2E tests
This fix changes render-state normalization only. It does not change layout,
touch behavior, scrolling, navigation, or viewport-dependent interaction.
The existing `mobile-plan-formatting-toolbar.spec.ts` remains the mobile
parity evidence. Run its `mobile-chrome` scenario after the focused unit test.
## Work orders
- [x] [Task 01: Stop the BubbleMenu transaction loop](task-01-stop-bubble-menu-transaction-loop.md) (done)
## Verification results
- Red regression: the controlled BubbleMenu fake observed four option-update
transactions before the fix and failed the settlement assertion.
- `cd apps/web && pnpm exec vitest run components/editors/tiptap/plan-bubble-menu.test.tsx`
passed, 13 tests, including active-mark and code-block transaction changes.
- `cd apps/web && pnpm run typecheck` passed.
- `cd apps/web && pnpm exec eslint components/editors/tiptap/plan-bubble-menu.tsx
components/editors/tiptap/plan-bubble-menu.test.tsx` passed.
- `cd apps/web && pnpm e2e:run --project mobile-chrome
tests/task/mobile-plan-formatting-toolbar.spec.ts` passed, 1 test.
- Fresh desktop and mobile PR screenshots were captured, inspected, validated,
and compressed. The disposable desktop capture spec was removed.
## Risks
- An incomplete snapshot comparison can leave pressed formatting state stale.
- An unstable BubbleMenu prop can reintroduce metadata transactions on each
render.
- A weak menu mock can hide the Tiptap effect that caused the fault.
## Public documentation
None. This fix changes no public command, configuration, API, workflow, or
plugin contract.
## Decisions
No ADR is required. The fix completes the current component design without a
new ownership or public contract boundary.
@@ -0,0 +1,99 @@
---
id: "01-stop-bubble-menu-transaction-loop"
title: "Stop the BubbleMenu transaction loop"
status: done
wave: 1
depends_on: []
plan: "plan.md"
requirements:
- REQ-UI-RESPONSIVE-PLAN-FORMATTING-001
acceptance_criteria:
- AC-UI-RESPONSIVE-PLAN-FORMATTING-001.1
- AC-UI-RESPONSIVE-PLAN-FORMATTING-001.2
system_design:
- ../../specs/ui/system-design/responsive-plan-formatting.md
---
# Task 01: Stop the BubbleMenu Transaction Loop
## Summary
Make the desktop BubbleMenu inputs stable and deduplicate the reactive editor
snapshot. Add a focused regression that models Tiptap's option-update
transaction and proves that the component settles.
## In scope
- Add the failing transaction-loop regression before production changes.
- Keep the desktop BubbleMenu placement options referentially stable.
- Compare all `EditorSnapshot` fields before publishing state.
- Keep focus, selection, code-block, and active-mark changes reactive.
- Run the existing mobile formatting scenario after the focused checks pass.
## Out of scope
- Change rendered layout, action order, touch targets, or keyboard geometry.
- Change Plan content, comments, autosave, or revisions.
- Change Tiptap or ProseMirror package code.
- Add a new browser performance harness.
## Acceptance
- The regression fails before production changes because BubbleMenu option
updates cause repeated editor transactions.
- The corrected component settles after one option update when the editor
snapshot is unchanged.
- Real focus, selection, code-block, and active-mark changes still update the
formatting controls.
## Verification
```bash
cd apps/web
pnpm exec vitest run components/editors/tiptap/plan-bubble-menu.test.tsx
pnpm run typecheck
pnpm exec eslint components/editors/tiptap/plan-bubble-menu.tsx components/editors/tiptap/plan-bubble-menu.test.tsx
pnpm e2e:run --project mobile-chrome tests/task/mobile-plan-formatting-toolbar.spec.ts
```
## Files likely touched
- `apps/web/components/editors/tiptap/plan-bubble-menu.tsx`
- `apps/web/components/editors/tiptap/plan-bubble-menu.test.tsx`
- `docs/plans/plan-bubble-menu-transaction-loop/plan.md`
- `docs/plans/plan-bubble-menu-transaction-loop/task-01-stop-bubble-menu-transaction-loop.md`
## Dependencies
None.
## Risks
- The test fake must model the Tiptap options effect without creating an
unbounded test loop.
- The snapshot comparison must include every field in `EditorSnapshot`.
## Parallelism
`sequential`
## Inputs
- `REQ-UI-RESPONSIVE-PLAN-FORMATTING-001` acceptance criteria `.1` and `.2`.
- `docs/specs/ui/system-design/responsive-plan-formatting.md`.
- The performance trace attached to the investigation task.
- Tiptap BubbleMenu's option-update effect in `@tiptap/react`.
- Existing PlanBubbleMenu component tests and the mobile formatting E2E.
## Results
- Added a controlled BubbleMenu fake that reproduces repeated option-update
transactions and fails before the fix.
- Stabilized the desktop BubbleMenu placement options at module scope.
- Deduplicated all `EditorSnapshot` fields before publishing React state.
- Extended the fake editor regression to verify active-mark button state and
mobile and desktop code-block visibility changes after transactions.
- Focused Vitest, typecheck, changed-file ESLint, and the existing mobile Plan
formatting E2E all pass.
- No mobile E2E change was needed because this internal normalization preserves
the existing mobile presentation; the existing scenario covers it.
@@ -100,6 +100,23 @@ The desktop BubbleMenu retains its non-empty-selection visibility condition.
It consumes the same reactive active-mark snapshot as the docked strip so
pressed styling follows selection and command transactions in both variants.
## Render and transaction stability
The desktop BubbleMenu receives stable option and callback identities when
their values do not change. Tiptap dispatches a metadata transaction when its
menu options change. A new options object on each render can create a feedback
loop between that transaction and the editor snapshot subscription.
The editor snapshot subscription compares each derived snapshot with the
current snapshot. It publishes state only when focus, selection, code-block
context, or active marks change. Metadata-only transactions do not cause a
render. Selection and formatting transactions still publish the changed
snapshot.
These two controls keep the desktop selection bubble reactive without making
Tiptap option updates part of the React render cycle. The mobile presentation
uses the same deduplicated editor snapshot.
## Viewport and scroll contract
The docked strip has a stable measured height. Its position resolves as follows: