-
-
Notifications
You must be signed in to change notification settings - Fork 708
fix: portal floating UI elements to document.body to prevent overflow clipping BLO-1115 #2591
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
50ebffb
11bdffd
1358d89
5756910
303eab2
eaae887
0d114d6
6550e46
670dffc
c6fbfec
73d3e51
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,3 @@ | ||
| .bn-container[data-changing-font-demo] .bn-editor * { | ||
| .bn-root[data-changing-font-demo] .bn-editor * { | ||
| font-family: "Comic Sans MS", sans-serif; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,10 @@ | ||
| /* Adds border and shadow to editor */ | ||
| .bn-container[data-theming-css-demo] .bn-editor * { | ||
| .bn-root[data-theming-css-demo] .bn-editor * { | ||
| color: blue; | ||
| } | ||
|
|
||
| /* Makes slash menu hovered items blue */ | ||
| .bn-container[data-theming-css-demo] | ||
| .bn-suggestion-menu-item[aria-selected="true"], | ||
| .bn-container[data-theming-css-demo] .bn-suggestion-menu-item:hover { | ||
| .bn-root[data-theming-css-demo] .bn-suggestion-menu-item[aria-selected="true"], | ||
| .bn-root[data-theming-css-demo] .bn-suggestion-menu-item:hover { | ||
| background-color: blue; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -714,6 +714,25 @@ export class BlockNoteEditor< | |
| return this.prosemirrorView?.dom as HTMLDivElement | undefined; | ||
| } | ||
|
|
||
| /** | ||
| * The portal container element at `document.body` used by floating UI | ||
| * elements (menus, toolbars) to escape overflow:hidden ancestors. | ||
| * Set by BlockNoteView; undefined in headless mode. | ||
| */ | ||
| public portalElement: HTMLElement | undefined; | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nperez0111 not sure about setting this on the editor. We need it for the UniqueID and SideMenu extensions though. wdyt?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope, I would not put this on the editor, much less public. Need to find a workaround here |
||
|
|
||
| /** | ||
| * Checks whether a DOM element belongs to this editor — either inside the | ||
| * editor's DOM tree or inside its portal container (used for floating UI | ||
| * elements like menus and toolbars). | ||
| */ | ||
| public isWithinEditor = (element: Element): boolean => { | ||
| return !!( | ||
| this.domElement?.parentElement?.contains(element) || | ||
| this.portalElement?.contains(element) | ||
| ); | ||
| }; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will have to change if portalElement isn't stored here |
||
|
|
||
| public isFocused() { | ||
| if (this.headless) { | ||
| return false; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,26 +20,6 @@ | |
| padding: 0; | ||
| } | ||
|
|
||
| /* | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this was unused, lingering code that I removed in this PR |
||
| bn-root should be applied to all top-level elements | ||
|
|
||
| This includes the Prosemirror editor, but also <div> element such as | ||
| Tippy popups that are appended to document.body directly | ||
| */ | ||
| .bn-root { | ||
| -webkit-box-sizing: border-box; | ||
| -moz-box-sizing: border-box; | ||
| box-sizing: border-box; | ||
| } | ||
|
|
||
| .bn-root *, | ||
| .bn-root *::before, | ||
| .bn-root *::after { | ||
| -webkit-box-sizing: inherit; | ||
| -moz-box-sizing: inherit; | ||
| box-sizing: inherit; | ||
| } | ||
|
|
||
| /* reset styles, they will be set on blockContent */ | ||
| .bn-default-styles p, | ||
| .bn-default-styles h1, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -612,9 +612,6 @@ export class SideMenuView< | |
| this.mousePos.y > editorOuterBoundingBox.top && | ||
| this.mousePos.y < editorOuterBoundingBox.bottom; | ||
|
|
||
| // TODO: remove parentElement, but then we need to remove padding from boundingbox or find a different solution | ||
| const editorWrapper = this.pmView.dom!.parentElement!; | ||
|
|
||
| // Doesn't update if the mouse hovers an element that's over the editor but | ||
| // isn't a part of it or the side menu. | ||
| if ( | ||
|
|
@@ -623,11 +620,8 @@ export class SideMenuView< | |
| // An element is hovered | ||
| event && | ||
| event.target && | ||
| // Element is outside the editor | ||
| !( | ||
| editorWrapper === event.target || | ||
| editorWrapper.contains(event.target as HTMLElement) | ||
| ) | ||
| // Element is outside this editor and its portaled UI | ||
| !this.editor.isWithinEditor(event.target as HTMLElement) | ||
|
Comment on lines
+623
to
+624
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure that this is correct. The TODO comment from before made it seem like it was intentionally using the parent element of the editor |
||
| ) { | ||
| if (this.state?.show) { | ||
| this.state.show = false; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,6 +51,9 @@ const UniqueID = Extension.create({ | |
| attributeName: "id", | ||
| types: [], | ||
| setIdAttribute: false, | ||
| isWithinEditor: undefined as | ||
| | ((element: Element) => boolean) | ||
| | undefined, | ||
| generateID: () => { | ||
| // Use mock ID if tests are running. | ||
| if (typeof window !== "undefined" && (window as any).__TEST_OPTIONS) { | ||
|
|
@@ -128,6 +131,7 @@ const UniqueID = Extension.create({ | |
| // view.dispatch(tr); | ||
| // }, | ||
| addProseMirrorPlugins() { | ||
| const { isWithinEditor } = this.options; | ||
| let dragSourceElement: any = null; | ||
| let transformPasted = false; | ||
| return [ | ||
|
|
@@ -228,14 +232,11 @@ const UniqueID = Extension.create({ | |
| // we register a global drag handler to track the current drag source element | ||
| view(view) { | ||
| const handleDragstart = (event: any) => { | ||
| let _a; | ||
| dragSourceElement = ( | ||
| (_a = view.dom.parentElement) === null || _a === void 0 | ||
| ? void 0 | ||
| : _a.contains(event.target) | ||
| ) | ||
| ? view.dom.parentElement | ||
| : null; | ||
| const editorParent = view.dom.parentElement; | ||
| const isFromEditor = | ||
| editorParent?.contains(event.target) || | ||
| isWithinEditor?.(event.target); | ||
| dragSourceElement = isFromEditor ? editorParent : null; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does dragging from a popover really matter? Why is this check here? |
||
| }; | ||
| window.addEventListener("dragstart", handleDragstart); | ||
| return { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fyi, this was not implemented in ariakit / shadcn, so decided to fix