Block Toolbar: Add edge padding and stop disorienting flip jumps#77846
Draft
richtabor wants to merge 1 commit into
Draft
Block Toolbar: Add edge padding and stop disorienting flip jumps#77846richtabor wants to merge 1 commit into
richtabor wants to merge 1 commit into
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refines block toolbar positioning to add consistent edge padding and to prevent mid-edit “flip” jumps by switching from dynamic flip logic to a static placement rule (below only for the first top-level block; above for all others). It also extends Popover’s shift prop to support a configurable viewport-edge padding.
Changes:
- Extend
Popovershiftprop to accept{ padding }and plumb it into Floating UI’sshiftmiddleware. - Simplify block toolbar popover positioning logic to a static rule based on “first top-level block” status; add consistent
offsetandshift.paddingspacing. - Remove CSS margins on the contextual toolbar that would double-count spacing now handled via Floating UI middleware.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/components/src/popover/types.ts | Widens shift prop typing/docs to allow { padding }. |
| packages/components/src/popover/index.tsx | Uses optional shift.padding to configure shift middleware padding (default remains 1). |
| packages/block-editor/src/components/block-tools/use-block-toolbar-popover-props.js | Replaces dynamic flip/resize/scroll logic with static placement + offset/shift.padding. |
| packages/block-editor/src/components/block-tools/style.scss | Removes contextual toolbar vertical margins now replaced by middleware spacing. |
| packages/block-editor/src/components/block-tools/empty-block-inserter.js | Updates hook usage to new useBlockToolbarPopoverProps({ clientId }) signature. |
| packages/block-editor/src/components/block-tools/block-toolbar-popover.js | Updates hook usage to no longer pass contentElement. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+35
to
+42
| const isFirstTopLevelBlock = useSelect( | ||
| ( select ) => { | ||
| const { getBlockIndex, getBlockAttributes } = | ||
| const { getBlockRootClientId, getBlockIndex } = | ||
| select( blockEditorStore ); | ||
| return { | ||
| blockIndex: getBlockIndex( clientId ), | ||
| isSticky: hasStickyOrFixedPositionValue( | ||
| getBlockAttributes( clientId ) | ||
| ), | ||
| }; | ||
| return ( | ||
| ! getBlockRootClientId( clientId ) && | ||
| getBlockIndex( clientId ) === 0 | ||
| ); |
There was a problem hiding this comment.
The previous implementation treated blocks with style.position.type of sticky/fixed as a special-case to avoid placing the toolbar above a block that can be pinned to the top of the viewport (which can cause the toolbar to be shifted into view and overlap/obscure the block). This refactor removes that check entirely, so sticky/fixed blocks that are not the first top-level block will always get placement: 'top-start' with shift enabled. Consider reintroducing a sticky/fixed condition (e.g. return FIRST_BLOCK_PROPS or another dedicated set of props) so toolbars for sticky/fixed blocks don’t end up covering content when the block is stuck at the viewport top.
Contributor
There was a problem hiding this comment.
The sticky/fixed position check has been reintroduced in commit af9feb2. Blocks with style.position.type of sticky or fixed now also receive placement: 'bottom-start' via BELOW_BLOCK_PROPS, just like the first top-level block — so their toolbar renders below the block rather than above where it could overlap pinned content at the viewport top.
|
Size Change: -370 B (0%) Total Size: 7.87 MB 📦 View Changed
ℹ️ View Unchanged
|
…ng flip jumps
The block toolbar had two related rough edges:
1. When a block sat flush against the left or right of the editor frame,
the toolbar butted right up against the edge — no visual gap between it
and the surrounding chrome, which felt cramped and disorienting at a
glance.
2. The previous geometric flip logic recomputed placement on every block
resize and viewport change. When block height or scroll position
crossed a hidden threshold, the toolbar would suddenly teleport from
above the block to below it (or vice versa) mid-edit.
Replaces the dynamic flip with a static rule. The toolbar anchors below
the block when:
- It's the first top-level block — scrolling can't rescue the user from a
clipped toolbar when they're already at the canvas top, so this is the
only block that genuinely needs an alternate placement.
- The block has sticky/fixed positioning — it can pin to the viewport top
where a toolbar above would overlap or get clipped by editor chrome.
For all other blocks the toolbar anchors above as usual. The user can
scroll to create headroom if needed; the toolbar never decides to jump on
its own.
Adds 8px of consistent breathing room via Floating UI's `offset`
middleware (toolbar↔block) and a new `shift.padding` option on the shared
`Popover` (toolbar↔viewport edge). Removes redundant CSS margins on the
toolbar that were double-counting against these new values. Widens the
`Popover`'s `shift` prop to accept `{ padding }` so other callers can opt
into a larger viewport inset without changing the global default.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
99c4852 to
af9feb2
Compare
ciampo
reviewed
Apr 30, 2026
Contributor
ciampo
left a comment
There was a problem hiding this comment.
What if the first block is taller than the viewport? Showing the toolbar anchored below it would look unexpected and confusing, I'm not sure if it's a better UX.
This scenario can easily happen, for example, if the first block is a Group that internally has many blocks.
Comment on lines
+141
to
+150
| * | ||
| * Pass an object with a `padding` value (in px) to control the gap kept | ||
| * from the viewport edges before shifting kicks in (default `1`). | ||
| * | ||
| * _Note: The `resize` and `shift` props are not intended to be used together. | ||
| * Enabling both can cause unexpected behavior._ | ||
| * | ||
| * @default false | ||
| */ | ||
| shift?: boolean; | ||
| shift?: boolean | { padding?: number }; |
Contributor
There was a problem hiding this comment.
Instead of changing the current shift prop, we could add a new collisionPadding?: number prop, which will conveniently match the API surface of the Popover component from @wordpress/ui ?
Something like
/**
* The padding (in px) maintained from the collision boundary edges
* when `shift` is enabled. Defaults to `1` to avoid sub-pixel flicker
* at the viewport edge.
*
* @default 1
*/
collisionPadding?: number;
Member
There was a problem hiding this comment.
We should also just consider changing the baked-in shift value to 8. Doesn't seem like something that really needs to be customizable.
Contributor
There was a problem hiding this comment.
We will need to add a CHANGELOG entry for the @wordpress/components
Comment on lines
+12
to
+17
| // `offset` keeps the toolbar 8px ($grid-unit-10) off the block; | ||
| // `shift.padding` keeps it the same 8px off the viewport edges when shifting. | ||
| const COMMON_PROPS = { | ||
| placement: 'top-start', | ||
| flip: false, | ||
| offset: 8, | ||
| shift: { padding: 8 }, |
Contributor
There was a problem hiding this comment.
Let's try to keep this DRY-er, especially since it's already mirroring a Scss variable — maybe something like
const TOOLBAR_GAP = 8; // $grid-unit-10
const COMMON_PROPS = {
flip: false,
offset: TOOLBAR_GAP,
shift: { padding: TOOLBAR_GAP },
};
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
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.
Summary
The block toolbar had two related rough edges:
This PR replaces the dynamic flip with a static rule that makes the first block have the toolbar rendered below (because you can't scroll "up" to get it out of the way), while other blocks do not flip the toolbar:
Test plan
shift={true}and behave identically to before.Visuals
First block toolbar is below:
Other blocks persist the toolbar in its top location:
Both have appropriate spacing from the editor UI (header, inspector, list view etc).
🤖 Generated with Claude Code