Skip to content

Clip markdown editor toolbar corners to the rounded outline #5765

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
FadhlanR wants to merge 4 commits into
base: main
Choose a base branch
from
+290 −116
Open

Clip markdown editor toolbar corners to the rounded outline #5765

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0be9701
Clip markdown editor toolbar corners to the rounded outline
FadhlanR Aug 13, 2026
c7ce9a8
Clarify the embed-dropdown clip contract and pin it with a test
FadhlanR Aug 14, 2026
8a88e2f
Keep the markdown toolbar menus out of the corner-clip box
FadhlanR Aug 18, 2026
e973eda
Clamp the corner-clip radius so a sub-1px theme token can't invalidat…
FadhlanR Aug 18, 2026
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
302 changes: 187 additions & 115 deletions packages/base/codemirror-editor.gts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,15 +527,54 @@ export default class CodeMirrorEditor extends GlimmerComponent<CodeMirrorEditorS
// ── Markdown embed chooser (toolbar) ────────────────────────────────────

@tracked _embedPopoverOpen = false;
// Inline style pinning the popover under its trigger. The popover is
// `position: fixed` (see below) so it escapes the `.codemirror-body` corner
// clip, which means it needs explicit viewport coordinates rather than the
// `top: 100%` an in-flow absolute box would get for free.
@tracked _embedPopoverStyle = '';
_embedTriggerEl: HTMLElement | null = null;

get _currentBfmRef(): BfmRefRange | undefined {
return this._selectionInfo?.currentRef;
}

