fix(mantine): restore compact AI menu items - #3027
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (4)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe change makes Mantine AI suggestion items fit their content, adds end-to-end coverage for prompt and error states, and updates screenshot test argument normalization and Docker image hashing. ChangesAI menu appearance
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This PR restores compact AI menu item sizing in the Mantine theme and adds focused visual and behavioral coverage. No actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (2 skipped: 2 unsupported.) Full details: Description checkExplanation The description is detailed and on-topic. It explains the regression, cause, fix, testing, tooling changes, scope, and verification results. It does not use the template headings or include the checklist, but the required technical information is mostly complete.
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/diagram-block
@blocknote/mantine
@blocknote/math-block
@blocknote/react
@blocknote/server-util
@blocknote/shadcn
@blocknote/xl-ai
@blocknote/xl-docx-exporter
@blocknote/xl-email-exporter
@blocknote/xl-multi-column
@blocknote/xl-odt-exporter
@blocknote/xl-pdf-exporter
@blocknote/xl-typst-exporter
commit: |
|
#2909 changed `.bn-suggestion-menu-item` from `height: 52px` to `min-height: 52px` so items could grow to fit wrapped text. But that rule also matches the small variant, which opted out via `height: fit-content` - which overrode the old fixed `height` and does nothing against a `min-height`. Small items therefore rendered at exactly the same 52px as full-size ones, leaving `size: "small"` as a font-size change and a hidden subtitle. Every default AI menu item is `size: "small"`, so the whole AI menu - prompt actions and the Retry / Cancel error state - rendered stretched. Declares the floor only on the items it is for, rather than setting it for everything and having the small rule try to undo it. `height: fit-content` goes with it: it only ever existed to beat the old fixed `height`. A new size variant is now content-sized by default instead of silently inheriting the clamp. Mantine only: ariakit and shadcn never set a height on suggestion items.
The AI menu had no coverage that could catch this: the one AI screenshot (`ai_menu_scroll_position`) is a `document.body` shot on an 800x400 viewport where the menu is clipped just below the first item, which is why #2909 only needed to update the *slash* menu baselines. Screenshots the AI menu in its prompt and error states. Without the fix the menu is 88px taller, so both fail on all three browsers before pixels are even compared: "Expected image dimensions to be 623x200px, but received 623x288px".
6b789cc to
a21ede6
Compare
a21ede6 to
9944dcd
Compare
…g twice Two papercuts hit while regenerating baselines for the fix in this PR: - vitest declares the flag as `-u, --update [type]` - an *optional* value, which also accepts `new`/`all`/`none`. So in `--run -u <filter>`, cac reads <filter> as the value of `-u`: the filter is dropped and the whole suite silently runs in update mode. `e2e:updateSnaps` ended in `-u`, so any filter a caller appends (as CLAUDE.md instructs) landed in exactly that trap - which quietly rewrote an unrelated, flaky `deleteShallowerBlock` baseline to a wrong value. Attaching the value as `--update=true` makes it unambiguous, so filters stay filters. - wasm-pack emits `packages/xl-typst-compiler/pkg/package.json`, which the image content hash globbed, so building the wasm that docker-run.sh itself requires invalidated the image. A fresh clone paid two full image builds. `pkg` is now pruned alongside `dist`.
9944dcd to
6cd9cd3
Compare
Every item in the AI menu — the prompt actions, and the Retry / Cancel error state — has been rendering at full suggestion-item height since v0.53.0.
Cause
#2909 (BLO-1192) changed the base item rule so items could grow to fit wrapped text:
.bn-mantine .bn-suggestion-menu-item { - height: 52px; + min-height: 52px; + flex-shrink: 0; }That rule also matches the small variant, which opted out with
height: fit-content. That overrode the old fixedheight, but does nothing against amin-height— min-height clamps the used height regardless.So small items rendered at exactly the same 52px as full-size items, reducing
size: "small"to a font-size change and a hidden subtitle:size: "small"is set in exactly one place in the repo (getDefaultAIMenuItems.tsx), so the AI menu absorbed all of it.Fix
Declare the floor only on the items it's for, instead of setting it for everything and having the small rule try to undo it. A new size variant is now content-sized by default, rather than silently inheriting the clamp — which is how this bug happened.
height: fit-contentgoes with it: it only ever existed to beat the old fixedheight.The 52px floor itself is left alone. It predates this (#2033, originally
height: 52px) and is closer to a design constant than a necessity: all 24 default slash menu items have asubtext, so they are naturally 49px and already uniform, and the floor only adds ~3px of padding plus alignment for custom items that omitsubtext(47px). Dropping it is defensible, but it changes the look of every suggestion menu and churns baselines — a separate call from this regression fix.flex-shrink: 0stays on the base rule and is deliberately kept next to a comment explaining why, because it's coupled to the floor rather than decorative — in a height-constrained, scrolling menu:min-height: 52px+flex-shrink: 0min-height: 52pxaloneflex-shrink: 0aloneA flex item defaults to
min-height: auto, whose automatic minimum size already prevents shrinking below content. Writing an explicitmin-heightreplacesautoand switches that off, soflex-shrink: 0is what restores it — without it a wrapped item collapses back to 52px, re-creating the overflow BLO-1192 was about.Mantine only. ariakit and shadcn never set a height on suggestion items.
Test
The AI menu had no coverage that could catch this. The one AI screenshot (
ai_menu_scroll_position) is adocument.bodyshot on an 800×400 viewport where the menu is clipped just below the first item — which is why #2909 only needed to update the slash menu baselines, and why no existing baseline moves in this PR either.aiMenuAppearance.test.tsxscreenshots the AI menu in its prompt and error states. Without the fix the menu is 88px taller, so both fail on all three browsers before pixels are even compared:Should this be ported to ariakit / shadcn?
No — checked, and they never had the bug.
The original report (#2744) shows Code Block's subtext wrapping to two lines with the second line spilling below the item's hover highlight. The overflow is vertical, and the cause is an item box that can't grow. Narrowing the menu until that subtext wraps, then measuring how far the text escapes its item:
height: 52px)min-height)Only the pinned height reproduces it. ariakit and shadcn let the row grow, so there is nothing to port.
Tooling
Two papercuts hit while regenerating baselines here, both in
docker-run.sh:-uswallows the filter. vitest declares the flag as-u, --update [type]— an optional value, which also acceptsnew/all/none. So in--run -u <filter>, cac reads the filter as the value of-u: the filter is dropped and the whole suite silently runs in update mode.e2e:updateSnapsended in-u, so any filter appended to it (as CLAUDE.md instructs) landed in exactly that trap — it quietly rewrote an unrelated, flakydeleteShallowerBlockbaseline to a wrong value during this work. Attaching the value as--update=truemakes it unambiguous.vp run e2e:updateSnaps <filter>now runs 3 files instead of 156.xl-typst-compiler/pkg/package.json, which the image content hash globbed — so building the wasm thatdocker-run.shitself requires invalidated the image.pkgis now pruned alongsidedist.Verification
Full suite green in Docker: 149 passed / 10 skipped (159 files), 890 tests, no snapshot churn beyond the six new baselines.
🤖 Generated with Claude Code