Skip to content

Commit 5176866

Browse files
alicodingclaude
andauthored
fix: the note toolbar floats above the full-size overlay, and pressing it keeps the edit alive (#532)
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. Claude-Session: https://claude.ai/code/session_012im1JxQQV2ahnXzZDdVmZq Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent ea7a7a0 commit 5176866

3 files changed

Lines changed: 76 additions & 4 deletions

File tree

frontend/e2e/atlas-note-formatting.spec.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,72 @@ test('selecting note text shows the floating toolbar outside the text, and Bold
6565
await deleteSticky(page, note)
6666
})
6767

68+
// Regression (goal 0253 follow-up): in the full-size note overlay the
69+
// toolbar rendered UNDER the Dialog -- its body-level element sat at a
70+
// z-index below the Primer portal root's, so it existed with
71+
// data-show=true yet the dialog surface painted over it and swallowed
72+
// its clicks. Pinned by hit-testing the toolbar's own center, not just
73+
// visibility (Playwright "visible" was true throughout the bug). Also
74+
// pinned: a toolbar press must not END the overlay's edit session --
75+
// MarkdownNoteField's outside-press commit excludes the body-level
76+
// toolbar the same way the sticky's does.
77+
test('the full-size note overlay shows the toolbar above the dialog, and Bold applies without ending the edit', async ({ page }) => {
78+
await page.goto('/')
79+
await page.getByRole('link', { name: 'Atlas' }).click()
80+
const board = page.getByTestId('atlas-board')
81+
await expect(board).toBeVisible()
82+
83+
await placeNoteClear(page, board)
84+
await expect(stickyEditor(page)).toBeVisible()
85+
await fillSticky(page, 'Overlay formatting target')
86+
await blurSticky(page)
87+
const note = page.getByTestId('atlas-sticky-note').filter({ hasText: 'Overlay formatting target' })
88+
await expect(note).toBeVisible()
89+
90+
await note.click({ modifiers: ['Meta'] })
91+
const overlay = page.locator('[data-component="atlas-note-overlay"]')
92+
await expect(overlay).toBeVisible()
93+
await page.getByTestId('atlas-note-overlay-editor-rendered').click()
94+
const editor = page.getByTestId('atlas-note-overlay-editor')
95+
await expect(editor).toBeVisible()
96+
// The engine mounts async (lazy chunk): the field's own focus hop
97+
// can fire before the contenteditable exists, so acquire focus the
98+
// user's way -- click the text itself -- before selecting.
99+
const editable = editor.locator('[contenteditable="true"]')
100+
await editable.click()
101+
await expect
102+
.poll(() => page.evaluate(() => document.activeElement?.getAttribute('contenteditable') === 'true'))
103+
.toBe(true)
104+
await page.keyboard.press('ControlOrMeta+a')
105+
106+
const toolbar = page.getByTestId('milkdown-selection-toolbar')
107+
await expect(toolbar).toBeVisible()
108+
// The regression's own observable: the topmost element at the
109+
// toolbar's center must be the toolbar itself, not the dialog.
110+
await expect
111+
.poll(() => page.evaluate(() => {
112+
const tb = document.querySelector('[data-testid="milkdown-selection-toolbar"]')
113+
if (!tb) return 'no-toolbar'
114+
const r = tb.getBoundingClientRect()
115+
const under = document.elementFromPoint(r.x + r.width / 2, r.y + r.height / 2)
116+
return under && tb.contains(under) ? 'toolbar-on-top' : 'buried'
117+
}))
118+
.toBe('toolbar-on-top')
119+
120+
// Bold from the toolbar: applies, and the edit session SURVIVES the
121+
// press (the editor stays mounted instead of committing closed).
122+
await page.getByTestId('milkdown-toolbar-bold').click()
123+
await expect(page.getByTestId('milkdown-toolbar-bold')).toHaveAttribute('aria-pressed', 'true')
124+
await expect(editor).toBeVisible()
125+
await expect(editor.locator('strong')).toHaveText('Overlay formatting target')
126+
127+
await page.keyboard.press('Escape')
128+
await expect(overlay).not.toBeVisible()
129+
await expect(note.locator('strong')).toHaveText('Overlay formatting target')
130+
131+
await deleteSticky(page, note)
132+
})
133+
68134
// Regression (goal 0254): typing `[x] `/`[ ] `/`[] ` at the START of
69135
// a plain note line creates a to-do -- the engine's own task rule
70136
// only fires inside an existing list item, so the converged

frontend/src/atlas/MarkdownNoteField.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@ export function MarkdownNoteField({ value, onChange, onCommit, placeholder, aria
5353
if (!editing) return
5454
const handlePointerDown = (e: PointerEvent) => {
5555
if (editorWrapRef.current?.contains(e.target as Node | null)) return
56+
// The floating selection toolbar lives at body level, outside
57+
// this wrap -- but a press on it is part of THIS edit session,
58+
// never an outside press (same exclusion as AtlasStickyNode's).
59+
if ((e.target as Element | null)?.closest?.('[data-milkdown-selection-toolbar]')) return
5660
commitRef.current()
5761
}
5862
const handleWindowBlur = () => {

frontend/src/shared/MilkdownEditor.module.css

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,14 +123,16 @@
123123
floats at UI scale beside the selection -- never inside the note
124124
node, never clipped by its box, never shrunk by the board's zoom
125125
transform. TooltipProvider writes left/top and toggles data-show;
126-
this rule owns everything else. z-index sits above the board's own
127-
node tiers (inline, small integers) and below nothing it needs to
128-
defer to -- the toolbar only exists while its editor has focus. */
126+
this rule owns everything else. z-index must clear the Primer
127+
portal root's 1200 (app/index.css) -- a note opened full-size edits
128+
inside a Dialog, and anything lower buries the toolbar under the
129+
dialog surface. Topmost is safe: the toolbar only exists while its
130+
editor has focus, and it defers to nothing. */
129131
.selectionToolbar {
130132
position: absolute;
131133
top: 0;
132134
left: 0;
133-
z-index: 100;
135+
z-index: 1300;
134136
display: flex;
135137
gap: 2px;
136138
padding: 2px;

0 commit comments

Comments
 (0)