_toggleEmbedPopover = () => {
this._embedPopoverOpen = !this._embedPopoverOpen;
// Pin the popover directly under the trigger, in viewport coordinates. The
// trigger is inside a sticky toolbar, so this is recomputed on scroll/resize
// (see `_trackEmbedPopover`) to keep the two glued together.
_positionEmbedPopover = () => {
let trigger = this._embedTriggerEl;
if (!trigger) return;
let r = trigger.getBoundingClientRect();
this._embedPopoverStyle = `position: fixed; top: ${r.bottom + 4}px; left: ${r.left}px;`;
};

_toggleEmbedPopover = (e: Event) => {
if (this._embedPopoverOpen) {
this._embedPopoverOpen = false;
return;
}
this._embedTriggerEl = e.currentTarget as HTMLElement;
this._positionEmbedPopover();
this._embedPopoverOpen = true;
};

// While the popover is open, keep it pinned under the trigger as the outer
// panel scrolls or the window resizes. Installed on the popover element, so
// its lifecycle tracks the popover's presence in the DOM.
_trackEmbedPopover = modifier(() => {
this._positionEmbedPopover();
let reposition = () => this._positionEmbedPopover();
window.addEventListener('scroll', reposition, {
capture: true,
passive: true,
});
window.addEventListener('resize', reposition);
return () => {
window.removeEventListener('scroll', reposition, { capture: true });
window.removeEventListener('resize', reposition);
};
});

_openEmbedChooser = async (defaultTab: 'card' | 'file') => {
this._embedPopoverOpen = false;
let chooser = this.cardContext?.markdownEmbedChooser;
Expand Down Expand Up @@ -938,129 +977,134 @@ export default class CodeMirrorEditor extends GlimmerComponent<CodeMirrorEditorS
data-test-codemirror-editor
...attributes
>
{{! ── Docked toolbar ── }}
{{! template-lint-disable no-pointer-down-event-binding }}
<div
class='codemirror-toolbar'
data-overlay-clip-header
data-test-markdown-toolbar
>
{{yield to='leadingControls'}}
{{#if (has-block 'leadingControls')}}
<span class='toolbar-divider'></span>
{{/if}}

{{#if this._currentBfmRef}}
<Tooltip @placement='top' data-test-toolbar-tooltip='edit-embed'>
<:trigger>
<button
class='toolbar-btn'
data-test-toolbar='edit-embed'
type='button'
aria-label='Edit embed'
{{on 'mousedown' this._preventFocusLoss}}
{{on 'click' this._openEditEmbed}}
><PencilIcon width='16' height='16' /></button>
</:trigger>
<:content>
<span class='toolbar-tooltip'>
<span class='toolbar-tooltip__label'>Edit embed</span>
</span>
</:content>
</Tooltip>
{{else}}
<div class='toolbar-embed-trigger'>
<Tooltip @placement='top' data-test-toolbar-tooltip='add-embed'>
<div class='codemirror-body' data-test-codemirror-body>
{{! ── Docked toolbar ── }}
{{! template-lint-disable no-pointer-down-event-binding }}
<div
class='codemirror-toolbar'
data-overlay-clip-header
data-test-markdown-toolbar
>
{{yield to='leadingControls'}}
{{#if (has-block 'leadingControls')}}
<span class='toolbar-divider'></span>
{{/if}}

{{#if this._currentBfmRef}}
<Tooltip @placement='top' data-test-toolbar-tooltip='edit-embed'>
<:trigger>
<button
class='toolbar-btn
{{if this._embedPopoverOpen "toolbar-btn--active"}}'
data-test-toolbar='add-embed'
class='toolbar-btn'
data-test-toolbar='edit-embed'
type='button'
aria-label='Add embed'
aria-expanded={{if this._embedPopoverOpen 'true' 'false'}}
aria-label='Edit embed'
{{on 'mousedown' this._preventFocusLoss}}
{{on 'click' this._toggleEmbedPopover}}
><PlusIcon width='16' height='16' /></button>
{{on 'click' this._openEditEmbed}}
><PencilIcon width='16' height='16' /></button>
</:trigger>
<:content>
<span class='toolbar-tooltip'>
<span class='toolbar-tooltip__label'>Add embed</span>
<span class='toolbar-tooltip__label'>Edit embed</span>
</span>
</:content>
</Tooltip>
{{#if this._embedPopoverOpen}}
<div
class='toolbar-embed-popover'
data-test-toolbar-embed-popover
>
<button
type='button'
class='toolbar-embed-popover__item'
data-test-toolbar-embed='card'
{{on 'mousedown' this._preventFocusLoss}}
{{on 'click' (fn this._openEmbedChooser 'card')}}
>Add a card</button>
<button
type='button'
class='toolbar-embed-popover__item'
data-test-toolbar-embed='file'
{{on 'mousedown' this._preventFocusLoss}}
{{on 'click' (fn this._openEmbedChooser 'file')}}
>Add a file</button>
</div>
{{/if}}
</div>
{{/if}}
<span class='toolbar-divider'></span>

{{#each this.toolbarButtons as |btn|}}
{{#if btn.divider}}
<span class='toolbar-divider'></span>
{{else}}
{{! Every item gets a styled tooltip — the label, plus a shortcut
<div class='toolbar-embed-trigger'>
<Tooltip @placement='top' data-test-toolbar-tooltip='add-embed'>
<:trigger>
<button
class='toolbar-btn
{{if this._embedPopoverOpen "toolbar-btn--active"}}'
data-test-toolbar='add-embed'
type='button'
aria-label='Add embed'
aria-expanded={{if this._embedPopoverOpen 'true' 'false'}}
{{on 'mousedown' this._preventFocusLoss}}
{{on 'click' this._toggleEmbedPopover}}
><PlusIcon width='16' height='16' /></button>
</:trigger>
<:content>
<span class='toolbar-tooltip'>
<span class='toolbar-tooltip__label'>Add embed</span>
</span>
</:content>
</Tooltip>
{{#if this._embedPopoverOpen}}
<div
class='toolbar-embed-popover'
style={{htmlSafe this._embedPopoverStyle}}
data-test-toolbar-embed-popover
{{this._trackEmbedPopover}}
>
<button
type='button'
class='toolbar-embed-popover__item'
data-test-toolbar-embed='card'
{{on 'mousedown' this._preventFocusLoss}}
{{on 'click' (fn this._openEmbedChooser 'card')}}
>Add a card</button>
<button
type='button'
class='toolbar-embed-popover__item'
data-test-toolbar-embed='file'
{{on 'mousedown' this._preventFocusLoss}}
{{on 'click' (fn this._openEmbedChooser 'file')}}
>Add a file</button>
</div>
{{/if}}
</div>
{{/if}}
<span class='toolbar-divider'></span>

{{#each this.toolbarButtons as |btn|}}
{{#if btn.divider}}
<span class='toolbar-divider'></span>
{{else}}
{{! Every item gets a styled tooltip — the label, plus a shortcut
key badge when the item has a CodeMirror binding. The tooltip
is suppressed while the control is disabled. }}
<Tooltip
@placement='top'
@disabled={{btn.disabled}}
data-test-toolbar-tooltip={{btn.testId}}
>
<:trigger>
<button
class='toolbar-btn {{if btn.active "toolbar-btn--active"}}'
data-test-toolbar={{btn.testId}}
type='button'
aria-label={{btn.label}}
aria-pressed={{btn.ariaPressed}}
disabled={{btn.disabled}}
{{on 'mousedown' this._preventFocusLoss}}
{{on 'click' btn.action}}
>{{#let btn.icon as |Icon|}}<Icon
width='16'
height='16'
/>{{/let}}</button>
</:trigger>
<:content>
<span class='toolbar-tooltip'>
<span class='toolbar-tooltip__label'>{{btn.label}}</span>
{{#if btn.shortcut}}
<kbd class='shortcut-key'>{{btn.shortcut}}</kbd>
{{/if}}
</span>
</:content>
</Tooltip>
{{/if}}
{{/each}}
<Tooltip
@placement='top'
@disabled={{btn.disabled}}
data-test-toolbar-tooltip={{btn.testId}}
>
<:trigger>
<button
class='toolbar-btn
{{if btn.active "toolbar-btn--active"}}'
data-test-toolbar={{btn.testId}}
type='button'
aria-label={{btn.label}}
aria-pressed={{btn.ariaPressed}}
disabled={{btn.disabled}}
{{on 'mousedown' this._preventFocusLoss}}
{{on 'click' btn.action}}
>{{#let btn.icon as |Icon|}}<Icon
width='16'
height='16'
/>{{/let}}</button>
</:trigger>
<:content>
<span class='toolbar-tooltip'>
<span class='toolbar-tooltip__label'>{{btn.label}}</span>
{{#if btn.shortcut}}
<kbd class='shortcut-key'>{{btn.shortcut}}</kbd>
{{/if}}
</span>
</:content>
</Tooltip>
{{/if}}
{{/each}}
</div>

{{! template-lint-disable no-invalid-interactive }}
<div
class='codemirror-mount'
data-test-codemirror-mount
{{on 'mousedown' this._focusEditorOnPointerDown}}
{{this.mountEditor this.cm @content @onUpdate this.livePreview}}
></div>
</div>

{{! template-lint-disable no-invalid-interactive }}
<div
class='codemirror-mount'
data-test-codemirror-mount
{{on 'mousedown' this._focusEditorOnPointerDown}}
{{this.mountEditor this.cm @content @onUpdate this.livePreview}}
></div>
</div>

{{#if this.livePreview}}
Expand Down Expand Up @@ -1169,6 +1213,32 @@ export default class CodeMirrorEditor extends GlimmerComponent<CodeMirrorEditorS
outline-color: var(--ring, var(--boxel-highlight));
}

/* Wraps the sticky toolbar + editor mount so the editor's rounded
corners clip the toolbar's square bottom corners when it docks at
the bottom on scroll. `overflow: clip` (not `hidden`) does this
without establishing a scroll container, so the toolbar keeps
sticking to the outer scroll panel. The radius is inset by the 1px
border so it hugs the border's inner curve.

The toolbar's two menus must escape this clip: when the toolbar
docks at the bottom there is zero room below it inside the box, so a
menu rendered here would be swallowed while its trigger still looks
normal. Both are kept out — the mode selector wormholes out of place
(BoxelSelect without `renderInPlace`) and the Add-embed popover is
`position: fixed` (its containing block is the viewport, so this
clip can't reach it). */
.codemirror-body {
display: flex;
flex-direction: column;
flex: 1;
min-height: 0;
/* Inset by the 1px border; clamped at 0 so a card that themes
`--boxel-border-radius` below 1px doesn't drive the calc negative
(which would invalidate the declaration). */
border-radius: max(0px, calc(var(--boxel-border-radius) - 1px));
overflow: clip;

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Exclude the Add-embed popover from the clipping wrapper

The new overflow: clip also clips .toolbar-embed-popover, which remains inside this wrapper and is positioned below the toolbar with top: 100%. When the outer edit panel is scrolled far enough that the sticky toolbar is constrained against the editor's bottom edge—the state this change targets—there is no room below it inside the wrapper, so opening Add embed hides part or all of the menu and can make its actions unclickable. The added test only verifies that the menu exists in the DOM, not that it is visible; the popover should escape the clipped subtree or flip above the toolbar near the bottom.

Useful? React with 👍 / 👎.

@FadhlanR FadhlanR Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] Fixed in 8a88e2f. The Add-embed popover is now position: fixed, pinned under its trigger from getBoundingClientRect() and repositioned on scroll/resize while it's open. Its containing block is the viewport, so .codemirror-body's overflow: clip can no longer reach it — the menu stays visible and clickable when the toolbar docks at the bottom. The test was inverted to assert the popover is fixed-positioned rather than merely present in the DOM.

You were right and it under-counted: the mode selector had the same problem (its menu is taller), and it's fixed too — it now renders in the shared dropdown wormhole instead of in place.

}
Comment on lines +1230 to +1240

@habdelra habdelra Aug 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] Blocking — regression. This clip box swallows both menus that live in the toolbar, not just the Add-embed dropdown the comment above accounts for. The one that isn't mentioned anywhere in this change is the Compose / Source / Preview mode selector — the control this change's own summary names as the left end of the grey bar.

Background: what overflow: clip does and does not clip. An overflow value other than visible clips a descendant only when the clipping element sits in that descendant's containing-block chain. So a position: fixed descendant (containing block = the viewport, an ancestor of .codemirror-body) escapes; a position: absolute descendant whose containing block is inside .codemirror-body does not. I verified both directions in Chromium against this exact box structure:

positionFixed    -> probe painted below the clip box, hit test returns the probe   (escapes)
positionAbsolute -> probe below the clip box, hit test falls through to the page   (clipped)

Both toolbar menus land on the clipped side of that rule.

  1. .toolbar-embed-popover — position: absolute, containing block .toolbar-embed-trigger (position: relative, in the toolbar). This is the case the automated reviewer's open thread on this rule flags, and it is real; see my note on the comment block just above.
  2. MarkdownEditorModeSelect — it renders BoxelSelect with @renderInPlace={{true}} (packages/base/components/markdown-editor-mode-select.gts), and rich-markdown.gts yields it into <:leadingControls>, which this template renders as the first child of .codemirror-toolbar. renderInPlace keeps .ember-basic-dropdown-content in the DOM next to its trigger, and packages/boxel-ui/src/styles/ember-basic-dropdown.css gives it position: absolute inside .ember-basic-dropdown { position: relative }. Containing block is inside the clip box → clipped.

Measured, using this repo's real CSS and tokens. I rebuilt the box structure in Chromium from the declarations in this file, rich-markdown.gts, ember-basic-dropdown.css, select/index.gts and markdown-editor-mode-select.gts, with variables.css loaded so the tokens resolve to their real values (--boxel-border-radius: 10px, --boxel-sp-xs: 9px, --boxel-sp-3xs: 5.06px, --boxel-font-size-sm: 14px). The 3-option mode menu measures 94px tall. Scrolling the outer edit panel, with a minimum-height editor (a one-line body field, editorH: 155px):

panel scrollTop room below toolbar in clip box toolbar on screen menu visible options clickable
0 118px yes 94/94 3/3
57 62px yes 64/94 2/3
86 33px yes 35/94 1/3
115 4px yes 6/94 0/3
143 0px yes 2/94 0/3

The toolbar is still fully on screen in every one of those rows — the trigger looks and behaves normally, and the menu it opens is simply not there. On a one-line body field that state is ~57px of outer-panel scroll away. With a long body the failure window is the last ~100px of scroll, i.e. precisely the bottom-docked state this change exists to fix.

Flipping the menu upward is not an escape either: ember-basic-dropdown's in-place positioning measures window.innerHeight - triggerRect.bottom, so it keys off the browser window, not this clip box. In the docked state the toolbar is near the top of the window, so it stays below and gets clipped; and if it did flip, the toolbar is pinned at the clip box's top edge, so it would be clipped upward instead.

The way out. The menus have to live outside whatever box does the clipping — any clip that contains the toolbar clips the toolbar's positioned descendants, so moving or resizing the wrapper won't help.

  • Mode selector: dropping @renderInPlace={{true}} puts the menu in #ember-basic-dropdown-wormhole, outside the clip entirely. BoxelSelect already supports this path deliberately for themed contexts — onOpen() calls startObservingTheme() → syncCustomProps(), which copies --background / --foreground / --border / --primary onto the wormhole container so the card theme still applies (packages/boxel-ui/src/components/select/index.gts). Worth confirming that holds for a card-rendered field before committing to it, since packages/base/enum.gts reaches for renderInPlace too and that may encode a constraint I haven't found.
  • Add-embed popover: render it outside .codemirror-body — either {{in-element}} into a container under .codemirror-editor (which already carries position: relative), or switch it to position: fixed placed from the trigger's getBoundingClientRect(), which the probe above shows escapes the clip.
  • Cheaper fallback, if both menus must stay in place: skip the clip entirely and put border-bottom-left-radius / border-bottom-right-radius on .codemirror-toolbar itself. One line, zero risk to the menus — but the bar then carries rounded bottom corners in the undocked state too, where they read as small notches against the mount. That is a design call rather than a correctness one.

Generated by Claude Code

@FadhlanR FadhlanR Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] Fixed in 8a88e2f. Dropped @renderInPlace={{true}} from MarkdownEditorModeSelect, so BoxelSelect renders its menu in #ember-basic-dropdown-wormhole — out of the clip box entirely. BoxelSelect's onOpen still syncs the card theme onto the wormhole container, so the themed look holds. The Add-embed popover is handled separately (now position: fixed, since it's a hand-rolled div rather than a BoxelSelect).


/* Fill the field so clicking anywhere below the text focuses it. */
.codemirror-mount {
flex: 1;
Expand Down Expand Up @@ -1483,11 +1553,13 @@ export default class CodeMirrorEditor extends GlimmerComponent<CodeMirrorEditorS
position: relative;
display: inline-flex;
}
/* `position: fixed` (coordinates set inline from the trigger's rect)
so the popover escapes the `.codemirror-body` corner clip — its
containing block is the viewport, which that clip can't reach. An
absolute popover here would be swallowed once the toolbar docks at
the bottom, where there is no room left below it inside the clip. */
.toolbar-embed-popover {
position: absolute;
top: 100%;
left: 0;
margin-top: 4px;
position: fixed;
min-width: 140px;
background: var(--boxel-light);
color: var(--boxel-dark);
Expand Down
6 changes: 5 additions & 1 deletion packages/base/components/markdown-editor-mode-select.gts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,11 @@ export default class MarkdownEditorModeSelect extends GlimmerComponent<Signature
@selected={{this.selectedOption}}
@onChange={{this.handleChange}}
@searchEnabled={{false}}
@renderInPlace={{true}}
{{! Render the menu in the shared dropdown wormhole, not in place: this
toolbar sits inside CodeMirrorEditor's `overflow: clip` corner-clip
box, and an in-place menu would be swallowed by it once the toolbar
docks at the bottom on scroll. BoxelSelect syncs the card theme onto
the wormhole on open, so the themed look is preserved. }}
@matchTriggerWidth={{false}}
@dropdownClass='markdown-editor-mode-select-dropdown'
data-test-markdown-mode-select={{this.selectedOption.value}}
Expand Down
Loading
Loading