refactor: portal floating UI elements to document.body to prevent overflow clipping BLO-1115#2591
refactor: portal floating UI elements to document.body to prevent overflow clipping BLO-1115#2591
Conversation
… clipping Floating UI elements (menus, toolbars, emoji picker) are now portaled to a dedicated container at document.body, preventing them from being clipped by overflow:hidden ancestors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR implements a portal system for floating UI elements to escape parent overflow constraints. CSS selectors migrate from Changes
Sequence DiagramsequenceDiagram
participant User
participant Editor as BlockNoteView
participant Portal as Portal Container
participant Floating as FloatingUI<br/>(Popover/Menu)
participant DOM as Document.body
User->>Editor: Interact (type `/`, select text, etc.)
Editor->>Editor: Initialize portalRoot element
Editor->>Portal: createPortal() with bn-root class
Portal->>DOM: Mount separate portal container
User->>Editor: Trigger floating UI (menu, toolbar)
Editor->>Floating: Request floating content
Floating->>Portal: Query portalRoot from context
Portal-->>Floating: Return portalRoot element
Floating->>Floating: Calculate position<br/>(escapes overflow constraints)
Floating->>DOM: Render in portal container<br/>(outside editor subtree)
Note over Portal,DOM: Floating UI escapes parent overflow<br/>and renders above editor hierarchy
Floating-->>User: Display menu/toolbar unclipped
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/extensions/SideMenu/SideMenu.ts (1)
615-626:⚠️ Potential issue | 🟡 MinorScope the
.bn-rootexemption to the current editor instance.
closest(".bn-root")now treats any BlockNote root as local UI. On multi-editor pages, a portaled toolbar from editor B hovering over editor A can keep editor A's side menu active because the target still has a.bn-rootancestor. This check needs an instance-scoped marker for the current editor/root pair, not a global class match.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/extensions/SideMenu/SideMenu.ts` around lines 615 - 626, The current check uses (event.target as HTMLElement).closest(".bn-root") which matches any editor root on the page; change it to an instance-scoped check so only the current editor/root keeps the side menu active. Use the SideMenu instance's root reference (e.g. this.rootElement or this.editorRoot) and replace the global closest(".bn-root") logic with an instance-scoped test such as using this.rootElement.contains(event.target as Node) or matching a unique root identifier/data-attribute (e.g. data-bn-root-id === this.rootId) on ancestor lookup; update the closestBNRoot usage and the conditional that references cursorWithinEditor so it only treats elements within this specific editor root as local UI.
🤖 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/mantine/src/popover/Popover.tsx`:
- Line 26: Popover.tsx currently clears zIndex when portalRoot exists which lets
Mantine's default z-index win; change the zIndex logic to use the CSS variable
fallback pattern from GenericPopover.tsx by assigning zIndex to the CSS variable
--bn-ui-base-z-index with a 10000 fallback when portalRoot is truthy (and keep
10000 when not portaled) so portaled popovers respect the app's base z-index
system; update the zIndex prop usage in the Popover component (the zIndex prop
set at the repository diff line) accordingly to use the
var(--bn-ui-base-z-index, 10000) approach.
In `@packages/react/src/editor/BlockNoteView.tsx`:
- Around line 216-227: The portaled root created by createPortal currently only
forwards className and data-color-scheme, causing loss of important attributes
from the editor container; update the BlockNoteView portaling logic (the element
created where setPortalRoot is used) to derive and forward explicit props from
...rest (e.g., dir, any data-* attributes used for theming like data-theming-*,
data-mantine-color-scheme, and inline style/CSS variable overrides) instead of
relying on className alone, and avoid forwarding layout-only classes by either
whitelisting these attributes or picking them out from rest (preserve
editorColorScheme and className via mergeCSSClasses, but also copy dir, style,
and any data-* theming attributes into the portaled div so floating UI retains
same theme/context as the editor).
---
Outside diff comments:
In `@packages/core/src/extensions/SideMenu/SideMenu.ts`:
- Around line 615-626: The current check uses (event.target as
HTMLElement).closest(".bn-root") which matches any editor root on the page;
change it to an instance-scoped check so only the current editor/root keeps the
side menu active. Use the SideMenu instance's root reference (e.g.
this.rootElement or this.editorRoot) and replace the global closest(".bn-root")
logic with an instance-scoped test such as using
this.rootElement.contains(event.target as Node) or matching a unique root
identifier/data-attribute (e.g. data-bn-root-id === this.rootId) on ancestor
lookup; update the closestBNRoot usage and the conditional that references
cursorWithinEditor so it only treats elements within this specific editor root
as local UI.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c2308343-2a71-42d8-9eee-c0b1fdf5b028
📒 Files selected for processing (21)
docs/content/docs/react/styling-theming/themes.mdxexamples/01-basic/12-multi-editor/src/App.tsxexamples/02-backend/04-rendering-static-documents/src/App.tsxexamples/04-theming/02-changing-font/src/styles.cssexamples/04-theming/03-theming-css/src/styles.cssexamples/04-theming/04-theming-css-variables/src/styles.cssexamples/05-interoperability/09-blocks-to-html-static-render/src/App.tsxpackages/ariakit/src/comments/Comment.tsxpackages/ariakit/src/popover/Popover.tsxpackages/core/src/editor/editor.csspackages/core/src/extensions/SideMenu/SideMenu.tspackages/mantine/src/popover/Popover.tsxpackages/react/src/components/Comments/EmojiPicker.tsxpackages/react/src/components/Popovers/GenericPopover.tsxpackages/react/src/editor/BlockNoteContext.tspackages/react/src/editor/BlockNoteView.tsxpackages/react/src/editor/ComponentsContext.tsxpackages/react/src/editor/styles.csspackages/shadcn/src/comments/Comment.tsxpackages/shadcn/src/popover/popover.tsxplayground/src/style.css
💤 Files with no reviewable changes (1)
- packages/core/src/editor/editor.css
| onChange={onOpenChange} | ||
| position={position} | ||
| zIndex={10000} | ||
| zIndex={portalRoot ? undefined : 10000} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify z-index handling for Mantine popovers and bn root variable wiring.
# 1) Confirm changed Mantine popover behavior in this file.
rg -nP -C3 'withinPortal=|portalProps=|zIndex=' packages/mantine/src/popover/Popover.tsx
# 2) Check whether bn-ui-base-z-index is consumed anywhere in styling/code.
rg -nP -C2 'bn-ui-base-z-index|--bn-ui-base-z-index|z-index' packages/react packages/mantine
# 3) Inspect portal root creation context for explicit stacking styles.
rg -nP -C8 'createPortal\\(|className=|data-color-scheme|bn-root' packages/react/src/editor/BlockNoteView.tsxRepository: TypeCellOS/BlockNote
Length of output: 2993
🏁 Script executed:
# Find the portal root creation and its styling context
rg -nA15 -B5 'createPortal|portalRoot.*=' packages/react/src/editor/BlockNoteView.tsx | head -100Repository: TypeCellOS/BlockNote
Length of output: 2391
🏁 Script executed:
# Check if there's z-index styling on the portal root or bn-root specifically
rg -nP 'bn-root|portal.*root' packages/react/src/editor/styles.css packages/react/src/editor/BlockNoteView.tsx -A3 -B3Repository: TypeCellOS/BlockNote
Length of output: 4550
🏁 Script executed:
# Check Mantine Popover implementation or types to see how zIndex behaves
fd -t f -e tsx -e ts packages/mantine | xargs grep -l "zIndex" | head -5Repository: TypeCellOS/BlockNote
Length of output: 433
🏁 Script executed:
# Find Mantine Popover files
fd -t f --full-path 'packages/mantine' -e tsx -e ts | head -20Repository: TypeCellOS/BlockNote
Length of output: 856
🏁 Script executed:
# Check if there's any handling of zIndex prop for Popover in Mantine code
rg -n 'zIndex|z-index' packages/mantine/src/popover/ -B3 -A3Repository: TypeCellOS/BlockNote
Length of output: 538
🏁 Script executed:
# Verify what Mantine's default z-index behavior is for portaled popovers
rg -n 'const.*Popover|export.*Popover' packages/mantine/src/popover/ -A20 | head -80Repository: TypeCellOS/BlockNote
Length of output: 3408
Portaled popovers lose stacking control with Mantine's default z-index.
At line 26, zIndex is set to undefined when portalRoot exists. The portal root (created in packages/react/src/editor/BlockNoteView.tsx:217-226) has no explicit z-index styling and inherits --bn-ui-base-z-index: 0 from the .bn-root CSS rule. This causes Mantine to apply its default z-index (300 via CSS variables), which can fall behind fixed/sticky app chrome without explicit stacking control.
The fix aligns with the established pattern used in GenericPopover.tsx and other UI components, which leverage the --bn-ui-base-z-index CSS variable for consistent z-index management:
Proposed fix
- zIndex={portalRoot ? undefined : 10000}
+ zIndex={portalRoot ? "var(--bn-ui-base-z-index, 10000)" : 10000}This ensures portaled popovers respect the base z-index system while maintaining fallback to 10000 when the variable is not defined.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| zIndex={portalRoot ? undefined : 10000} | |
| zIndex={portalRoot ? "var(--bn-ui-base-z-index, 10000)" : 10000} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/mantine/src/popover/Popover.tsx` at line 26, Popover.tsx currently
clears zIndex when portalRoot exists which lets Mantine's default z-index win;
change the zIndex logic to use the CSS variable fallback pattern from
GenericPopover.tsx by assigning zIndex to the CSS variable --bn-ui-base-z-index
with a 10000 fallback when portalRoot is truthy (and keep 10000 when not
portaled) so portaled popovers respect the app's base z-index system; update the
zIndex prop usage in the Popover component (the zIndex prop set at the
repository diff line) accordingly to use the var(--bn-ui-base-z-index, 10000)
approach.
YousefED
changed the title
nperez0111
left a comment
nperez0111
left a comment
There was a problem hiding this comment.
Definitely not comfortable with this approach. I agree it is worth solving, but as a lib I don't think we have this luxury
Fair, ideally you wouldn't add sth to document.root as a library. Do you think it needs a user-facing API, or would you take a completely different approach? Note that @ https://floating-ui.com/docs/misc#clipping, they mention a portal is the only reliable solution to fix the clipping problem. It's also similar to what other libraries like base-ui etc do by default, if I'm not mistaken |
Unsure exactly how I'd go about it, I didn't review the existing issues to know where the current limitations are, to know the right problem to solve.
Yea, I think we should give the fixed positioning strategy a try, which they also mention:
I think base-ui can get away with it because it is unstyled, whereas I think we will run into some synchronization of styles issues between something within the portal and something within the editor. I'd rather not have to make that distinction if we can get away with it. We would be fighting against the |
|
fyi, this is what claude gives when asking for downsides of "fixed" (didn't validate all if this): The Misc page doesn't go into the downsides directly — let me grab the Explicitly from the docs:
What that means in practice (CSS-level consequences):
So if your floating element lives inside — or gets portalled into — a subtree with any of those CSS properties on an ancestor, Also relevant for BlockNote specifically: if you have a scrollable editor container, let's review later 👍 |
…l element access and customization
|
We've decided to continue with this, after confirming that:
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/mantine/src/BlockNoteView.tsx (1)
41-41:⚠️ Potential issue | 🟠 MajorCompose the incoming
portalRefinstead of overwriting it.The
restobject (destructured at line 41) can contain a consumer-suppliedportalRef, but it gets replaced unconditionally at line 100. This pattern differs frompackages/react/src/editor/BlockNoteView.tsx(lines 163-174), which explicitly destructuresportalRefand merges it with the internal callback usingmergeRefs. DestructureportalReffrom props, merge it with the Mantine-specific callback, and pass the merged ref instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mantine/src/BlockNoteView.tsx` at line 41, The code overwrites a consumer-supplied portalRef because props are destructured into rest (const { className, theme, ...rest } = props) and then the component assigns its own portal ref unconditionally; instead, destructure portalRef from props (const { className, theme, portalRef, ...rest } = props), import/use mergeRefs, compose the consumer portalRef with the component's internal Mantine portal ref callback via mergeRefs(consumerPortalRef, internalPortalRefCallback) and pass that merged ref where the component currently assigns its own portal ref so the consumer ref is preserved and called as well.
🤖 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/react/src/editor/BlockNoteView.tsx`:
- Around line 235-242: The createPortal call in BlockNoteView currently
dereferences document.body unconditionally (using mergedPortalRef and
mergeCSSClasses), which breaks SSR; wrap the portal rendering in a conditional
that checks typeof document !== "undefined" (and optionally that document.body
exists) and only call createPortal when true, otherwise render null or the
component's existing nullable portal state so the component doesn't access
document during server rendering.
---
Outside diff comments:
In `@packages/mantine/src/BlockNoteView.tsx`:
- Line 41: The code overwrites a consumer-supplied portalRef because props are
destructured into rest (const { className, theme, ...rest } = props) and then
the component assigns its own portal ref unconditionally; instead, destructure
portalRef from props (const { className, theme, portalRef, ...rest } = props),
import/use mergeRefs, compose the consumer portalRef with the component's
internal Mantine portal ref callback via mergeRefs(consumerPortalRef,
internalPortalRefCallback) and pass that merged ref where the component
currently assigns its own portal ref so the consumer ref is preserved and called
as well.
🪄 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: 53880307-20d0-4b11-b649-892b9ee5da4e
📒 Files selected for processing (2)
packages/mantine/src/BlockNoteView.tsxpackages/react/src/editor/BlockNoteView.tsx
…i-overflow-clipping
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…i-overflow-clipping
…al portal root support
|
Updated PR to fix:
Tested with:
|
| * elements (menus, toolbars) to escape overflow:hidden ancestors. | ||
| * Set by BlockNoteView; undefined in headless mode. | ||
| */ | ||
| public portalElement: HTMLElement | undefined; |
There was a problem hiding this comment.
@nperez0111 not sure about setting this on the editor. We need it for the UniqueID and SideMenu extensions though. wdyt?
There was a problem hiding this comment.
Nope, I would not put this on the editor, much less public. Need to find a workaround here
| padding: 0; | ||
| } | ||
|
|
||
| /* |
There was a problem hiding this comment.
this was unused, lingering code that I removed in this PR
| const internalPortalRef = useCallback( | ||
| (node: HTMLDivElement | null) => { | ||
| editor.portalElement = node ?? undefined; | ||
| setPortalRoot(node); |
There was a problem hiding this comment.
@nperez0111 this will cause an extra render unfortunately, but afaik it's needed because the blockNoteContext depends on it.
An alternative would be to not use a ref but just document.createElement ahead-of-time - but that seems less "idiomatic react". wdyt?
There was a problem hiding this comment.
I changed this to the document.createElement approach, because that also made it easier to support shadow dom
| actions, | ||
| edited, | ||
| emojiPickerOpen, // Unused | ||
| emojiPickerOpen, |
There was a problem hiding this comment.
this was missing in shadcn and ariakit implementations
| actions, | ||
| children, | ||
| edited, | ||
| emojiPickerOpen, // Unused |
There was a problem hiding this comment.
fyi, this was not implemented in ariakit / shadcn, so decided to fix
| * elements (menus, toolbars) to escape overflow:hidden ancestors. | ||
| * Set by BlockNoteView; undefined in headless mode. | ||
| */ | ||
| public portalElement: HTMLElement | undefined; |
There was a problem hiding this comment.
Nope, I would not put this on the editor, much less public. Need to find a workaround here
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/mantine/src/BlockNoteView.tsx (1)
41-41:⚠️ Potential issue | 🟠 MajorCompose
portalRootRef; the current wiring turns it into a dead prop.Line 100 overrides any
portalRootRefthat comes in throughprops. Since this wrapper’s prop type is derived fromBlockNoteViewRaw, Mantine consumers can still passportalRootRef, but it never reaches the raw view anymore.🩹 Suggested fix
- const { className, theme, ...rest } = props; + const { className, theme, portalRootRef, ...rest } = props;- const applyThemeVariables = useCallback( - (node: HTMLDivElement | null) => { + const applyThemeVariables = useCallback( + (node: HTMLElement | null) => { if (!node) { return; }- const portalRef = useCallback( - (node: HTMLDivElement | null) => { + const portalRef = useCallback( + (node: HTMLElement | null) => { if (!node) { return; } node.setAttribute("data-mantine-color-scheme", finalTheme); applyThemeVariables(node); }, [applyThemeVariables, finalTheme], ); + + const mergedPortalRootRef = useCallback( + (node: HTMLElement | null) => { + portalRef(node); + if (typeof portalRootRef === "function") { + portalRootRef(node); + } else if (portalRootRef) { + portalRootRef.current = node; + } + }, + [portalRef, portalRootRef], + );- portalRootRef={portalRef} + portalRootRef={mergedPortalRootRef}Also applies to: 79-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mantine/src/BlockNoteView.tsx` at line 41, BlockNoteView currently overwrites any incoming portalRootRef from props, dropping consumer refs; change the wiring so you merge the incoming portalRootRef prop with the component's internal ref instead of overriding it. In practice: when destructuring props (const { className, theme, portalRootRef, ...rest } = props) retain portalRootRef and use a merged ref utility (e.g., useMergedRef/useRef-merge) to combine the external portalRootRef with the internal portal root ref, then pass that merged ref into BlockNoteViewRaw (and any internal portalRootRef usages) so consumers' refs are preserved.
♻️ Duplicate comments (2)
packages/react/src/editor/BlockNoteView.tsx (2)
187-195:⚠️ Potential issue | 🟠 MajorAvoid overwriting
classNameon externally suppliedportalRoot.For external roots, this effect replaces
classNamewholesale, which can clobber user-managed classes. That also conflicts with the prop docs saying caller handles theming.Suggested fix
useEffect(() => { editor.portalElement = internalPortalRoot; - internalPortalRoot.className = mergeCSSClasses( - "bn-root", - editorColorScheme, - className || "", - ); - internalPortalRoot.setAttribute("data-color-scheme", editorColorScheme); -}, [internalPortalRoot, editorColorScheme, className]); + if (!portalRoot) { + internalPortalRoot.className = mergeCSSClasses( + "bn-root", + editorColorScheme, + className || "", + ); + internalPortalRoot.setAttribute("data-color-scheme", editorColorScheme); + } +}, [internalPortalRoot, editorColorScheme, className, portalRoot]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/editor/BlockNoteView.tsx` around lines 187 - 195, The effect in useEffect overwrites the external portal root's className, clobbering caller-managed classes; update the logic in the useEffect that sets editor.portalElement and manipulates internalPortalRoot so it preserves any existing classes on internalPortalRoot (and on the external portal passed in) instead of replacing them—e.g., read the element's current className/classList and merge it with mergeCSSClasses(editorColorScheme, className, existingClasses) or use classList.add to append the theme and prop classes; keep references to editor.portalElement, internalPortalRoot, mergeCSSClasses, editorColorScheme, and className when implementing the non-destructive class update.
172-175:⚠️ Potential issue | 🟠 MajorGuard
document.createElementin render for SSR paths.Creating the fallback portal element during render (
document.createElement) will throw whendocumentis unavailable (server render). Please make this client-guarded and handle an undefined internal root until mounted.#!/bin/bash # Verify SSR-sensitive usage in BlockNoteView rg -n 'document\.createElement|document\.body' packages/react/src/editor/BlockNoteView.tsx -C2 rg -n '"use client"' packages/react/src/editor/BlockNoteView.tsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/editor/BlockNoteView.tsx` around lines 172 - 175, The useMemo that creates internalPortalRoot (portalRoot ?? document.createElement("div")) runs during render and will throw in SSR; change to defer creation to the client by initializing internalPortalRoot as undefined (or portalRoot) in state and only creating/appending the fallback div inside a useEffect when running on the client (i.e., when BlockNoteView mounts), cleaning it up on unmount; update any consumers to handle an undefined internalPortalRoot until the effect runs.
🤖 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/react/src/editor/BlockNoteView.tsx`:
- Around line 197-211: internalRef currently treats a null element as falsy and
falls through to appending internalPortalRoot to document.body; change the logic
so that when internalRef is called with element === null (unmount) you
remove/detach internalPortalRoot (call internalPortalRoot.remove()) instead of
appending, and only perform the append when element is non-null and portalRoot
is falsy; update the callback for internalRef (and use the existing variables
portalRoot, internalPortalRoot and the root variable) to early-return for
portalRoot, remove on null element, otherwise compute root (ShadowRoot vs
document.body) and append the internalPortalRoot.
---
Outside diff comments:
In `@packages/mantine/src/BlockNoteView.tsx`:
- Line 41: BlockNoteView currently overwrites any incoming portalRootRef from
props, dropping consumer refs; change the wiring so you merge the incoming
portalRootRef prop with the component's internal ref instead of overriding it.
In practice: when destructuring props (const { className, theme, portalRootRef,
...rest } = props) retain portalRootRef and use a merged ref utility (e.g.,
useMergedRef/useRef-merge) to combine the external portalRootRef with the
internal portal root ref, then pass that merged ref into BlockNoteViewRaw (and
any internal portalRootRef usages) so consumers' refs are preserved.
---
Duplicate comments:
In `@packages/react/src/editor/BlockNoteView.tsx`:
- Around line 187-195: The effect in useEffect overwrites the external portal
root's className, clobbering caller-managed classes; update the logic in the
useEffect that sets editor.portalElement and manipulates internalPortalRoot so
it preserves any existing classes on internalPortalRoot (and on the external
portal passed in) instead of replacing them—e.g., read the element's current
className/classList and merge it with mergeCSSClasses(editorColorScheme,
className, existingClasses) or use classList.add to append the theme and prop
classes; keep references to editor.portalElement, internalPortalRoot,
mergeCSSClasses, editorColorScheme, and className when implementing the
non-destructive class update.
- Around line 172-175: The useMemo that creates internalPortalRoot (portalRoot
?? document.createElement("div")) runs during render and will throw in SSR;
change to defer creation to the client by initializing internalPortalRoot as
undefined (or portalRoot) in state and only creating/appending the fallback div
inside a useEffect when running on the client (i.e., when BlockNoteView mounts),
cleaning it up on unmount; update any consumers to handle an undefined
internalPortalRoot until the effect runs.
🪄 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: efb3b186-3b7b-4f49-8922-4942f9845301
📒 Files selected for processing (10)
docs/content/docs/react/styling-theming/overriding-css.mdxpackages/ariakit/src/style.csspackages/core/src/editor/managers/ExtensionManager/extensions.tspackages/core/src/extensions/SideMenu/SideMenu.tspackages/mantine/src/BlockNoteView.tsxpackages/mantine/src/popover/Popover.tsxpackages/react/src/components/Popovers/GenericPopover.tsxpackages/react/src/editor/BlockNoteContext.tspackages/react/src/editor/BlockNoteView.tsxpackages/shadcn/src/popover/popover.tsx
💤 Files with no reviewable changes (1)
- packages/ariakit/src/style.css
✅ Files skipped from review due to trivial changes (2)
- packages/core/src/extensions/SideMenu/SideMenu.ts
- docs/content/docs/react/styling-theming/overriding-css.mdx
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/core/src/editor/managers/ExtensionManager/extensions.ts
- packages/react/src/editor/BlockNoteContext.ts
- packages/mantine/src/popover/Popover.tsx
…itor class and remove redundant portal props from BlockNoteView.
nperez0111
left a comment
nperez0111
left a comment
There was a problem hiding this comment.
cool that did simplify things
YousefED
changed the title
3 participants
Summary
Floating UI elements (menus, toolbars, emoji picker, etc.) are now portaled to
document.body, preventing clipping byoverflow: hiddenancestors.Closes #2543
Closes #2544
Closes #2558
Closes #2578
Supersedes the approach from #2092
Rationale
When BlockNote is inside a container with
overflow: hidden(sidebar, modal, scrollable panel), floating UI gets clipped. Floating UI docs confirm portaling is the only reliable fix. This is also the default behavior in libraries like Base UI.Changes
Portal infrastructure
BlockNoteEditor.portalElement— lazily-createddivon the editor instance. Appended todocument.body(or ShadowRoot) bymount(), removed byunmount().BlockNoteEditor.isWithinEditor(element)— new method that checks if an element is inside the editor DOM or its portal container. Used by core extensions to correctly handle portaled elements.BlockNoteViewsyncs theming classes (bn-root, color scheme, user className) onto the portal element via effects.GenericPopoverwraps all floating elements with<FloatingPortal root={portalRoot}>.CSS:
bn-rootvsbn-containerbn-root: theming (CSS variables, fonts). Applied to both editor and portal containers.bn-container: layout (width, height). Editor container only..bn-containerto.bn-root.Popover
portalRoot(for EmojiPicker)portalRootto genericPopover.Rootinterface.withinPortal+portalProps.portalRootvia React context, usescreatePortal.portalRootvia React context, nativeportalElementprop.Core extension fixes
editor.isWithinEditor()so hovering portaled floating elements doesn't dismiss the side menu.isWithinEditoroption to detect internal drags from the portaled drag handle, preventing block IDs from being regenerated on drag-drop.Z-index
z-index: 10000removed from Mantine popover and ariakit styles when portaling.--bn-ui-base-z-indexon.bn-rootremains available for user customization.EmojiPicker simplification
createPortal— passesportalRoottoPopover.Rootinstead.em-emoji-pickerstyles to.bn-root, removed stalez-index: 11000.Impact
.bn-rootinstead of.bn-container..bn-containernow only wraps the editor itself, while.bn-rootwraps both the editor and its floating UI (menus, toolbars). Layout related properties (width, height, margin) should still target.bn-container.Testing
overflow: hiddencontainers.Checklist