Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/fix-resolved-comment-toggle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@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.

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.
35 changes: 22 additions & 13 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).

Expand Down
80 changes: 79 additions & 1 deletion e2e/tests/comment-button.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
}
});
});
Loading