From bd9d207966375d440956912e8f26bdc54831b577 Mon Sep 17 00:00:00 2001 From: Jedr Blaszyk Date: Thu, 16 Apr 2026 11:04:43 +0200 Subject: [PATCH 1/6] fix: resolved comment checkmark didn't reopen (fixes #268) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PagedEditor.updateSelectionOverlay fires from six paths — transactions, view-ready, font-load, ResizeObserver, layout effect, HiddenPM selection change — but the onSelectionChange callback fired unconditionally on every one of them. Any overlay redraw re-invoked consumers with the unchanged cursor; in the sidebar-expansion branch added by #244 that meant the ResizeObserver fired right after the click that set expandedSidebarItem, re-ran the cursor-mark detector against the (unchanged) cursor at the document end, and cleared the expansion within a frame — so the checkmark click looked like it did nothing. Fix at the source: dedup by PM state identity. States are immutable, so reference equality is the canonical "nothing changed" signal and covers selection / doc / stored-marks changes in one check. Overlay geometry redraw still runs — only the public callback is guarded. Also adds a comprehensive e2e suite (comments-sidebar.spec.ts, 11 tests) covering cursor-based expansion, resolve/reopen, and feedback-loop stability. The resize-specific test fails without the dedup (verified by stashing the fix) and passes with it. Co-Authored-By: Claude Opus 4.6 (1M context) --- .changeset/fix-resolved-comment-toggle.md | 5 + CLAUDE.md | 35 +- e2e/tests/comments-sidebar.spec.ts | 447 ++++++++++++++++++ .../sidebar/ResolvedCommentMarker.tsx | 8 +- .../react/src/paged-editor/PagedEditor.tsx | 19 +- 5 files changed, 497 insertions(+), 17 deletions(-) create mode 100644 .changeset/fix-resolved-comment-toggle.md create mode 100644 e2e/tests/comments-sidebar.spec.ts diff --git a/.changeset/fix-resolved-comment-toggle.md b/.changeset/fix-resolved-comment-toggle.md new file mode 100644 index 00000000..da454697 --- /dev/null +++ b/.changeset/fix-resolved-comment-toggle.md @@ -0,0 +1,5 @@ +--- +'@eigenpal/docx-js-editor': patch +--- + +Fix a regression where clicking the checkmark of a resolved comment did not re-open the comment card (issue #268). `PagedEditor.updateSelectionOverlay` fired `onSelectionChange` from every overlay redraw — including ResizeObserver and layout/font callbacks — not only on actual selection changes. When the sidebar card resize (or any window resize) triggered a redraw, the parent received a spurious callback with the unchanged cursor and cleared the just-set expansion. Dedup by PM state identity (immutable references) so consumers are only notified for real selection / doc / stored-marks changes. diff --git a/CLAUDE.md b/CLAUDE.md index d21bab6c..0935864b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -30,19 +30,28 @@ bun run typecheck && npx playwright test --timeout=60000 --workers=4 ### Test File Mapping -| Feature Area | Test File | Quick Verify Pattern | -| --------------------- | ------------------------------ | ----------------------- | -| Bold/Italic/Underline | `formatting.spec.ts` | `--grep "apply bold"` | -| Alignment | `alignment.spec.ts` | `--grep "align text"` | -| Lists | `lists.spec.ts` | `--grep "bullet list"` | -| Colors | `colors.spec.ts` | `--grep "text color"` | -| Fonts | `fonts.spec.ts` | `--grep "font family"` | -| Enter/Paragraphs | `text-editing.spec.ts` | `--grep "Enter"` | -| Undo/Redo | `scenario-driven.spec.ts` | `--grep "undo"` | -| Line spacing | `line-spacing.spec.ts` | `--grep "line spacing"` | -| Paragraph styles | `paragraph-styles.spec.ts` | `--grep "Heading"` | -| Toolbar state | `toolbar-state.spec.ts` | `--grep "toolbar"` | -| Cursor-only ops | `cursor-paragraph-ops.spec.ts` | `--grep "cursor only"` | +| Feature Area | Test File | Quick Verify Pattern | +| --------------------- | ------------------------------ | --------------------------- | +| Bold/Italic/Underline | `formatting.spec.ts` | `--grep "apply bold"` | +| Alignment | `alignment.spec.ts` | `--grep "align text"` | +| Lists | `lists.spec.ts` | `--grep "bullet list"` | +| Colors | `colors.spec.ts` | `--grep "text color"` | +| Fonts | `fonts.spec.ts` | `--grep "font family"` | +| Enter/Paragraphs | `text-editing.spec.ts` | `--grep "Enter"` | +| Undo/Redo | `scenario-driven.spec.ts` | `--grep "undo"` | +| Line spacing | `line-spacing.spec.ts` | `--grep "line spacing"` | +| Paragraph styles | `paragraph-styles.spec.ts` | `--grep "Heading"` | +| Toolbar state | `toolbar-state.spec.ts` | `--grep "toolbar"` | +| Cursor-only ops | `cursor-paragraph-ops.spec.ts` | `--grep "cursor only"` | +| Comments sidebar | `comments-sidebar.spec.ts` | `--grep "Comments sidebar"` | + +**When touching anything in these paths, run `comments-sidebar.spec.ts`:** + +- `packages/react/src/components/UnifiedSidebar.tsx` +- `packages/react/src/components/sidebar/**` +- `packages/react/src/hooks/useCommentSidebarItems.tsx` +- `packages/react/src/paged-editor/PagedEditor.tsx` → `updateSelectionOverlay` / `onSelectionChange` +- `packages/react/src/components/DocxEditor.tsx` → `onSelectionChange` handler, `expandedSidebarItem` state **Known flaky tests:** `formatting.spec.ts` (bold toggle/undo/redo), `text-editing.spec.ts` (clipboard ops). diff --git a/e2e/tests/comments-sidebar.spec.ts b/e2e/tests/comments-sidebar.spec.ts new file mode 100644 index 00000000..897fa23f --- /dev/null +++ b/e2e/tests/comments-sidebar.spec.ts @@ -0,0 +1,447 @@ +/** + * Comments Sidebar — cursor-based expansion, resolve, reopen + * + * Covers the cursor-driven sidebar expansion refactor (#244) and the + * state-identity dedup in PagedEditor.updateSelectionOverlay that fixes #268 + * (resolved comment checkmark didn't reopen because ResizeObserver re-fired + * onSelectionChange with the unchanged cursor, clearing the just-set expansion). + * + * Run when touching: + * - packages/react/src/paged-editor/PagedEditor.tsx (updateSelectionOverlay) + * - packages/react/src/components/UnifiedSidebar.tsx + * - packages/react/src/components/sidebar/** + * - packages/react/src/hooks/useCommentSidebarItems.tsx + * - DocxEditor.tsx onSelectionChange / expandedSidebarItem logic + */ + +import { test, expect, Page } from '@playwright/test'; +import { EditorPage } from '../helpers/editor-page'; + +// --------------------------------------------------------------------------- +// Selectors & helpers +// --------------------------------------------------------------------------- + +const SIDEBAR = '.docx-unified-sidebar'; +const COMMENT_CARD = '.docx-comment-card'; +// Sentinel word appended to every test's body to give moveCursorTo a +// known location that is definitely outside every comment mark. +const CLEAN = 'ZZEND'; + +async function openSidebar(page: Page) { + const toggle = page.locator('[aria-label="Toggle comments sidebar"]'); + const pressed = await toggle.getAttribute('aria-pressed'); + if (pressed !== 'true') { + await toggle.click(); + await page.waitForTimeout(100); + } +} + +async function startAddComment(editor: EditorPage, searchText: string) { + const { page } = editor; + const found = await editor.selectText(searchText); + expect(found, `selectText should find "${searchText}"`).toBe(true); + await page.waitForTimeout(200); + + await page.waitForFunction( + () => { + const buttons = document.querySelectorAll('[data-testid="docx-editor"] button'); + for (const b of buttons) { + const s = getComputedStyle(b); + if (s.position === 'absolute' && s.zIndex === '50') return true; + } + return false; + }, + null, + { timeout: 3000 } + ); + + const floating = await page.evaluateHandle(() => { + const buttons = document.querySelectorAll('[data-testid="docx-editor"] button'); + for (const b of buttons) { + const s = getComputedStyle(b); + if (s.position === 'absolute' && s.zIndex === '50') return b; + } + return null; + }); + const floatingEl = floating.asElement(); + expect(floatingEl, 'floating add-comment button should mount').not.toBeNull(); + await floatingEl!.click(); + await page.waitForSelector(`${SIDEBAR} textarea`, { state: 'visible', timeout: 3000 }); +} + +async function submitComment(page: Page, text: string) { + const textarea = page.locator(`${SIDEBAR} textarea`).last(); + await textarea.fill(text); + await textarea.press('Enter'); + await page.waitForTimeout(300); +} + +async function addCommentOn(editor: EditorPage, searchText: string, body: string) { + await startAddComment(editor, searchText); + await submitComment(editor.page, body); +} + +function commentCard(page: Page, commentId: number) { + return page.locator(`${SIDEBAR} ${COMMENT_CARD}[data-comment-id="${commentId}"]`); +} + +function resolvedMarker(page: Page, commentId: number) { + return page.locator(`${SIDEBAR} [data-comment-id="${commentId}"][data-comment-resolved="true"]`); +} + +async function getSidebarCommentIds(page: Page): Promise { + return page.evaluate(() => { + const els = document.querySelectorAll('.docx-unified-sidebar [data-comment-id]'); + const ids = new Set(); + for (const el of els) { + const raw = (el as HTMLElement).dataset.commentId; + if (raw) { + const n = Number(raw); + if (!Number.isNaN(n)) ids.add(n); + } + } + return [...ids].sort((a, b) => a - b); + }); +} + +/** + * A card is "expanded" when the full CommentCard renders — that form has a + * Resolve (or Reopen, if already resolved) button. Collapsed active comments + * render only the avatar/name/text; collapsed resolved ones render the + * checkmark marker (no data-comment-resolved on the full card either). + */ +async function isCommentExpanded(page: Page, commentId: number): Promise { + const card = commentCard(page, commentId); + if ((await card.count()) === 0) return false; + const actionBtn = card.locator('button[title="Resolve"], button[title="Reopen"]'); + return (await actionBtn.count()) > 0; +} + +/** + * Move the PM cursor to a position with no comment/tracked-change mark by + * selecting a known-clean word (caller supplies one) and collapsing forward. + * Using selectText rather than keyboard shortcuts avoids issues with the + * hidden off-screen PM editor not receiving focus from editor.focus(). + */ +async function moveCursorTo(editor: EditorPage, cleanWord: string) { + const found = await editor.selectText(cleanWord); + expect(found, `moveCursorTo: word "${cleanWord}" should be in the doc`).toBe(true); + await editor.page.keyboard.press('ArrowRight'); + await editor.page.waitForTimeout(200); +} + +async function setupEmptyDoc(page: Page): Promise { + const editor = new EditorPage(page); + await editor.goto(); + await editor.waitForReady(); + await editor.newDocument(); + await editor.focus(); + return editor; +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +test.describe('Comments sidebar — cursor-based expansion', () => { + test('cursor entering a comment range expands its card, leaving collapses it', async ({ + page, + }) => { + const editor = await setupEmptyDoc(page); + await openSidebar(page); + await editor.typeText(`Alpha bravo charlie delta echo fox ${CLEAN}.`); + + await addCommentOn(editor, 'bravo', 'on bravo'); + const [id] = await getSidebarCommentIds(page); + expect(id).toBeDefined(); + + // Freshly submitted: cursor sits right after the comment range, expanded. + expect(await isCommentExpanded(page, id)).toBe(true); + + // Move far away → collapses. + await moveCursorTo(editor, CLEAN); + expect(await isCommentExpanded(page, id)).toBe(false); + + // Move back onto the commented word → expands again. + await editor.selectText('bravo'); + await page.keyboard.press('ArrowRight'); + await page.waitForTimeout(200); + expect(await isCommentExpanded(page, id)).toBe(true); + }); + + test('only one card is expanded at a time, driven by cursor position', async ({ page }) => { + const editor = await setupEmptyDoc(page); + await openSidebar(page); + await editor.typeText(`Zero one two three four five six seven ${CLEAN}.`); + + await addCommentOn(editor, 'two', 'second'); + await addCommentOn(editor, 'five', 'fifth'); + + const ids = await getSidebarCommentIds(page); + expect(ids).toHaveLength(2); + const [firstId, secondId] = ids; + + await editor.selectText('two'); + await page.keyboard.press('ArrowRight'); + await page.waitForTimeout(200); + expect(await isCommentExpanded(page, firstId)).toBe(true); + expect(await isCommentExpanded(page, secondId)).toBe(false); + + await editor.selectText('five'); + await page.keyboard.press('ArrowRight'); + await page.waitForTimeout(200); + expect(await isCommentExpanded(page, firstId)).toBe(false); + expect(await isCommentExpanded(page, secondId)).toBe(true); + }); + + test('arrow-key navigation into a comment expands its card', async ({ page }) => { + const editor = await setupEmptyDoc(page); + await openSidebar(page); + await editor.typeText(`prefix commented suffix ${CLEAN}.`); + + await addCommentOn(editor, 'commented', 'kbd nav'); + const [id] = await getSidebarCommentIds(page); + + await moveCursorTo(editor, CLEAN); + expect(await isCommentExpanded(page, id)).toBe(false); + + // Arrow-key navigation: start on "prefix", walk right until cursor lands + // inside "commented". ArrowRight from a collapsed-after-selection point + // advances one PM position at a time. + await editor.selectText('prefix'); + await page.keyboard.press('ArrowRight'); // collapse to end of "prefix" + await page.keyboard.press('ArrowRight'); // past space, into "commented" + await page.keyboard.press('ArrowRight'); + await page.waitForTimeout(200); + expect(await isCommentExpanded(page, id)).toBe(true); + }); +}); + +test.describe('Comments sidebar — resolve / reopen (regression #268)', () => { + test('clicking the resolved checkmark expands the card', async ({ page }) => { + // Direct regression for #268: after Resolve + navigate away, clicking the + // checkmark must re-expand the card. Before the fix, the ResizeObserver on + // the editor container fired onSelectionChange with the unchanged cursor + // right after the click set the expansion, clearing it within one frame. + const editor = await setupEmptyDoc(page); + await openSidebar(page); + await editor.typeText(`Here is a resolvable target word in the doc ${CLEAN}.`); + + await addCommentOn(editor, 'target', 'resolve me'); + const [id] = await getSidebarCommentIds(page); + + await commentCard(page, id).locator('button[title="Resolve"]').click(); + await page.waitForTimeout(150); + + await moveCursorTo(editor, CLEAN); + await page.waitForTimeout(200); + + const marker = resolvedMarker(page, id); + await expect(marker).toBeVisible(); + + await marker.click(); + await page.waitForTimeout(300); + + expect(await isCommentExpanded(page, id)).toBe(true); + await expect(commentCard(page, id).locator('button[title="Reopen"]')).toBeVisible(); + }); + + test('clicking the resolved checkmark expansion survives multiple resize frames', async ({ + page, + }) => { + // Harder version of the regression: make sure the expansion is not just + // set-then-cleared, but stays across multiple render frames worth of + // ResizeObserver/layout effect firings. + const editor = await setupEmptyDoc(page); + await openSidebar(page); + await editor.typeText(`Persistence check with marker word here ${CLEAN}.`); + + await addCommentOn(editor, 'marker', 'persist'); + const [id] = await getSidebarCommentIds(page); + + await commentCard(page, id).locator('button[title="Resolve"]').click(); + await moveCursorTo(editor, CLEAN); + await page.waitForTimeout(200); + + await resolvedMarker(page, id).click(); + + for (let i = 0; i < 5; i++) { + await page.waitForTimeout(100); + expect(await isCommentExpanded(page, id), `still expanded after ${i * 100}ms`).toBe(true); + } + }); + + test('clicking an expanded resolved card collapses it back to the checkmark', async ({ + page, + }) => { + const editor = await setupEmptyDoc(page); + await openSidebar(page); + await editor.typeText(`Toggle me on and off here ${CLEAN}.`); + + await addCommentOn(editor, 'Toggle', 'toggle test'); + const [id] = await getSidebarCommentIds(page); + + await commentCard(page, id).locator('button[title="Resolve"]').click(); + await moveCursorTo(editor, CLEAN); + await page.waitForTimeout(200); + + await resolvedMarker(page, id).click(); + await page.waitForTimeout(200); + expect(await isCommentExpanded(page, id)).toBe(true); + + // CommentCard root has onClick={onToggleExpand}. Click a safe spot (not on + // the action buttons) to flip it closed. + await commentCard(page, id).click({ position: { x: 10, y: 10 } }); + await page.waitForTimeout(200); + + expect(await isCommentExpanded(page, id)).toBe(false); + await expect(resolvedMarker(page, id)).toBeVisible(); + }); + + test('reopen converts a resolved comment back to a normal active comment', async ({ page }) => { + const editor = await setupEmptyDoc(page); + await openSidebar(page); + await editor.typeText(`Resolve then reopen this phrase now ${CLEAN}.`); + + await addCommentOn(editor, 'phrase', 'reopen flow'); + const [id] = await getSidebarCommentIds(page); + + await commentCard(page, id).locator('button[title="Resolve"]').click(); + await moveCursorTo(editor, CLEAN); + await page.waitForTimeout(200); + + await resolvedMarker(page, id).click(); + await page.waitForTimeout(200); + await commentCard(page, id).locator('button[title="Reopen"]').click(); + await page.waitForTimeout(200); + + await expect(resolvedMarker(page, id)).toHaveCount(0); + await expect(commentCard(page, id).locator('button[title="Resolve"]')).toBeVisible(); + }); + + test('resolved comments remain visible as checkmarks while sidebar is open', async ({ page }) => { + const editor = await setupEmptyDoc(page); + await openSidebar(page); + await editor.typeText(`Alpha beta gamma delta epsilon zeta eta theta ${CLEAN}.`); + + await addCommentOn(editor, 'beta', 'one'); + await addCommentOn(editor, 'delta', 'two'); + await addCommentOn(editor, 'zeta', 'three'); + + const ids = await getSidebarCommentIds(page); + expect(ids).toHaveLength(3); + const midId = ids[1]; + + await editor.selectText('delta'); + await page.keyboard.press('ArrowRight'); + await page.waitForTimeout(150); + await commentCard(page, midId).locator('button[title="Resolve"]').click(); + await moveCursorTo(editor, CLEAN); + await page.waitForTimeout(200); + + await expect(resolvedMarker(page, midId)).toBeVisible(); + await expect(commentCard(page, ids[0])).toBeVisible(); + await expect(commentCard(page, ids[2])).toBeVisible(); + }); +}); + +test.describe('Comments sidebar — feedback-loop stability', () => { + test('expanding a card does not immediately re-collapse (state-identity dedup)', async ({ + page, + }) => { + // When the sidebar expands, the editor container resizes → ResizeObserver + // runs updateSelectionOverlay. Without state-identity dedup, the callback + // fired with the unchanged cursor and cleared the expansion (#268). This + // test verifies the card stays expanded across multiple resize frames. + const editor = await setupEmptyDoc(page); + await openSidebar(page); + await editor.typeText(`Stability test text with target word here ${CLEAN}.`); + + await addCommentOn(editor, 'target', 'stable'); + const [id] = await getSidebarCommentIds(page); + + await moveCursorTo(editor, CLEAN); + await page.waitForTimeout(200); + + await editor.selectText('target'); + await page.keyboard.press('ArrowRight'); + + for (let i = 0; i < 5; i++) { + await page.waitForTimeout(100); + expect(await isCommentExpanded(page, id), `still expanded at ${i * 100}ms`).toBe(true); + } + }); + + test('resize while a resolved card is expanded does not collapse it (ResizeObserver feedback)', async ({ + page, + }) => { + // Directly exercises the ResizeObserver path on the paged-editor container. + // Before the state-identity dedup, updateSelectionOverlay fired every time + // the container resized, invoking onSelectionChange with the unchanged + // cursor and clearing the expansion that the marker click had just set. + const editor = await setupEmptyDoc(page); + await openSidebar(page); + await editor.typeText(`Resize loop test with marker here ${CLEAN}.`); + + await addCommentOn(editor, 'marker', 'resize'); + const [id] = await getSidebarCommentIds(page); + + await commentCard(page, id).locator('button[title="Resolve"]').click(); + await moveCursorTo(editor, CLEAN); + await page.waitForTimeout(200); + + await resolvedMarker(page, id).click(); + await page.waitForTimeout(150); + expect(await isCommentExpanded(page, id)).toBe(true); + + // Force several ResizeObserver ticks by shrinking then restoring the + // viewport. Each resize re-runs updateSelectionOverlay with the same + // PM state; without dedup, each one re-invokes onSelectionChange and + // clears the expansion. + const original = page.viewportSize(); + await page.setViewportSize({ width: 900, height: 700 }); + await page.waitForTimeout(100); + await page.setViewportSize({ width: 1280, height: 800 }); + await page.waitForTimeout(100); + if (original) await page.setViewportSize(original); + await page.waitForTimeout(200); + + expect(await isCommentExpanded(page, id)).toBe(true); + }); + + test('clicking a collapsed card in the sidebar expands it without moving the cursor', async ({ + page, + }) => { + const editor = await setupEmptyDoc(page); + await openSidebar(page); + await editor.typeText(`Click the card safely one two three four ${CLEAN}.`); + + await addCommentOn(editor, 'two', 'click target'); + const [id] = await getSidebarCommentIds(page); + + await moveCursorTo(editor, CLEAN); + await page.waitForTimeout(200); + expect(await isCommentExpanded(page, id)).toBe(false); + + // Capture PM cursor position before the sidebar click. + const posBefore = await page.evaluate(() => { + const view = document.querySelector('.paged-editor__hidden-pm .ProseMirror'); + if (!view) return null; + return window.getSelection()?.anchorOffset ?? null; + }); + + await commentCard(page, id).click({ position: { x: 10, y: 10 } }); + await page.waitForTimeout(250); + + expect(await isCommentExpanded(page, id)).toBe(true); + + const posAfter = await page.evaluate(() => { + const view = document.querySelector('.paged-editor__hidden-pm .ProseMirror'); + if (!view) return null; + return window.getSelection()?.anchorOffset ?? null; + }); + // The click should not have moved the PM cursor (onMouseDown on the card + // and sidebar root both stopPropagation to prevent cursor reposition). + expect(posAfter).toEqual(posBefore); + }); +}); diff --git a/packages/react/src/components/sidebar/ResolvedCommentMarker.tsx b/packages/react/src/components/sidebar/ResolvedCommentMarker.tsx index d396d688..921deebe 100644 --- a/packages/react/src/components/sidebar/ResolvedCommentMarker.tsx +++ b/packages/react/src/components/sidebar/ResolvedCommentMarker.tsx @@ -6,10 +6,16 @@ export interface ResolvedCommentMarkerProps extends SidebarItemRenderProps { comment: Comment; } -export function ResolvedCommentMarker({ measureRef, onToggleExpand }: ResolvedCommentMarkerProps) { +export function ResolvedCommentMarker({ + comment, + measureRef, + onToggleExpand, +}: ResolvedCommentMarkerProps) { return (
e.stopPropagation()} style={{ diff --git a/packages/react/src/paged-editor/PagedEditor.tsx b/packages/react/src/paged-editor/PagedEditor.tsx index de65e3ba..e5ed5a9c 100644 --- a/packages/react/src/paged-editor/PagedEditor.tsx +++ b/packages/react/src/paged-editor/PagedEditor.tsx @@ -1613,6 +1613,13 @@ const PagedEditorComponent = forwardRef( const onDocumentChangeRef = useRef(onDocumentChange); const onReadyRef = useRef(onReady); const onRenderedDomContextReadyRef = useRef(onRenderedDomContextReady); + // Last PM state we invoked onSelectionChange for. updateSelectionOverlay + // runs from ResizeObserver / layout / font-load paths too, not only on real + // state changes — firing the callback in those cases caused the sidebar + // expand→resize→re-fire→collapse feedback loop (regression #268). PM states + // are immutable so reference equality is the canonical "nothing changed" + // signal (covers selection, doc, and stored-marks changes alike). + const lastNotifiedStateRef = useRef(null); // Keep refs in sync with latest props onSelectionChangeRef.current = onSelectionChange; @@ -2190,9 +2197,15 @@ const PagedEditorComponent = forwardRef( (state: EditorState) => { const { from, to } = state.selection; - // Always notify selection change (for toolbar sync) even if layout not ready - // Use ref to avoid infinite loops when callback is unstable - onSelectionChangeRef.current?.(from, to); + // Notify consumers only when PM state actually changed. Overlay may + // still need redraw for DOM geometry reasons (resize, layout, font + // load) — that happens below — but the public callback should only + // fire for real selection / doc / stored-marks changes. See + // lastNotifiedStateRef comment; regression #268. + if (lastNotifiedStateRef.current !== state) { + lastNotifiedStateRef.current = state; + onSelectionChangeRef.current?.(from, to); + } // Update visual cell selection highlighting on visible layout table cells if (pagesContainerRef.current) { From 942979f16ccf23cc86d63da27e6e448081b4d106 Mon Sep 17 00:00:00 2001 From: Jedr Blaszyk Date: Thu, 16 Apr 2026 11:09:40 +0200 Subject: [PATCH 2/6] fix: cursor on a resolved comment no longer re-expands it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolved comments in the sidebar should stay collapsed to the checkmark marker unless the user clicks it. Previously, the cursor-mark detector treated resolved and active comments the same — any time the cursor passed through previously-commented text (editing near it, arrow keys, text selection), the old thread re-opened in the sidebar. Skip comments with done=true in the cursor→sidebar sync. Marker click is now the only way to re-open a resolved thread. Co-Authored-By: Claude Opus 4.6 (1M context) --- .changeset/fix-resolved-comment-toggle.md | 2 ++ e2e/tests/comments-sidebar.spec.ts | 34 ++++++++++++++++++++ packages/react/src/components/DocxEditor.tsx | 9 +++++- 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/.changeset/fix-resolved-comment-toggle.md b/.changeset/fix-resolved-comment-toggle.md index da454697..eb367353 100644 --- a/.changeset/fix-resolved-comment-toggle.md +++ b/.changeset/fix-resolved-comment-toggle.md @@ -3,3 +3,5 @@ --- Fix a regression where clicking the checkmark of a resolved comment did not re-open the comment card (issue #268). `PagedEditor.updateSelectionOverlay` fired `onSelectionChange` from every overlay redraw — including ResizeObserver and layout/font callbacks — not only on actual selection changes. When the sidebar card resize (or any window resize) triggered a redraw, the parent received a spurious callback with the unchanged cursor and cleared the just-set expansion. Dedup by PM state identity (immutable references) so consumers are only notified for real selection / doc / stored-marks changes. + +Also: cursor-based sidebar expansion now skips resolved comments. Moving the cursor through previously-commented text no longer re-opens old resolved threads — they stay collapsed to the checkmark marker until the user explicitly clicks it. diff --git a/e2e/tests/comments-sidebar.spec.ts b/e2e/tests/comments-sidebar.spec.ts index 897fa23f..dd8519a5 100644 --- a/e2e/tests/comments-sidebar.spec.ts +++ b/e2e/tests/comments-sidebar.spec.ts @@ -343,6 +343,40 @@ test.describe('Comments sidebar — resolve / reopen (regression #268)', () => { await expect(commentCard(page, ids[0])).toBeVisible(); await expect(commentCard(page, ids[2])).toBeVisible(); }); + + test('cursor landing on a resolved comment range does not auto-expand its card', async ({ + page, + }) => { + // Resolved comments should stay collapsed to the checkmark marker unless + // the user explicitly clicks the marker. Otherwise the sidebar is polluted + // with old threads every time the cursor passes through commented text. + const editor = await setupEmptyDoc(page); + await openSidebar(page); + await editor.typeText(`Before resolved word after ${CLEAN}.`); + + await addCommentOn(editor, 'resolved', 'to be resolved'); + const [id] = await getSidebarCommentIds(page); + + await commentCard(page, id).locator('button[title="Resolve"]').click(); + await moveCursorTo(editor, CLEAN); + await page.waitForTimeout(200); + await expect(resolvedMarker(page, id)).toBeVisible(); + + // Move the cursor onto the still-marked "resolved" word. With an active + // comment this would auto-expand the card; for a resolved comment it must + // stay collapsed as the checkmark. + await editor.selectText('resolved'); + await page.keyboard.press('ArrowRight'); + await page.waitForTimeout(250); + + expect(await isCommentExpanded(page, id)).toBe(false); + await expect(resolvedMarker(page, id)).toBeVisible(); + + // Clicking the marker is still the way to open it. + await resolvedMarker(page, id).click(); + await page.waitForTimeout(200); + expect(await isCommentExpanded(page, id)).toBe(true); + }); }); test.describe('Comments sidebar — feedback-loop stability', () => { diff --git a/packages/react/src/components/DocxEditor.tsx b/packages/react/src/components/DocxEditor.tsx index cd1e5ad5..71697c9a 100644 --- a/packages/react/src/components/DocxEditor.tsx +++ b/packages/react/src/components/DocxEditor.tsx @@ -3980,7 +3980,14 @@ body { background: white; } let cursorSidebarItem: string | null = null; for (const mark of marks) { if (mark.type.name === 'comment' && mark.attrs.commentId != null) { - cursorSidebarItem = `comment-${mark.attrs.commentId}`; + // Skip resolved comments — they stay collapsed as a checkmark + // marker unless the user explicitly clicks the marker. Otherwise + // the sidebar gets polluted with old threads every time the + // cursor passes through commented text. + const commentId = mark.attrs.commentId as number; + const c = comments.find((x) => x.id === commentId); + if (c?.done) continue; + cursorSidebarItem = `comment-${commentId}`; break; } if ( From 539173cd8170d8618c18eea3853dcc805cca24fd Mon Sep 17 00:00:00 2001 From: Jedr Blaszyk Date: Thu, 16 Apr 2026 11:33:22 +0200 Subject: [PATCH 3/6] fix: auto-collapse card on Resolve; hide highlight for collapsed resolved MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clicking Resolve now immediately collapses the card to its checkmark marker. The resolve handler updates React state only (no PM transaction), so the cursor-based collapse path never fired — the card hung around expanded until the user clicked somewhere in the doc. Explicitly clear expandedSidebarItem in the handler. This also cascades into the doc-body highlight: resolvedIdsForRender was already excluding the currently-expanded resolved comment so its range gets highlighted, then including all others so their yellow is hidden. With the card now collapsing on resolve, the highlight correctly disappears at the same instant. Clicking the checkmark re-expands the card AND re-shows the highlight on that comment's range only. 3 new e2e tests: - clicking Resolve immediately collapses the card - resolved comment highlight is hidden in the doc body by default - expanding a resolved comment re-shows the highlight on its range only Co-Authored-By: Claude Opus 4.6 (1M context) --- e2e/tests/comments-sidebar.spec.ts | 92 ++++++++++++++++++++ packages/react/src/components/DocxEditor.tsx | 7 ++ 2 files changed, 99 insertions(+) diff --git a/e2e/tests/comments-sidebar.spec.ts b/e2e/tests/comments-sidebar.spec.ts index dd8519a5..aa2cf048 100644 --- a/e2e/tests/comments-sidebar.spec.ts +++ b/e2e/tests/comments-sidebar.spec.ts @@ -89,6 +89,32 @@ function resolvedMarker(page: Page, commentId: number) { return page.locator(`${SIDEBAR} [data-comment-id="${commentId}"][data-comment-resolved="true"]`); } +/** + * Return the computed background-color of the first doc-body span that + * carries the given commentId, or '' if no such span / no inline style. + * Active comments paint a yellow background; hidden resolved ones fall back + * to transparent / the paragraph default. + */ +async function getCommentHighlight(page: Page, commentId: number): Promise { + return page.evaluate((id) => { + const span = document.querySelector( + `.paged-editor__pages [data-comment-id="${id}"]` + ) as HTMLElement | null; + if (!span) return ''; + return getComputedStyle(span).backgroundColor || ''; + }, commentId); +} + +function isYellowish(rgb: string): boolean { + // 'rgba(255, 212, 0, 0.15)' → keep as-is; covers both the 0.15 default and + // the 0.35 expanded override. Anything with rgb starting high-red, high-green, + // low-blue counts. + const m = rgb.match(/rgba?\(([^)]+)\)/); + if (!m) return false; + const [r, g, b] = m[1].split(',').map((s) => Number(s.trim())); + return r >= 200 && g >= 150 && b < 80; +} + async function getSidebarCommentIds(page: Page): Promise { return page.evaluate(() => { const els = document.querySelectorAll('.docx-unified-sidebar [data-comment-id]'); @@ -377,6 +403,72 @@ test.describe('Comments sidebar — resolve / reopen (regression #268)', () => { await page.waitForTimeout(200); expect(await isCommentExpanded(page, id)).toBe(true); }); + + test('clicking Resolve immediately collapses the card to the checkmark marker', async ({ + page, + }) => { + // Resolving without moving the cursor should not leave the expanded card + // hanging — clicking Resolve is itself the "I'm done with this" action. + const editor = await setupEmptyDoc(page); + await openSidebar(page); + await editor.typeText(`Collapse on resolve target word test ${CLEAN}.`); + + await addCommentOn(editor, 'target', 'to resolve'); + const [id] = await getSidebarCommentIds(page); + expect(await isCommentExpanded(page, id)).toBe(true); + + // Resolve click — no cursor movement afterwards. + await commentCard(page, id).locator('button[title="Resolve"]').click(); + await page.waitForTimeout(250); + + expect(await isCommentExpanded(page, id)).toBe(false); + await expect(resolvedMarker(page, id)).toBeVisible(); + }); + + test('resolved comment highlight is hidden in the doc body by default', async ({ page }) => { + const editor = await setupEmptyDoc(page); + await openSidebar(page); + await editor.typeText(`Hide highlight for target when resolved ${CLEAN}.`); + + await addCommentOn(editor, 'target', 'hide highlight'); + const [id] = await getSidebarCommentIds(page); + + // Active comment: highlight is yellowish. + const activeBg = await getCommentHighlight(page, id); + expect(isYellowish(activeBg), `active bg "${activeBg}" should be yellowish`).toBe(true); + + // Resolve → card collapses → highlight hides. + await commentCard(page, id).locator('button[title="Resolve"]').click(); + await page.waitForTimeout(300); + const resolvedBg = await getCommentHighlight(page, id); + expect(isYellowish(resolvedBg), `resolved bg "${resolvedBg}" should NOT be yellowish`).toBe( + false + ); + }); + + test('expanding a resolved comment re-shows the highlight on its range only', async ({ + page, + }) => { + const editor = await setupEmptyDoc(page); + await openSidebar(page); + await editor.typeText(`Show on expand target word here ${CLEAN}.`); + + await addCommentOn(editor, 'target', 'show on expand'); + const [id] = await getSidebarCommentIds(page); + + await commentCard(page, id).locator('button[title="Resolve"]').click(); + await moveCursorTo(editor, CLEAN); + await page.waitForTimeout(250); + + // Collapsed: highlight hidden. + expect(isYellowish(await getCommentHighlight(page, id))).toBe(false); + + // Expand by clicking the checkmark: highlight should return. + await resolvedMarker(page, id).click(); + await page.waitForTimeout(250); + const expandedBg = await getCommentHighlight(page, id); + expect(isYellowish(expandedBg), `expanded bg "${expandedBg}" should be yellowish`).toBe(true); + }); }); test.describe('Comments sidebar — feedback-loop stability', () => { diff --git a/packages/react/src/components/DocxEditor.tsx b/packages/react/src/components/DocxEditor.tsx index 71697c9a..8e5e44c3 100644 --- a/packages/react/src/components/DocxEditor.tsx +++ b/packages/react/src/components/DocxEditor.tsx @@ -3571,6 +3571,13 @@ body { background: white; } onCommentResolve: (id) => { const target = comments.find((c) => c.id === id); setComments((prev) => prev.map((c) => (c.id === id ? { ...c, done: true } : c))); + // Collapse the card to its checkmark marker immediately. Resolving + // doesn't go through a PM transaction, so the cursor-based collapse + // path wouldn't fire; do it explicitly. Cascades into the highlight + // hide via resolvedIdsForRender. + if (expandedSidebarItem === `comment-${id}`) { + setExpandedSidebarItem(null); + } if (target) onCommentResolve?.({ ...target, done: true }); }, onCommentUnresolve: (id) => { From 311ae59926078d17cc1b5945d3185d0b7cb0ffee Mon Sep 17 00:00:00 2001 From: Jedr Blaszyk Date: Thu, 16 Apr 2026 11:36:20 +0200 Subject: [PATCH 4/6] refactor: reuse resolvedCommentIds Set in cursor-mark detector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Was doing comments.find() per mark per selection change; swap to the already-memoized Set lookup. O(n)→O(1), and the intent reads better. Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/react/src/components/DocxEditor.tsx | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/react/src/components/DocxEditor.tsx b/packages/react/src/components/DocxEditor.tsx index 8e5e44c3..e37fe040 100644 --- a/packages/react/src/components/DocxEditor.tsx +++ b/packages/react/src/components/DocxEditor.tsx @@ -3988,12 +3988,11 @@ body { background: white; } for (const mark of marks) { if (mark.type.name === 'comment' && mark.attrs.commentId != null) { // Skip resolved comments — they stay collapsed as a checkmark - // marker unless the user explicitly clicks the marker. Otherwise - // the sidebar gets polluted with old threads every time the - // cursor passes through commented text. + // marker unless the user explicitly clicks it. Otherwise the + // sidebar fills up with old threads every time the cursor + // passes through commented text. const commentId = mark.attrs.commentId as number; - const c = comments.find((x) => x.id === commentId); - if (c?.done) continue; + if (resolvedCommentIds.has(commentId)) continue; cursorSidebarItem = `comment-${commentId}`; break; } From 6065bed9470c0b398cac2bcc36cfc47b2fef9e69 Mon Sep 17 00:00:00 2001 From: Jedr Blaszyk Date: Thu, 16 Apr 2026 11:47:26 +0200 Subject: [PATCH 5/6] fix: collapse expanded card on grey-gutter click + sidebar toggle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two interaction gaps: 1. Expanded cards only collapsed when the user clicked inside the PM doc body, because that's what moved the cursor and triggered the cursor→ sidebar sync. Clicks in the grey area around the page didn't touch PM state, so the card hung around. Add a mousedown handler on the scroll container: if the target is not inside the pages, sidebar, or margin markers, clear expandedSidebarItem. 2. Toggling the sidebar off and back on preserved expandedSidebarItem across re-render, so a previously-expanded resolved comment came back as a full card instead of the checkmark default. Clear expansion on user-initiated toggle. 2 new e2e tests. Co-Authored-By: Claude Opus 4.6 (1M context) --- e2e/tests/comments-sidebar.spec.ts | 60 ++++++++++++++++++++ packages/react/src/components/DocxEditor.tsx | 27 ++++++++- 2 files changed, 85 insertions(+), 2 deletions(-) diff --git a/e2e/tests/comments-sidebar.spec.ts b/e2e/tests/comments-sidebar.spec.ts index aa2cf048..87c0e845 100644 --- a/e2e/tests/comments-sidebar.spec.ts +++ b/e2e/tests/comments-sidebar.spec.ts @@ -469,6 +469,66 @@ test.describe('Comments sidebar — resolve / reopen (regression #268)', () => { const expandedBg = await getCommentHighlight(page, id); expect(isYellowish(expandedBg), `expanded bg "${expandedBg}" should be yellowish`).toBe(true); }); + + test('clicking the grey gutter outside the page collapses an expanded card', async ({ page }) => { + const editor = await setupEmptyDoc(page); + await openSidebar(page); + await editor.typeText(`Gutter collapse target word here ${CLEAN}.`); + + await addCommentOn(editor, 'target', 'gutter test'); + const [id] = await getSidebarCommentIds(page); + + // Resolve then expand so we have a resolved card open — clicking the gutter + // should collapse it back to the checkmark. + await commentCard(page, id).locator('button[title="Resolve"]').click(); + await moveCursorTo(editor, CLEAN); + await page.waitForTimeout(200); + await resolvedMarker(page, id).click(); + await page.waitForTimeout(200); + expect(await isCommentExpanded(page, id)).toBe(true); + + // Click in the grey area well outside the page and sidebar. The scroll + // container extends left of the page; 20px from the viewport left edge is + // reliably in the gutter. + const pageEl = page.locator('.layout-page').first(); + const pageBox = await pageEl.boundingBox(); + if (!pageBox) throw new Error('page not found'); + await page.mouse.click(Math.max(20, pageBox.x - 40), pageBox.y + 50); + await page.waitForTimeout(250); + + expect(await isCommentExpanded(page, id)).toBe(false); + await expect(resolvedMarker(page, id)).toBeVisible(); + }); + + test('toggling the sidebar off and on resets resolved comments to the checkmark default', async ({ + page, + }) => { + const editor = await setupEmptyDoc(page); + await openSidebar(page); + await editor.typeText(`Sidebar toggle reset target word ${CLEAN}.`); + + await addCommentOn(editor, 'target', 'toggle test'); + const [id] = await getSidebarCommentIds(page); + + // Resolve + expand so a resolved card is open right before toggling. + await commentCard(page, id).locator('button[title="Resolve"]').click(); + await moveCursorTo(editor, CLEAN); + await page.waitForTimeout(200); + await resolvedMarker(page, id).click(); + await page.waitForTimeout(200); + expect(await isCommentExpanded(page, id)).toBe(true); + + // Toggle sidebar off then back on. + const toggle = page.locator('[aria-label="Toggle comments sidebar"]'); + await toggle.click(); + await page.waitForTimeout(150); + await toggle.click(); + await page.waitForTimeout(250); + + // Resolved comment should be back to the checkmark, not the expanded card. + await expect(resolvedMarker(page, id)).toBeVisible(); + expect(await isCommentExpanded(page, id)).toBe(false); + }); }); test.describe('Comments sidebar — feedback-loop stability', () => { diff --git a/packages/react/src/components/DocxEditor.tsx b/packages/react/src/components/DocxEditor.tsx index e37fe040..a5d32b1b 100644 --- a/packages/react/src/components/DocxEditor.tsx +++ b/packages/react/src/components/DocxEditor.tsx @@ -3762,7 +3762,12 @@ body { background: white; } <> setShowCommentsSidebar(!showCommentsSidebar)} + onClick={() => { + // Also reset expansion so reshowing the sidebar lands on the default + // collapsed state — resolved threads stay as checkmarks, not opened. + setShowCommentsSidebar((v) => !v); + setExpandedSidebarItem(null); + }} active={showCommentsSidebar} title="Toggle comments sidebar" ariaLabel="Toggle comments sidebar" @@ -3895,7 +3900,25 @@ body { background: white; } )} {/* Editor container - this is the scroll container (toolbar is above, not inside) */} -
+
{ + // Click in the grey gutter around the page → collapse any + // expanded sidebar card. Clicks on the doc body already + // collapse via the cursor-mark detector; clicks inside the + // sidebar are user interactions with the card itself. + const target = e.target as HTMLElement; + if ( + target.closest('.paged-editor__pages') || + target.closest('.docx-unified-sidebar') || + target.closest('.docx-comment-margin-markers') + ) { + return; + } + setExpandedSidebarItem(null); + }} + > {/* Editor content wrapper */}
{/* Editor content area */} From 957f4faa38d48f9fba71c8f7957093d3272ecfea Mon Sep 17 00:00:00 2001 From: Jedr Blaszyk Date: Thu, 16 Apr 2026 12:08:10 +0200 Subject: [PATCH 6/6] fix: reposition floating add-comment button on resize MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fallout from the state-identity dedup: updateSelectionOverlay no longer re-fires onSelectionChange for overlay-only redraws, so the button's geometry callers stopped getting called on window resize / sidebar toggle / loading→ready. The button's `left` is computed from the page's right edge, and the page re-centers on resize, so the button would strand at the stale coordinate while the page moved underneath it. Extract the position logic into a stable callback and drive it from its own ResizeObserver + window-resize listener + zoom effect. Re-run on state.isLoading changes so the observer gets attached after the loading subtree swaps in (the scroll container doesn't exist on first mount). 2 new e2e tests (regression-verified by stashing the fix): - floating button re-anchors to page right edge after window resize - floating button tracks page right edge across multiple resize steps Co-Authored-By: Claude Opus 4.6 (1M context) --- e2e/tests/comment-button.spec.ts | 80 +++++++++++++++++++- packages/react/src/components/DocxEditor.tsx | 76 +++++++++++++++---- 2 files changed, 139 insertions(+), 17 deletions(-) diff --git a/e2e/tests/comment-button.spec.ts b/e2e/tests/comment-button.spec.ts index 27480797..ca891cdb 100644 --- a/e2e/tests/comment-button.spec.ts +++ b/e2e/tests/comment-button.spec.ts @@ -22,13 +22,29 @@ async function findCommentButton(page: import('@playwright/test').Page) { const style = getComputedStyle(btn); if (style.position === 'absolute' && style.zIndex === '50') { const rect = btn.getBoundingClientRect(); - return { top: rect.top, bottom: rect.bottom, centerY: rect.top + rect.height / 2 }; + return { + top: rect.top, + bottom: rect.bottom, + left: rect.left, + right: rect.right, + centerX: rect.left + rect.width / 2, + centerY: rect.top + rect.height / 2, + }; } } return null; }); } +/** Right edge of the first visible page element, in viewport coordinates. */ +async function getPageRightEdge(page: import('@playwright/test').Page) { + return page.evaluate(() => { + const pageEl = document.querySelector('.layout-page') as HTMLElement | null; + if (!pageEl) return null; + return pageEl.getBoundingClientRect().right; + }); +} + /** * Find the visible span in .paged-editor__pages that contains the given text, * return its bounding box center Y. @@ -128,3 +144,65 @@ test.describe('Comment Button - Scroll Position (#185)', () => { } }); }); + +test.describe('Comment Button - Geometry changes (#268 dedup)', () => { + // Regression guard for the state-identity dedup in + // PagedEditor.updateSelectionOverlay (#268). That fix stopped re-firing + // onSelectionChange on every overlay redraw, so geometry-driven callers + // like the floating comment button now subscribe to resize / zoom + // themselves. These tests verify they still track the selection. + + let editor: EditorPage; + + test.beforeEach(async ({ page }) => { + editor = new EditorPage(page); + await editor.goto(); + await editor.waitForReady(); + await editor.loadDocxFile(DEMO_DOCX_PATH); + await page.waitForSelector('.paged-editor__pages .layout-page', { timeout: 10000 }); + }); + + test('floating button re-anchors to page right edge after window resize', async ({ page }) => { + // The button's left coord is computed from the page's right edge. Before + // the fix, resizing the window (which re-centers the page) left the button + // stranded at the OLD page edge while the page moved. + const selected = await editor.selectText('Demonstration'); + expect(selected).toBe(true); + await page.waitForTimeout(300); + + await page.setViewportSize({ width: 900, height: 700 }); + await page.waitForTimeout(400); + + const btn = await findCommentButton(page); + const pageRight = await getPageRightEdge(page); + expect(btn, 'button should still exist after resize').not.toBeNull(); + expect(pageRight).not.toBeNull(); + + // Button is centered on the page right edge (transform: translate(-50%)). + // It must track within a few px of the current page right edge. + expect( + Math.abs(btn!.centerX - pageRight!), + `button centerX=${btn!.centerX} drifted from pageRight=${pageRight}` + ).toBeLessThan(20); + }); + + test('floating button tracks page right edge across multiple resize steps', async ({ page }) => { + const selected = await editor.selectText('Demonstration'); + expect(selected).toBe(true); + await page.waitForTimeout(300); + + for (const width of [1024, 800, 1400, 1100]) { + await page.setViewportSize({ width, height: 800 }); + await page.waitForTimeout(300); + + const btn = await findCommentButton(page); + const pageRight = await getPageRightEdge(page); + expect(btn, `button missing at width ${width}`).not.toBeNull(); + expect(pageRight, `page right missing at width ${width}`).not.toBeNull(); + expect( + Math.abs(btn!.centerX - pageRight!), + `button drifted from page edge at width ${width}: ${btn!.centerX} vs ${pageRight}` + ).toBeLessThan(20); + } + }); +}); diff --git a/packages/react/src/components/DocxEditor.tsx b/packages/react/src/components/DocxEditor.tsx index a5d32b1b..502005da 100644 --- a/packages/react/src/components/DocxEditor.tsx +++ b/packages/react/src/components/DocxEditor.tsx @@ -935,6 +935,8 @@ export const DocxEditor = forwardRef(function Do }; // 'viewing' mode acts as read-only const readOnly = readOnlyProp || editingMode === 'viewing'; + // Accessed by the stable recomputeFloatingCommentBtn callback below. + // Kept in sync below after that callback is declared. // Floating "add comment" button position (relative to scroll container, null = hidden) const [floatingCommentBtn, setFloatingCommentBtn] = useState<{ top: number; @@ -1389,6 +1391,63 @@ export const DocxEditor = forwardRef(function Do [onChange, pushDocument, cleanOrphanedComments] ); + // Recompute the floating "add comment" button position from the current PM + // selection + page/container geometry. Called from handleSelectionChange and + // from the geometry-change effects below (resize, zoom), because PagedEditor's + // onSelectionChange no longer fires on mere overlay redraws after the + // state-identity dedup in #268. + const readOnlyForFloatingBtnRef = useRef(false); + const recomputeFloatingCommentBtn = useCallback(() => { + const view = pagedEditorRef.current?.getView(); + if (!view) return; + if (isAddingCommentRef.current || readOnlyForFloatingBtnRef.current) { + setFloatingCommentBtn(null); + return; + } + const { from, to } = view.state.selection; + if (from === to) { + setFloatingCommentBtn(null); + return; + } + const container = scrollContainerRef.current; + const parentEl = editorContentRef.current; + if (!container || !parentEl) return; + const top = findSelectionYPosition(container, parentEl, from); + if (top == null) return; + const pagesEl = container.querySelector('.paged-editor__pages'); + const pageEl = pagesEl?.querySelector('.layout-page') as HTMLElement | null; + const left = pageEl + ? pageEl.getBoundingClientRect().right - parentEl.getBoundingClientRect().left + : parentEl.getBoundingClientRect().width / 2 + 408; + setFloatingCommentBtn({ top, left }); + }, []); + // Keep the readOnly ref used by recomputeFloatingCommentBtn in sync + readOnlyForFloatingBtnRef.current = readOnly; + + // Reposition the floating "add comment" button when the editor container + // resizes (window resize, sidebar toggle, loading→ready transition) or when + // zoom changes. Both move the page edges without changing PM selection, so + // the onSelectionChange path no longer covers them after the dedup fix in + // #268. The scroll container may not be mounted on the first render (loading + // state renders a different subtree), so re-run the effect whenever that + // state flips — that's the point at which the container first becomes + // available. + useEffect(() => { + const container = scrollContainerRef.current; + if (!container) return; + const ro = new ResizeObserver(() => recomputeFloatingCommentBtn()); + ro.observe(container); + const onWinResize = () => recomputeFloatingCommentBtn(); + window.addEventListener('resize', onWinResize); + return () => { + ro.disconnect(); + window.removeEventListener('resize', onWinResize); + }; + }, [state.isLoading, recomputeFloatingCommentBtn]); + useEffect(() => { + recomputeFloatingCommentBtn(); + }, [state.zoom, recomputeFloatingCommentBtn]); + // Handle selection changes from ProseMirror const handleSelectionChange = useCallback( (selectionState: SelectionState | null) => { @@ -1526,22 +1585,7 @@ export const DocxEditor = forwardRef(function Do })); // Update floating comment button position - if (view && selectionState.hasSelection && !isAddingComment && !readOnly) { - const container = scrollContainerRef.current; - const parentEl = editorContentRef.current; - const { from: selFrom } = view.state.selection; - const top = findSelectionYPosition(container, parentEl, selFrom); - if (top != null && container && parentEl) { - const pagesEl = container.querySelector('.paged-editor__pages'); - const pageEl = pagesEl?.querySelector('.layout-page') as HTMLElement | null; - const left = pageEl - ? pageEl.getBoundingClientRect().right - parentEl.getBoundingClientRect().left - : parentEl.getBoundingClientRect().width / 2 + 408; - setFloatingCommentBtn({ top, left }); - } - } else { - setFloatingCommentBtn(null); - } + recomputeFloatingCommentBtn(); // Notify parent onSelectionChange?.(selectionState);