feat: Remove useRegisterPortalRoot from ShadCN - #3054
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@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
commit: |
Summary
Generally, the components in the
ariakit,mantine, andshadcnpackages that are used for theComponentsContextare supposed to be "dumb" adapters for primitives like buttons, menus, etc. In #3046, this contract was broken somewhat as ShadCN components required the use ofuseEditorPortalElement. This is because unlike the other 2 UI libs, which render dropdowns as siblings of their trigger elements, ShadCN portals them tobodyby default. This doesn't work for us as that puts them outside container elements which provide additional theming and styling (basically the same issue #3046 addresses). Therefore, the default portal location was overridden to the return ofuseEditorPortalElement.To ensure consistency across UI libs then, this PR makes it so that the
portalRootprop, which tells an Ariakit/Mantine/ShadCN component if & where it should portal its dropdown to, is always passed the element returned byuseEditorPortalElement.Additionally, the
preventFocusOnOpenprop has been added. This is so that for mobile, we can disable dropdowns from grabbing focus from the editor on open, as this would close the virtual keyboard. But on desktop, this should be enabled for better accessibility. This was previously set based on ifportalRootwas defined, which isn't quite right. Yes, it works in our case because the only time we'd actually setportalRootwas for the mobile formatting toolbar. But there is no direct correlation between a dropdown needing to be portalled, and preventing focus from moving to a dropdown on open, so these 2 concerns have been separated.Rationale
See above.
Changes
See above.
Impact
N/A
Testing
E2E snapshots will need updates, but I'm holding off on this until we decide whether this change is smth we want.
Screenshots/Video
Checklist
Additional Notes