Skip to content

Commit 37a79fb

Browse files
alicodingclaude
andcommitted
fix: the note's formatting toolbar floats beside the selection, never clipped inside the note (goal 0253)
Crepe's own toolbar mounted through TooltipProvider's default parent -- inside the note node -- where the node's box clipped it to nothing, the board zoom shrank it, and floating-ui misplaced it. Crepe's config exposes no mount point, so its toolbar is off and Mill registers its own through the same kit primitives (tooltipFactory + TooltipProvider) with the provider's documented root: document.body -- the toolbar now floats at UI scale beside the selection with Primer buttons (bold/italic/strikethrough/code) dispatching the kit's own commands. The sticky's outside-press commit excludes the body-level toolbar, so pressing Bold formats instead of ending the edit session. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012im1JxQQV2ahnXzZDdVmZq
1 parent a38f06e commit 37a79fb

6 files changed

Lines changed: 283 additions & 1 deletion

File tree

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,3 +384,56 @@ test('a note holding markdown renders real elements, keeps the edit surface insi
384384

385385
await deleteSticky(page, stickyAfterReload)
386386
})
387+
388+
// Regression (goal 0253): the formatting toolbar used to mount INSIDE
389+
// the note node (Crepe's default TooltipProvider parent), where the
390+
// node's box clipped it to nothing -- reproduced live as "element in
391+
// the DOM, zero bounding box". It now floats at body level through the
392+
// same provider's own `root` option: a real on-screen box, outside the
393+
// selected text it acts on, and its buttons apply without ending the
394+
// edit session (the sticky's outside-press commit excludes it).
395+
test('selecting note text shows the floating toolbar outside the text, and Bold round-trips into the committed note', async ({ page }) => {
396+
await page.goto('/')
397+
await page.getByRole('link', { name: 'Atlas' }).click()
398+
const board = page.getByTestId('atlas-board')
399+
await expect(board).toBeVisible()
400+
401+
await placeNoteClear(page, board)
402+
await expect(stickyEditor(page)).toBeVisible()
403+
await fillSticky(page, 'Quarterly review meeting notes')
404+
await page.keyboard.press('Meta+a')
405+
406+
const toolbar = page.getByTestId('milkdown-selection-toolbar')
407+
await expect(toolbar).toBeVisible()
408+
const toolbarBox = await toolbar.boundingBox()
409+
if (!toolbarBox) throw new Error('toolbar has no bounding box')
410+
expect(toolbarBox.width).toBeGreaterThan(40)
411+
412+
// Never covering the text it acts on: the toolbar's box must not
413+
// intersect the selected text's own rect (floating-ui places it
414+
// above/below the selection with an offset).
415+
const selRect = await page.evaluate(() => {
416+
const range = window.getSelection()?.getRangeAt(0)
417+
const r = range?.getBoundingClientRect()
418+
return r ? { x: r.x, y: r.y, width: r.width, height: r.height } : null
419+
})
420+
if (!selRect) throw new Error('no live selection rect')
421+
const overlaps = !(
422+
toolbarBox.x + toolbarBox.width <= selRect.x ||
423+
selRect.x + selRect.width <= toolbarBox.x ||
424+
toolbarBox.y + toolbarBox.height <= selRect.y ||
425+
selRect.y + selRect.height <= toolbarBox.y
426+
)
427+
expect(overlaps, 'the toolbar must not cover the selected text').toBe(false)
428+
429+
// Bold applies from the toolbar WITHOUT ending the edit session,
430+
// and survives the commit: the resting note renders a real <strong>.
431+
await page.getByTestId('milkdown-toolbar-bold').click()
432+
await expect(page.getByTestId('milkdown-toolbar-bold')).toHaveAttribute('aria-pressed', 'true')
433+
await blurSticky(page)
434+
const note = page.getByTestId('atlas-sticky-note').filter({ hasText: 'Quarterly review meeting notes' })
435+
await expect(note.locator('strong')).toHaveText('Quarterly review meeting notes')
436+
await expect(toolbar).not.toBeVisible()
437+
438+
await deleteSticky(page, note)
439+
})

