From 3799f962bea2f24897c7e1facbaa151a725225ce Mon Sep 17 00:00:00 2001 From: Ali Al Dallal Date: Sun, 30 Aug 2026 19:56:55 -0400 Subject: [PATCH] fix: the note toolbar floats above the full-size overlay, and pressing it keeps the edit alive The selection toolbar's body-level element sat at z-index 100 while Primer's portal root (every Dialog) sits at 1200 -- in the full-size note overlay the toolbar existed with data-show=true yet the dialog painted over it and swallowed its clicks. Raised above the portal root; topmost is safe since the toolbar only exists while its editor has focus. Second latent defect behind it: MarkdownNoteField's outside-press commit lacked the [data-milkdown-selection-toolbar] exclusion AtlasStickyNode already carries, so once visible, a toolbar press would have committed the overlay edit closed before the format applied. Same exclusion added. Regression e2e hit-tests the toolbar's center (Playwright visibility was true throughout the bug) and asserts Bold applies with the editor still mounted. Follow-up to goal 0253. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012im1JxQQV2ahnXzZDdVmZq --- frontend/e2e/atlas-note-formatting.spec.ts | 66 +++++++++++++++++++ frontend/src/atlas/MarkdownNoteField.tsx | 4 ++ frontend/src/shared/MilkdownEditor.module.css | 10 +-- 3 files changed, 76 insertions(+), 4 deletions(-) diff --git a/frontend/e2e/atlas-note-formatting.spec.ts b/frontend/e2e/atlas-note-formatting.spec.ts index 1617f9d9..b8abf206 100644 --- a/frontend/e2e/atlas-note-formatting.spec.ts +++ b/frontend/e2e/atlas-note-formatting.spec.ts @@ -65,6 +65,72 @@ test('selecting note text shows the floating toolbar outside the text, and Bold await deleteSticky(page, note) }) +// Regression (goal 0253 follow-up): in the full-size note overlay the +// toolbar rendered UNDER the Dialog -- its body-level element sat at a +// z-index below the Primer portal root's, so it existed with +// data-show=true yet the dialog surface painted over it and swallowed +// its clicks. Pinned by hit-testing the toolbar's own center, not just +// visibility (Playwright "visible" was true throughout the bug). Also +// pinned: a toolbar press must not END the overlay's edit session -- +// MarkdownNoteField's outside-press commit excludes the body-level +// toolbar the same way the sticky's does. +test('the full-size note overlay shows the toolbar above the dialog, and Bold applies without ending the edit', async ({ page }) => { + await page.goto('/') + await page.getByRole('link', { name: 'Atlas' }).click() + const board = page.getByTestId('atlas-board') + await expect(board).toBeVisible() + + await placeNoteClear(page, board) + await expect(stickyEditor(page)).toBeVisible() + await fillSticky(page, 'Overlay formatting target') + await blurSticky(page) + const note = page.getByTestId('atlas-sticky-note').filter({ hasText: 'Overlay formatting target' }) + await expect(note).toBeVisible() + + await note.click({ modifiers: ['Meta'] }) + const overlay = page.locator('[data-component="atlas-note-overlay"]') + await expect(overlay).toBeVisible() + await page.getByTestId('atlas-note-overlay-editor-rendered').click() + const editor = page.getByTestId('atlas-note-overlay-editor') + await expect(editor).toBeVisible() + // The engine mounts async (lazy chunk): the field's own focus hop + // can fire before the contenteditable exists, so acquire focus the + // user's way -- click the text itself -- before selecting. + const editable = editor.locator('[contenteditable="true"]') + await editable.click() + await expect + .poll(() => page.evaluate(() => document.activeElement?.getAttribute('contenteditable') === 'true')) + .toBe(true) + await page.keyboard.press('ControlOrMeta+a') + + const toolbar = page.getByTestId('milkdown-selection-toolbar') + await expect(toolbar).toBeVisible() + // The regression's own observable: the topmost element at the + // toolbar's center must be the toolbar itself, not the dialog. + await expect + .poll(() => page.evaluate(() => { + const tb = document.querySelector('[data-testid="milkdown-selection-toolbar"]') + if (!tb) return 'no-toolbar' + const r = tb.getBoundingClientRect() + const under = document.elementFromPoint(r.x + r.width / 2, r.y + r.height / 2) + return under && tb.contains(under) ? 'toolbar-on-top' : 'buried' + })) + .toBe('toolbar-on-top') + + // Bold from the toolbar: applies, and the edit session SURVIVES the + // press (the editor stays mounted instead of committing closed). + await page.getByTestId('milkdown-toolbar-bold').click() + await expect(page.getByTestId('milkdown-toolbar-bold')).toHaveAttribute('aria-pressed', 'true') + await expect(editor).toBeVisible() + await expect(editor.locator('strong')).toHaveText('Overlay formatting target') + + await page.keyboard.press('Escape') + await expect(overlay).not.toBeVisible() + await expect(note.locator('strong')).toHaveText('Overlay formatting target') + + await deleteSticky(page, note) +}) + // Regression (goal 0254): typing `[x] `/`[ ] `/`[] ` at the START of // a plain note line creates a to-do -- the engine's own task rule // only fires inside an existing list item, so the converged diff --git a/frontend/src/atlas/MarkdownNoteField.tsx b/frontend/src/atlas/MarkdownNoteField.tsx index 55810e3d..38227bcb 100644 --- a/frontend/src/atlas/MarkdownNoteField.tsx +++ b/frontend/src/atlas/MarkdownNoteField.tsx @@ -53,6 +53,10 @@ export function MarkdownNoteField({ value, onChange, onCommit, placeholder, aria if (!editing) return const handlePointerDown = (e: PointerEvent) => { if (editorWrapRef.current?.contains(e.target as Node | null)) return + // The floating selection toolbar lives at body level, outside + // this wrap -- but a press on it is part of THIS edit session, + // never an outside press (same exclusion as AtlasStickyNode's). + if ((e.target as Element | null)?.closest?.('[data-milkdown-selection-toolbar]')) return commitRef.current() } const handleWindowBlur = () => { diff --git a/frontend/src/shared/MilkdownEditor.module.css b/frontend/src/shared/MilkdownEditor.module.css index cbe3566d..afbabc5e 100644 --- a/frontend/src/shared/MilkdownEditor.module.css +++ b/frontend/src/shared/MilkdownEditor.module.css @@ -123,14 +123,16 @@ floats at UI scale beside the selection -- never inside the note node, never clipped by its box, never shrunk by the board's zoom transform. TooltipProvider writes left/top and toggles data-show; - this rule owns everything else. z-index sits above the board's own - node tiers (inline, small integers) and below nothing it needs to - defer to -- the toolbar only exists while its editor has focus. */ + this rule owns everything else. z-index must clear the Primer + portal root's 1200 (app/index.css) -- a note opened full-size edits + inside a Dialog, and anything lower buries the toolbar under the + dialog surface. Topmost is safe: the toolbar only exists while its + editor has focus, and it defers to nothing. */ .selectionToolbar { position: absolute; top: 0; left: 0; - z-index: 100; + z-index: 1300; display: flex; gap: 2px; padding: 2px;