Skip to content

Grid interactivity stack: review findings #85295

Open
Open
Grid interactivity stack: review findings#85295

Description

@draganescu
draganescu
opened on Oct 21, 2026

Review findings for the manual grid interactivity stack (#85397): #84499 → #84504 → #84500 → #84501 → #84502 → #84503 → #84509 → #84512 → #84894 → #84947 → #85210, plus the side PR #84505 that #84512 merges.

Each PR was reviewed against the PR it sits on, not against trunk. #84512 was reviewed against a merge of its base with #84504 and #84505, so the side PRs' code isn't counted twice. Categories: performance and multiple grids, public API, couplings between framework areas, could-be-its-own-package, alternative routes, conventions, contracts with 3rd parties, and core backports. Categories with nothing to report are listed under "No findings" for each PR. Severity is shown as [high], [medium] or [low]. Nits were left out.

Top priorities

Across the stack

#84499 — Grid: Give manual placement grids same-size cells

Public API

  • [medium] --wp--style--grid-cells: fixed|auto is a new front-end property that blocks are told to read with container style queries, so third-party blocks will copy it. It also sits in --wp--style--*, which so far only holds theme.json values. Decide whether it's public; if it is, document it as experimental.

Alternative routes

  • [medium] aspect-ratio together with min-height: 0 and 1fr rows means content never makes a cell taller. Text taller than its cell spills over the next row and past the grid, which is worst on phones (stacking is opt-in). Keep min-height: auto, or clip or scroll overflowing cells.

Conventions

  • [medium] This is the first experiment check in lib/block-supports/ (see "Across the stack"). Done: see "Across the stack" (ad07869).

Contracts with 3rd parties

  • [medium] With the experiment on, every saved manual grid changes layout, including core's navigation overlay pattern. Graduating needs a migration or an opt-in, not a default flip.

Core backports

  • [low] Add a "No Core Sync Required" note for now. At graduation, backport the layout.php changes without the experiment check. Done: labelled, with a note in the PR description.

No findings: performance and multiple grids, couplings, own package.

#84504 — Blocks: Fill fixed-size cells in manual placement grids

Public API

  • [medium] The fill rules ship ungated in block-library's front-end style.css and reach core through the package sync, while the PHP that sets the variable stays plugin-only. In core, --wp--style--grid-cells: fixed becomes a switch any theme can flip.

Couplings

  • [medium] Five blocks each hardcode .is-layout-grid > and the private variable, and they hide controls inconsistently. Media & Text and Video keep min height; Cover keeps aspect ratio, which its CSS then cancels. Hide the block-support dimension controls in one place, block-editor's dimensions hook. Done for the controls: the dimensions hook turns off the minimum height and aspect ratio block-support controls in fixed cells, and Cover still hides its own minimum height control, resize handle and full height toggle, which aren't block supports, in f7af3db. The five blocks' CSS still names the variable.

Alternative routes

  • [medium] Hiding DimensionsTool on Image and Post Featured Image also hides Scale, and the focal point control needs Scale. Cropped images can therefore never get a focal point. Keep Scale and focal point; hide only width, height and aspect ratio. Done: in fixed cells Image keeps Scale and Focal point (picking a focal point sets the scale to cover) and Post Featured Image keeps Scale; DimensionsTool shows Scale whenever it's the only tool, in f7af3db.

Conventions

  • [low] This is the first @container style() in the repo, and the rules rely on !important to beat inline styles. Explain both in the PR.

Contracts with 3rd parties

  • [medium] Check Firefox support for custom-property style queries. Without it, the grid becomes a fixed box but media doesn't fill it and overflows, while the editor still hides the size controls.

No findings: performance and multiple grids, own package, core backports.

#84505 — Block Supports: Add an experimental rotate block support

Public API

  • [high] supports.rotate is on by default and stores style.rotate and per-viewport copies in post content, which is permanent.
  • [medium] style.rotate is a top-level key, unlike style.dimensions.*. Consider style.transform.rotate while it's experimental.
  • [medium] There's no theme.json setting to turn the control off, and no styles.*.rotate for Global Styles.

Couplings

  • [low] hooks/style.tsx imports hasRotateSupport, so the shared style hook knows about one specific support.

Alternative routes

  • [medium] Make it opt-in, like the other visual supports. Rotation needs a single real wrapper on the front end, which is why 8 core blocks had to opt out. The grid's need can be met from the child-layout support instead.

Conventions

  • [low] The angle picker rounds to whole degrees but storage keeps two decimals, so a canvas angle of 12.5° is rewritten as 13°.

Contracts with 3rd parties

  • [medium] rotate.php:187 and the editor useBlockProps run with the experiment off (see "Top priorities"). Gate both on the experiment, or go opt-in. Done in 24161b5: lib/load.php removes the filter and the support's hasSupport is isRotateEnabled().
  • [medium] Third-party dynamic blocks without a single wrapper get the rotate class on the first tag in their markup, so the editor and front end differ.

Core backports

  • [high] It's labelled "No Core Sync Required" but adds rotate.php and a safe_style_css entry that core must ship. Drop the label and add a backport-changelog/7.2 entry. Done: label removed, Editor: Add the rotate block support wordpress-develop#14707 and backport-changelog/7.2/14707.md.
  • [medium] Add a guarded remove_filter for a future core wp_render_rotate_support, as custom-css.php and block-visibility.php do, to avoid duplicate rules.

No findings: performance and multiple grids, own package.

#84500 — Grid: Snap drags to cells in manual placement grids

Performance and multiple grids

  • [medium] The per-dragover measuring starts here (see "Across the stack"). Done: see "Across the stack" (5d81c33).
  • [medium] Switching the popover slot when a drag starts unmounts and remounts every manual grid's visualizer at drag start and end. That remount is the root cause of the later z-index 31 workarounds. Done: the visualizer stays in its slot, in f76e3b3.
  • [low] Nested manual grids put both drop layers in the same slot at the same z-index, so which one gets the drop depends on mount order.

Couplings

Alternative routes

Conventions

  • [high] Multi-selection drag bug (see "Top priorities"). Add ! isMultiSelectionDrag. Done in 2504a6c.
  • [low] -webkit-user-drag: none applies to every child, and to images at any depth inside the grid. Done: it only matches the item's own image (> img, > figure img), in c11149e.

Contracts with 3rd parties

  • [medium] useDropZone's onDragStart can now fire from a dragover. Combined with the end-on-mousemove fallback, a drag could start and end on every tick in browsers that send mousemove during a drag. Verify in Safari and Firefox, and update the hook's docs. Partly: a mousemove with a button held no longer ends the drag (de13468). drag-repeat passes in Playwright WebKit too, which also skips the dragenter, and the hook's docs are updated in Compose: Start useDropZone drags on the document's first dragover #85412 (6969911, d183b00). Playwright's Firefox didn't launch, so Firefox and Safari still need a check by hand.

No findings: public API (other than the useDropZone change), own package, core backports.

#84501 — Grid: Show alignment guides while dragging in manual grids

Alternative routes

  • [low] Hiding cell lines when idle is a UX change bundled into a guides PR. Split it out or confirm it with design.

No findings: performance and multiple grids (beyond the shared dragover cost), public API, couplings, own package (keep get-alignment-guides.ts private until something outside grid uses it), conventions, contracts with 3rd parties, core backports.

#84502 — Grid: Add and remove rows of manual placement grids

Couplings

  • [low] Makes the import cycle direct (see "Across the stack"). Done: see "Across the stack" (8947909).

Alternative routes

  • [medium] Consider discrete "Add row above / below" and "Remove empty row" buttons instead of a full-grid resize box. They're keyboard-accessible by default and reuse the same update helper.

Conventions

  • [medium] Adding or removing rows at the top (which shifts every block) has no keyboard path, and the "N rows" label isn't announced to screen readers. Partly: the new row count is announced with speak() when a row resize ends, in 89d4e2e. Still no keyboard path for rows at the top.

Contracts with 3rd parties

No findings: performance and multiple grids (updates are correctly batched into one action), public API, own package, core backports.

#84503 — Grid: Draw blocks across cells in manual placement grids

Note: #85277 would remove the marquee, the cell appenders and the touch two-tap, which makes some of these moot.

Performance and multiple grids

  • [medium] useGridLayoutSync is mounted for every grid, experiment on or off, and reads the whole style of the selected block. Any style slider on any block re-renders every grid, and each one re-reads all its children. Only select when the selected block is this grid's child, and only its layout entries. Done: it selects only when the selected block is this grid's item, and then only that item's default and viewport layouts, in 2e017b8. The hook is still mounted for every grid.
  • [low] Every manual-grid child injects the same placeholder CSS rule, so 100 children means 100 copies. Done: one rule in the grid's layout style (<grid> > * > .components-placeholder), in 87a1573.

Public API

  • [low] Every inserter insertion adds meta.isFromInserter, ungated. Since meta is free-form, that becomes a de facto contract. Use a private key from private-keys. Done in 073fd4d (isFromInserterKey).

Couplings

  • [medium] Layout code detects placeholders through the @wordpress/components class .components-placeholder, and the 300px minimum is tuned to Image and Cover placeholders.

Alternative routes

Conventions

  • [medium] On touch, the first tap is swallowed, so a VoiceOver user's first activation does nothing.
  • [medium] Closing the marquee's inserter without inserting sends focus to body.

Contracts with 3rd parties

No findings: own package, core backports.

#84509 — Grid: Stack manual grids on mobile, and edit them from the stack

Performance and multiple grids

  • [low] Three components each select all children's full block objects and recompute the stacked layout, so typing in any grid item re-renders all three. Share one memoized private selector. Done in 6997076: private selector getStackedGridLayouts, memoized on each child's style and block visibility.

Public API

  • [high] layout.stackOnMobile and style['@mobile'].layout.stackOnMobile are two homes for one permanent flag, with precedence and reset rules between them. Pick one location and a viewport-neutral name.
  • [medium] The first mobile edit writes a @mobile placement into every child, which stays as dead data if stacking is turned back on. Done in 6997076: turning stacking back on removes the blocks' mobile placements in the same undo step (Grid: Drag and resize blocks freely in manual grids, Shift to snap #85210 also removes a mobile free-placement box).
  • [medium] --wp--grid-item--row-span opens a new namespace (see "Across the stack").

Couplings

  • [medium] The generic layout hook hardcodes viewport === '@mobile', and LayoutType gains a method named after one viewport. Pass the viewport into getLayoutStyle instead. Done in 6997076: getLayoutStyle receives viewport; the grid adds its stacking rules for @mobile; getMobileStackingStyle is gone.
  • [medium] With Mobile preview on and Responsive editing off, the canvas is stacked but the tools edit desktop placements.

Alternative routes

  • [medium] The row-span property looks like it has no visible effect, because stacked rows are auto. Verify in a browser; if confirmed, drop it, along with the per-child class and the PHP early-return change. Verified in Chromium and WebKit: it doesn't keep blocks tall (only pads short blocks by the row gaps), but the editor's tools read the rows the browser draws, and the stacked layout numbers rows by row span, so dropping it would misalign them. Kept; comments and the PR description now say what it does (6997076).

Conventions

  • [low] The first mobile edit silently turns stacking off for the whole grid. Announce it with a notice or speak(). Done in 6997076: snackbar "Stack on mobile turned off for this grid."

Contracts with 3rd parties

  • [low] Child classes change for existing manual grids (experiment-only).

Core backports

  • [low] At graduation: the stacking rules, the row-span property if kept, and the early-return change. The current label is correct.

No findings: own package.

#84512 — Grid: Rotate manual grid items from the canvas

Performance and multiple grids

Public API

  • [low] It emits a new front-end rotate: none on children of stacked grids, and unstacking writes @mobile.rotate: 0 into content.

Couplings

  • [medium] mobile-stacking.ts (pure data) now imports the React rotate hook module. Move the pure helpers into utils/rotation.ts. Done in a48973c, differently: unstacking unrotates blocks in getGridChildUpdates, so mobile-stacking.ts no longer imports the rotate hook module (the store's new selector reads it). Since 5617881 (Block Supports: Add an experimental rotate block support #84505), getRotateForState and getUpdatedRotateStyle live in utils/rotation.ts, so getGridChildUpdates doesn't import the hook module either. cleanEmptyObject moved to utils/object.ts (d872067) so that module loads without the hook utilities.
  • [low] The layout support emits a property owned by the rotate support.

Own package

Alternative routes

  • [high] Resize on rotated blocks (see "Top priorities"). Convert pointer movement into the block's own axes, or hide the handles while the block is rotated. Done for now: handles hidden while rotated, in 1715b68.

Conventions

  • [medium] The keyboard path (the inspector Rotation control) doesn't unstack a stacked grid in the Mobile state, so edits there have no visible effect. Done: in the Mobile state of a stacked grid the control shows 0 and edits go through useUpdateGridChildLayout, which turns stacking off first, like the handle, in c68bb45.

Contracts with 3rd parties

  • [low] rotate: none on stacked children is gated only by the grid experiment, and overrides any theme or plugin rotate CSS.

Core backports

#84894 — Grid: Show block toolbars over manual grids and add a drag handle

Performance and multiple grids

  • [low] On every pointer move of a touch drag, each manual grid runs its own elementsFromPoint(), and the subscription re-subscribes on every render. Done: the touch drag looks up the drop layer under the touch once and passes it to every grid, and the visualizer subscribes once with a stable useEvent callback, in 14b9aa5.
  • [low] The touch pub/sub is a module-level Set shared by every editor on the page.

Public API

  • [low] The CHANGELOG presents capturesToolbars() as an extension point, but third parties can't register layout types. Done: the entry no longer mentions capturesToolbars(); it says a block can opt out with __experimentalCaptureToolbars: false, in 14b9aa5.

Couplings

  • [medium] It grows the grid branch in the generic drag handler (see "Across the stack"). Done: see "Across the stack" (bedba56).

Alternative routes

  • [medium] Derive capture in useInnerBlocksProps (|| ( isManualPlacement && experiment )) instead of adding a LayoutType method.

Conventions

  • [low] A handle drag is detected by target !== blockElement, so a drag from an image inside the block counts too. Pass an explicit isHandle. Done: startGridItemDrag takes an isHandle option, which only the grid item drag handle sets, in 14b9aa5.

Contracts with 3rd parties

  • [medium] The layout overrides a block's explicit __experimentalCaptureToolbars: false, and the result shows up in getBlockListSettings. Done: a block's own setting now wins, and the layout only applies when the block doesn't set one. useInnerBlocksProps used to send false for every block that hadn't opted in, so it now sends true or nothing, in 14b9aa5.
  • [medium] Because the topmost capturing ancestor wins, it reaches blocks at any depth. A Paragraph inside a Group inside a full-page grid gets its toolbar at the top of the grid.
  • [low] Multi-selection drags in a manual grid now carry only the pressed block's ID. Done: multi-selection drags no longer go through startGridItemDrag, so they carry every selected ID (2504a6c, merged into this PR).

No findings: own package, core backports.

#84947 — Grid: Show the canvas handles in a small toolbar, and add a Canvas block

Public API

  • [medium] group-canvas is a public variation: it appears in the inserter, transforms and getActiveBlockVariation. Its name and title will be relied on, so get design sign-off before it graduates.

Couplings

  • [low] variations.js reads window.__experimentalEnableGridInteractivity at module load, which depends on script order. Register the variation from the experiment's init. Done: the Group block's init calls registerCanvasVariation(), which reads the flag when the block is registered, in 846237a.

Alternative routes

  • [medium] Grid: Show block toolbars over manual grids and add a drag handle #84894 moved the toolbar off the block to avoid covering neighbours; the corner toolbar covers neighbouring cells again. Consider a BlockControls group in the captured toolbar instead.
  • [low] The Delete button duplicates the options menu and sits next to the drag handle, where a touch drag can easily hit it.

Conventions

Contracts with 3rd parties

  • [low] With the experiment on, existing manual grids show as "Canvas" instead of "Grid".

No findings: performance and multiple grids, own package, core backports.

#85210 — Grid: Drag and resize blocks freely in manual grids, Shift to snap

Performance and multiple grids

  • [medium] The drop target changes on almost every pixel, so the layer re-renders on almost every dragover, and the next read forces a synchronous layout. Cache the measurements and move only the preview element, once per animation frame. Partly: the measurements are read once per drag (58d798d). The layer still re-renders when the target changes.

Public API

  • [high] style.layout.box is a permanent content shape. top is a fraction of the cell area's width, only because CSS resolves margin-top percentages against width. left is physical, not logical start (RTL). Fix the shape before it ships.
  • [medium] It adds four --wp--grid-item--box-* properties (see "Across the stack").
  • [low] Validation in JS and PHP is loose: it accepts left up to 100 and doesn't check left + width <= 1.

Couplings

  • [low] The column-count remap measures the canvas DOM, so the saved fractions depend on the canvas width at the time of the edit (desktop vs tablet preview).

Alternative routes

  • [high] Consider a separate canvas or freeform layout type, or absolutely positioned grid items whose percentages resolve against the grid area. The current model only works with fixed cells, and needs the remap, the !important margins and the top-of-width quirk.
  • [medium] The remap turns every snapped block into a free box, so a tidy layout stays unsnapped after a column change.

Conventions

  • [high] Free position and size are pointer-only: no keyboard nudge, no indication in the inspector, no snap action, and typing a span silently deletes the box (WCAG 2.5.7).
  • [medium] Touch drags are always free, so touch users can't snap.

Contracts with 3rd parties

  • [high] Per-keystroke column remap (see "Top priorities"). Apply it on commit (blur or Enter). Done in 2c87758.
  • [medium] RTL: physical margin-left is combined with logical justify-self: start, so the offset is ignored in RTL.
  • [medium] The !important margins override user margins, while blocks with their own width or height keep them, so the result matches neither the editor nor the stored box.
  • [medium] Reset, switching the grid type, global styles and third-party updates all change the column count without the remap.

Core backports

No findings: own package.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions