fix: styles and e2e assumptions that relied on menus being nested in their trigger - #3057
Closed
YousefED wants to merge 4 commits into
Closed
fix: styles and e2e assumptions that relied on menus being nested in their trigger#3057YousefED wants to merge 4 commits into
YousefED wants to merge 4 commits into
Conversation
Menus and popovers are portalled into the editor container, so they are no longer descendants of the toolbar, side menu or table handle that opens them. Rules scoped through those ancestors stopped applying: - mantine: the 12px / 30px menu items (block type select, color menus, drag handle menu, table handle menu, comment actions) are now scoped on the dropdowns' own classes; the drag handle menu keeps its min-width. The `padding` and `position` it also set are already provided by the base dropdown rule and Mantine's own stylesheet, and the hover colour by the base item rule. - ariakit: menus and popovers get a z-index above the UI controllers' wrappers (`--bn-ui-base-z-index` + 10…90). Ariakit's stylesheet pins them at 50, which inline was inside the side menu's stacking context but as a sibling paints the drag handle over the colors submenu. The toolbar gap rule is dropped; it was not needed.
…enu by class Two e2e assumptions broke with portalled popovers: - `userEvent.click` releases the button within the same tick. The mantine toolbar's focus trap arms on mousedown and moves focus in a `setTimeout`; with a 0 ms press that lands after the link form's input has autofocused and steals focus back, so typing goes nowhere. Any real press (≥ ~40 ms) lets the input win, which is why it never reproduces manually. Add a `delay` to the positional mouse click and a `clickElement` helper that holds the button, and use it where a form popover is opened and typed into right away. - The drag handle menu is no longer a child of `.bn-side-menu`; match it by its own class.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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/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
@blocknote/xl-typst-exporter
commit: |
Mounting schedules an update from an effect that can commit after `act` has returned. On Linux WebKit whether it does varies from mount to mount, so the setup measured second sometimes counted one commit fewer or more than the baseline and the comparison failed (CI's webkit shard, 1 of 3 runs locally in Docker). Wait until no commit has landed for 50 ms before reading the counts, at mount and after the parent re-render, so every mount is measured once things have settled.
|
The removed toolbar gap rule also spaced the two fields of the edit-link form. Restore that for form popovers only, so lists keep the tighter spacing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #3052 (always-pass
portalElement): menus and popovers are portalled into the editor container, so three things that assumed they were DOM descendants of their trigger stopped holding..bn-toolbar/.bn-side-menu; now scoped on the dropdowns' own classes (.bn-select,.bn-menu-dropdown,.bn-table-handle-menu,.bn-drag-handle-menu). Measured identical computed styles tomobile-toolbar-demo(font, height, hover colour, padding, position, min-width).z-index: 50; as siblings of the UI controllers' wrappers (--bn-ui-base-z-index+ 10…90) the drag handle painted over the colors submenu. They now sit at base + 100. Mantine already uses 300; shadcn's vendoredz-50has the same numbers but no placement that overlaps today (left as is).userEvent.clickreleases within the same tick, which loses a mousedown/click focus race against the mantine toolbar's focus trap that no real press (≥ ~40 ms) can lose; the affected tests now click through a helper that holds the button. The drag handle menu selector matches the menu by its own class.Verified in Docker (chromium): ariakit, shadcn, comments, colors, draghandle and theming suites, 45/45.