frontend/src/atlas/AtlasStickyNode.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,11 @@ export const AtlasStickyNode = memo(function AtlasStickyNode({ data, selected }:
149149
const target = e.target as Element | null
150150
if (wrapRef.current?.contains(target)) return
151151
if (target?.closest('.react-flow__resize-control')) return
152+
// The floating selection toolbar (goal 0253) lives at body
153+
// level -- outside wrapRef by design, so board zoom/clipping
154+
// can't touch it -- but a press on it is part of THIS edit
155+
// session, never an outside press.
156+
if (target?.closest('[data-milkdown-selection-toolbar]')) return
152157
commitRef.current()
153158
}
154159
const handleWindowBlur = () => {

frontend/src/locales/en/common.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,5 +59,11 @@
5959
"deleteRow": "Delete row",
6060
"booleanTrue": "Yes",
6161
"booleanFalse": "No"
62+
},
63+
"formattingToolbar": {
64+
"bold": "Bold",
65+
"italic": "Italic",
66+
"strikethrough": "Strikethrough",
67+
"code": "Code"
6268
}
6369
}

frontend/src/shared/MilkdownEditor.module.css

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,3 +117,29 @@
117117
outline: none;
118118
border-color: var(--borderColor-accent-emphasis);
119119
}
120+
121+
/* The floating selection toolbar (goal 0253): appended to
122+
document.body by the kit's own TooltipProvider (root option), so it
123+
floats at UI scale beside the selection -- never inside the note
124+
node, never clipped by its box, never shrunk by the board's zoom
125+
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. */
129+
.selectionToolbar {
130+
position: absolute;
131+
top: 0;
132+
left: 0;
133+
z-index: 100;
134+
display: flex;
135+
gap: 2px;
136+
padding: 2px;
137+
background: var(--overlay-bgColor, var(--bgColor-default));
138+
border: 1px solid var(--borderColor-default);
139+
border-radius: var(--borderRadius-medium);
140+
box-shadow: var(--shadow-floating-small, var(--shadow-resting-medium));
141+
}
142+
143+
.selectionToolbar[data-show='false'] {
144+
display: none;
145+
}

frontend/src/shared/MilkdownEditor.tsx

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
11
import { useEffect, useRef, useState } from 'react'
2+
import { createPortal } from 'react-dom'
3+
import { useTranslation } from 'react-i18next'
4+
import { IconButton } from '@primer/react'
5+
import { BoldIcon, CodeIcon, ItalicIcon, StrikethroughIcon } from '@primer/octicons-react'
6+
import type { SelectionToolbarAction, SelectionToolbarHandle, SelectionToolbarState } from './milkdownCore'
27
import styles from './MilkdownEditor.module.css'
38

