chore: Move side menu height setting to FloatingUI #2224
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,9 @@ | ||
| import { BlockSchema, InlineContentSchema, StyleSchema } from "@blocknote/core"; | ||
| import { SideMenuExtension } from "@blocknote/core/extensions"; | ||
| import { size } from "@floating-ui/react"; | ||
| import { FC, useMemo } from "react"; | ||
|
|
||
| import { useBlockNoteEditor } from "../../hooks/useBlockNoteEditor.js"; | ||
| import { useExtensionState } from "../../hooks/useExtension.js"; | ||
| import { BlockPopover } from "../Popovers/BlockPopover.js"; | ||
| import { FloatingUIOptions } from "../Popovers/FloatingUIOptions.js"; | ||
|
|
@@ -11,6 +14,12 @@ export const SideMenuController = (props: { | |
| sideMenu?: FC<SideMenuProps>; | ||
| floatingUIOptions?: Partial<FloatingUIOptions>; | ||
| }) => { | ||
| const editor = useBlockNoteEditor< | ||
| BlockSchema, | ||
| InlineContentSchema, | ||
| StyleSchema | ||
| >(); | ||
|
|
||
| const state = useExtensionState(SideMenuExtension, { | ||
| selector: (state) => { | ||
| return state !== undefined | ||
|
|
@@ -29,6 +38,117 @@ export const SideMenuController = (props: { | |
| useFloatingOptions: { | ||
| open: show, | ||
| placement: "left-start", | ||
| middleware: [ | ||
| // Adjusts the side menu height to align it vertically with the | ||
| // block's content. In some cases, like file blocks with captions, | ||
| // the height and top offset is adjusted to align it with a specific | ||
| // element within the block's content instead. | ||
| size({ | ||
| apply({ elements }) { | ||
| // TODO: Need to fetch the block from extension, else it's | ||
|
Collaborator
There was a problem hiding this comment. this code is called in
Collaborator
Author
There was a problem hiding this comment. Even when changing the |
||
| // always `undefined` for some reason? Shouldn't the `apply` | ||
| // function get recreated with the updated `block` object each | ||
| // time it changes? | ||
| const block = | ||
| editor.getExtension(SideMenuExtension)?.store.state?.block; | ||
| if (block === undefined) { | ||
| return; | ||
| } | ||
|
|
||
| const blockElement = | ||
| elements.reference instanceof Element | ||
| ? elements.reference | ||
| : elements.reference.contextElement; | ||
| if (blockElement === undefined) { | ||
| return; | ||
| } | ||
|
|
||
| const blockContentElement = | ||
| blockElement.querySelector(".bn-block-content"); | ||
| if (blockContentElement === null) { | ||
| return; | ||
| } | ||
|
|
||
| const blockContentBoundingClientRect = | ||
| blockContentElement.getBoundingClientRect(); | ||
|
|
||
| // Special case for file blocks with captions. In this case, we | ||
| // align the side menu with the first sibling of the file caption | ||
| // element. | ||
| const fileCaptionParentElement = | ||
|
Collaborator
There was a problem hiding this comment. imo we can't have these special cases hardcoded here. The sidemenu should not be aware of our block types or their HTML structure. Can we come up with a more future-proof API? Also, is it easy to add e2e tests for this to prevent regressions?
Collaborator
Author
There was a problem hiding this comment. I'm not sure there's an alternative solution tbh. Either we make an exception for them, or things end up looking misaligned and wonky. I discussed an idea with @nperez0111 to maybe make it so that blocks that are taller than some height threshold have their side menus aligned to the top left corner of the block instead, but it wouldn't really address the issue for e.g. toggle blocks and audio blocks. The only solution I think makes sense is if you extract the logic in export type SideMenuException = (
elements: {
reference: ReferenceElement;
floating: FloatingElement;
}
) => { yOffset: number, height: number };
export function applySideMenuHeight(
elements: {
reference: ReferenceElement;
floating: FloatingElement;
},
exceptions?: SideMenuException[]
): void;Ofc naming and exact typing is tbd, but do you think this is worth implementing?
Collaborator
There was a problem hiding this comment. what about a classname or other child element selector that we can query for? (unless @nperez0111 can come up with something cleaner)? Imo most important is that this component should imo have no knowledge of different block types and their implementations
Contributor
There was a problem hiding this comment. I thought the logic would look like: const exceptions = [ ".file-caption", ".xyz" ]
let subtractFromHeight = 0;
exceptions.forEach(e=>{
subtractFromHeight += document.querySelector(e)?.getBoundingBox().height ?? 0
})But, it's unclear to me whether @matthewlipski is doing any sort of alignment of buttons or not. If it were just to subtract things from being counted as the height then either this, or just introducing a classname to put on any element we want to be discounted from the height would work like @YousefED mentioned. |
||
| blockContentElement.querySelector(".bn-file-caption") | ||
| ?.parentElement || null; | ||
| if (fileCaptionParentElement !== null) { | ||
| const fileElement = fileCaptionParentElement.firstElementChild; | ||
| if (fileElement) { | ||
| const fileBoundingClientRect = | ||
| fileElement.getBoundingClientRect(); | ||
|
|
||
| elements.floating.style.setProperty( | ||
| "height", | ||
| `${fileBoundingClientRect.height}px`, | ||
| ); | ||
| elements.floating.style.setProperty( | ||
| "top", | ||
| `${fileBoundingClientRect.y - blockContentBoundingClientRect.y}px`, | ||
| ); | ||
|
|
||
| return; | ||
| } | ||
| } | ||
|
|
||
| // Special case for toggleable blocks. In this case, we align the | ||
| // side menu with the element containing the toggle button and | ||
| // rich text. | ||
| const toggleWrapperElement = | ||
| blockContentElement.querySelector(".bn-toggle-wrapper"); | ||
| if (toggleWrapperElement !== null) { | ||
| const toggleWrapperBoundingClientRect = | ||
| toggleWrapperElement.getBoundingClientRect(); | ||
|
|
||
| elements.floating.style.setProperty( | ||
| "height", | ||
| `${toggleWrapperBoundingClientRect.height}px`, | ||
| ); | ||
| elements.floating.style.setProperty( | ||
| "top", | ||
| `${toggleWrapperBoundingClientRect.y - blockContentBoundingClientRect.y}px`, | ||
| ); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| // Special case for table blocks. In this case, we align the side | ||
| // menu with the table element inside the block. | ||
| const tableElement = blockContentElement.querySelector( | ||
| ".tableWrapper table", | ||
| ); | ||
| if (tableElement !== null) { | ||
| const tableBoundingClientRect = | ||
| tableElement.getBoundingClientRect(); | ||
|
|
||
| elements.floating.style.setProperty( | ||
| "height", | ||
| `${tableBoundingClientRect.height}px`, | ||
| ); | ||
| elements.floating.style.setProperty( | ||
| "top", | ||
| `${tableBoundingClientRect.y - blockContentBoundingClientRect.y}px`, | ||
| ); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| // Regular case, in which the side menu is aligned with the block | ||
| // content element. | ||
| elements.floating.style.setProperty( | ||
| "height", | ||
| `${blockContentBoundingClientRect.height}px`, | ||
| ); | ||
| elements.floating.style.setProperty("top", "0"); | ||
| }, | ||
| }), | ||
| ], | ||
| }, | ||
| useDismissProps: { | ||
| enabled: false, | ||
|
|
@@ -40,13 +160,17 @@ export const SideMenuController = (props: { | |
| }, | ||
| ...props.floatingUIOptions, | ||
| }), | ||
| [editor, props.floatingUIOptions, show], | ||
| ); | ||
|
|
||
| const Component = props.sideMenu || SideMenu; | ||
|
|
||
| return ( | ||
| <BlockPopover | ||
| blockId={show ? block?.id : undefined} | ||
| ignoreNestingOffset={true} | ||
| {...floatingUIOptions} | ||
| > | ||
| {block?.id && <Component />} | ||
| </BlockPopover> | ||
| ); | ||
|
|
||
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.
I'm not sure I like the use of
firstElementChild. It assumes a specific dom structure to get the dom element - which is not typesafe / futureproof.Better approach would be to use the methods we have like getblockinfo to get the content block, and then call getdomatpos for it
(example; the current approach would probably break for column blocks which don't have blockContent)