fix: hide side menu on scroll instead of overflow hacks#2630
Conversation
Reverts the overflow/positioning workarounds from #2043 and instead hides the side menu when the user scrolls, preventing it from overflowing outside the editor's scroll container. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
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 Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds auto-hide callbacks for side menu and table handles invoked from Floating UI mounted callbacks; GenericPopover now merges default autoUpdate with user-provided whileElementsMounted; minor CSS cleanup removing Changes
Sequence DiagramsequenceDiagram
participant FUI as Floating UI
participant CTRL as SideMenuController
participant EDIT as Editor (getExtension)
participant EXT as SideMenuExtension
participant STATE as View State
FUI->>CTRL: whileElementsMounted (mount)
Note over CTRL: initial mount recorded
FUI->>CTRL: whileElementsMounted (ancestorScroll/update)
CTRL->>EDIT: getExtension(SideMenuExtension)
EDIT->>EXT: hideMenuIfNotFrozen()
EXT->>EXT: if !menuFrozen and state.show
alt Menu not frozen & visible
EXT->>STATE: set state.show = false
EXT->>STATE: emitUpdate(state)
end
FUI->>CTRL: cleanup (unmount)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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/mantine
@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
commit: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/extensions/SideMenu/SideMenu.ts`:
- Around line 793-797: The method hideMenuIfNotFrozen currently sets
view.state.show = false and calls view.emitUpdate even when the menu is already
hidden; update hideMenuIfNotFrozen to first check that view.state.show is truthy
(and that view exists and is not frozen via view!.menuFrozen) and only then set
show = false and call view!.emitUpdate(view!.state!) to avoid redundant
emissions and re-renders; reference hideMenuIfNotFrozen, view!.menuFrozen,
view!.state!.show, and view!.emitUpdate when locating the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a8eecee9-d1d3-4e88-b69a-77deee5684fd
📒 Files selected for processing (4)
docs/app/styles.csspackages/core/src/extensions/SideMenu/SideMenu.tspackages/react/src/components/Popovers/GenericPopover.tsxpackages/react/src/components/SideMenu/SideMenuController.tsx
💤 Files with no reviewable changes (1)
- docs/app/styles.css
nperez0111
left a comment
There was a problem hiding this comment.
using floating ui for the scroll dismiss seems like a bit of an abuse, but it works
|
table handles would likely want the same fix applied, but it is super minor |
Apply the same scroll-hide pattern to table handles. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Hide the side menu when the user scrolls, preventing it from overflowing outside the editor's scroll container (e.g. in docs demos). This reverts the CSS workarounds introduced in #2043 in favor of a proper floating-ui–based solution.
Rationale
PR #2043 added
overflow: noneandposition: relativehacks to the docs demo containers to prevent the side menu from visually escaping the demo box. This approach was fragile and didn't address the root cause. The proper fix is to hide the side menu on scroll — the same UX pattern used for keyboard input.We prefer to handle this on the floating-ui layer instead of adding additional event listeners in the SideMenu extension, as we're trying to refactor more logic out of our custom extensions and into standard floating-ui patterns.
Changes
docs/app/styles.css— Removed.demo { overflow: none }and.demo .bn-container { position: relative }from docs: Made UI elements no longer overflow demo box #2043GenericPopover.tsx— AddedmergeWhileElementsMountedhelper that composes the defaultautoUpdatewith any additionalwhileElementsMountedhandler provided via propsSideMenuController.tsx— Added awhileElementsMountedhandler usingautoUpdatewithancestorScroll: trueto detect scroll and callhideMenuIfNotFrozen()SideMenu.ts— AddedhideMenuIfNotFrozen()extension method that hides the menu only when it's not frozen (i.e. drag handle submenu is not open)Impact
hideMenuIfNotFrozenrespects the frozen stateGenericPopover's new merge behavior is backwards-compatible: when nowhileElementsMountedis passed, behavior is identical to beforeTesting
Checklist
Additional Notes
Reverts CSS changes from #2043.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor
Style