49
export interface MilkdownEditorProps {
@@ -48,6 +53,11 @@ export function MilkdownEditor({ value, onChange, ariaLabel, placeholder, testId
4853
const containerRef = useRef<HTMLDivElement | null>(null)
4954
const editable = onChange !== undefined
5055
const [core, setCore] = useState<CoreModule | null>(null)
56+
// The floating selection toolbar's live handle (goal 0253) -- set
57+
// once the engine mounts an EDITABLE instance, null otherwise; the
58+
// React buttons render into its body-level element via portal.
59+
const [toolbar, setToolbar] = useState<SelectionToolbarHandle | null>(null)
60+
const [toolbarState, setToolbarState] = useState<SelectionToolbarState>({ bold: false, italic: false, strikethrough: false, code: false })
5161
// The doc's own draft, updated on every keystroke (markdownUpdated)
5262
// and read directly by the caller's own commit -- never round-
5363
// tripped through React state first (testing.md). Refreshed via an
@@ -88,6 +98,15 @@ export function MilkdownEditor({ value, onChange, ariaLabel, placeholder, testId
8898
// own $remark extension point, applied here rather than baked into
8999
// NOTE_FEATURES since it isn't a Crepe feature flag.
90100
crepe.editor.use(disableIndentedCodeBlock)
101+
// The floating selection toolbar (goal 0253): editable mounts
102+
// only -- a readonly display has no selection to format. Wired
103+
// before create(), like every plugin registration.
104+
let toolbarHandle: SelectionToolbarHandle | null = null
105+
if (editable) {
106+
toolbarHandle = core.attachSelectionToolbar(crepe, styles.selectionToolbar)
107+
toolbarHandle.onState(setToolbarState)
108+
setToolbar(toolbarHandle)
109+
}
91110
crepe.on((listener) => {
92111
listener.markdownUpdated((_ctx, markdown) => {
93112
onChangeRef.current?.(markdown)
@@ -115,7 +134,11 @@ export function MilkdownEditor({ value, onChange, ariaLabel, placeholder, testId
115134
return () => {
116135
destroyed = true
117136
onReadyRef.current?.(undefined)
118-
void ready.finally(() => crepe.destroy())
137+
setToolbar(null)
138+
void ready.finally(() => {
139+
toolbarHandle?.destroy()
140+
return crepe.destroy()
141+
})
119142
}
120143
// value/ariaLabel/placeholder deliberately excluded: every caller
121144
// remounts this component fresh for a new edit session or a new
@@ -141,6 +164,39 @@ export function MilkdownEditor({ value, onChange, ariaLabel, placeholder, testId
141164
) : (
142165
<div ref={containerRef} className={styles.mount} />
143166
)}
167+
{toolbar && createPortal(<SelectionToolbarButtons toolbar={toolbar} state={toolbarState} />, toolbar.contentEl)}
144168
</div>
145169
)
146170
}
171+
172+
// The floating toolbar's buttons -- portaled into the body-level
173+
// element the kit's TooltipProvider positions (goal 0253). Pointer-
174+
// down is prevented so pressing a button never steals the editor's
175+
// focus/selection out from under the very command it dispatches (the
176+
// selection-toolbar convention Crepe's own buttons also follow).
177+
function SelectionToolbarButtons({ toolbar, state }: { toolbar: SelectionToolbarHandle; state: SelectionToolbarState }) {
178+
const { t } = useTranslation('common')
179+
const items: { action: SelectionToolbarAction; icon: typeof BoldIcon; label: string }[] = [
180+
{ action: 'bold', icon: BoldIcon, label: t('formattingToolbar.bold') },
181+
{ action: 'italic', icon: ItalicIcon, label: t('formattingToolbar.italic') },
182+
{ action: 'strikethrough', icon: StrikethroughIcon, label: t('formattingToolbar.strikethrough') },
183+
{ action: 'code', icon: CodeIcon, label: t('formattingToolbar.code') },
184+
]
185+
return (
186+
<>
187+
{items.map(({ action, icon, label }) => (
188+
<IconButton
189+
key={action}
190+
icon={icon}
191+
size="small"
192+
variant="invisible"
193+
aria-label={label}
194+
aria-pressed={state[action]}
195+
data-testid={`milkdown-toolbar-${action}`}
196+
onMouseDown={(e) => e.preventDefault()}
197+
onClick={() => toolbar.run(action)}
198+
/>
199+
))}
200+
</>
201+
)
202+
}

frontend/src/shared/milkdownCore.ts

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,19 @@ import { Crepe } from '@milkdown/crepe'
88
// any literal color.
99
import '@milkdown/crepe/theme/common/style.css'
1010
import { $remark } from '@milkdown/utils'
11+
import { tooltipFactory, TooltipProvider } from '@milkdown/kit/plugin/tooltip'
12+
import { commandsCtx } from '@milkdown/kit/core'
13+
import {
14+
isMarkSelectedCommand,
15+
toggleEmphasisCommand,
16+
toggleInlineCodeCommand,
17+
toggleStrongCommand,
18+
strongSchema,
19+
emphasisSchema,
20+
inlineCodeSchema,
21+
} from '@milkdown/kit/preset/commonmark'
22+
import { toggleStrikethroughCommand, strikethroughSchema } from '@milkdown/kit/preset/gfm'
23+
import type { EditorView } from '@milkdown/kit/prose/view'
1124

1225
// The Milkdown machinery behind shared/MilkdownEditor.tsx -- reachable
1326
// only through that component's dynamic import (goal 0244 S3), so the
@@ -30,6 +43,10 @@ export const NOTE_FEATURES = {
3043
[Crepe.Feature.Latex]: false,
3144
[Crepe.Feature.BlockEdit]: false,
3245
[Crepe.Feature.CodeMirror]: false,
46+
// Crepe's own selection toolbar is replaced by Mill's floating one
47+
// (goal 0253, attachSelectionToolbar below) -- Crepe mounts its
48+
// toolbar inside the editor node, where a canvas note clips it.
49+
[Crepe.Feature.Toolbar]: false,
3350
}
3451

3552
// Disables CommonMark's INDENTED code block construct (a bare line
@@ -53,3 +70,122 @@ export const disableIndentedCodeBlock = $remark('disableIndentedCodeBlock', () =
5370
extensions.push({ disable: { null: ['codeIndented'] } })
5471
}
5572
})
73+
74+
// --- The selection toolbar (goal 0253) ---
75+
//
76+
// Crepe's own toolbar feature is OFF (NOTE_FEATURES below): it mounts
77+
// its content through TooltipProvider's default parent -- inside the
78+
// editor's own node -- where a canvas note clips it, the board's zoom
79+
// transform shrinks it, and floating-ui misplaces it (absolute
80+
// positioning inside a transformed ancestor). Its config exposes no
81+
// mount point, so Mill registers its OWN selection toolbar through the
82+
// same kit primitives, with the provider's documented `root:
83+
// document.body` -- body is untransformed, so the toolbar floats at UI
84+
// scale beside the selection regardless of board zoom, and nothing
85+
// ever clips it. The toolbar CONTENT stays framework-owned: the host
86+
// (MilkdownEditor.tsx) portals Mill's React buttons into `contentEl`;
87+
// this module only owns registration, positioning, active-mark state,
88+
// and the command calls.
89+
90+
export type SelectionToolbarAction = 'bold' | 'italic' | 'strikethrough' | 'code'
91+
92+
export interface SelectionToolbarState {
93+
bold: boolean
94+
italic: boolean
95+
strikethrough: boolean
96+
code: boolean
97+
}
98+
99+
export interface SelectionToolbarHandle {
100+
// The floating element Mill's React toolbar portals into. Appended
101+
// to document.body by the provider on first update; removed on
102+
// destroy.
103+
contentEl: HTMLElement
104+
run: (action: SelectionToolbarAction) => void
105+
onState: (cb: (state: SelectionToolbarState) => void) => void
106+
destroy: () => void
107+
}
108+
109+
const millSelectionToolbar = tooltipFactory('MILL_SELECTION_TOOLBAR')
110+
111+
// attachSelectionToolbar wires the tooltip plugin onto a Crepe editor.
112+
// Must run BEFORE crepe.create() (plugins register at create); the
113+
// returned handle stays valid for the editor's whole life.
114+
export function attachSelectionToolbar(crepe: Crepe, className: string): SelectionToolbarHandle {
115+
const contentEl = document.createElement('div')
116+
contentEl.className = className
117+
contentEl.dataset.testid = 'milkdown-selection-toolbar'
118+
// The marker outside-press commit listeners key on (goal 0253): the
119+
// toolbar floats at body level, OUTSIDE any editor wrapper, yet a
120+
// press on it is part of the edit session -- hosts exclude
121+
// [data-milkdown-selection-toolbar] from their own commit-on-
122+
// outside-press logic (AtlasStickyNode's document listener).
123+
contentEl.setAttribute('data-milkdown-selection-toolbar', '')
124+
let stateCb: ((state: SelectionToolbarState) => void) | null = null
125+
let provider: TooltipProvider | null = null
126+
127+
const readState = (): SelectionToolbarState =>
128+
crepe.editor.action((ctx) => {
129+
const commands = ctx.get(commandsCtx)
130+
return {
131+
bold: commands.call(isMarkSelectedCommand.key, strongSchema.type(ctx)),
132+
italic: commands.call(isMarkSelectedCommand.key, emphasisSchema.type(ctx)),
133+
strikethrough: commands.call(isMarkSelectedCommand.key, strikethroughSchema.type(ctx)),
134+
code: commands.call(isMarkSelectedCommand.key, inlineCodeSchema.type(ctx)),
135+
}
136+
})
137+
138+
class MillToolbarView {
139+
constructor(view: EditorView) {
140+
provider = new TooltipProvider({
141+
content: contentEl,
142+
root: document.body,
143+
debounce: 20,
144+
offset: 8,
145+
})
146+
provider.onShow = () => stateCb?.(readState())
147+
this.update(view)
148+
}
149+
150+
update = (view: EditorView, prevState?: Parameters<TooltipProvider['update']>[1]) => {
151+
provider?.update(view, prevState)
152+
if (contentEl.dataset.show === 'true') stateCb?.(readState())
153+
}
154+
155+
destroy = () => {
156+
provider?.destroy()
157+
contentEl.remove()
158+
}
159+
}
160+
161+
crepe.editor.use(millSelectionToolbar)
162+
crepe.editor.config((ctx) => {
163+
ctx.set(millSelectionToolbar.key, {
164+
view: (view: EditorView) => new MillToolbarView(view),
165+
})
166+
})
167+
168+
const COMMANDS = {
169+
bold: toggleStrongCommand,
170+
italic: toggleEmphasisCommand,
171+
strikethrough: toggleStrikethroughCommand,
172+
code: toggleInlineCodeCommand,
173+
} as const
174+
175+
return {
176+
contentEl,
177+
run: (action) => {
178+
crepe.editor.action((ctx) => {
179+
ctx.get(commandsCtx).call(COMMANDS[action].key)
180+
})
181+
stateCb?.(readState())
182+
},
183+
onState: (cb) => {
184+
stateCb = cb
185+
},
186+
destroy: () => {
187+
provider?.destroy()
188+
contentEl.remove()
189+
},
190+
}
191+
}

0 commit comments

Comments
 (0)