Repository navigation
feat: masonry layout option for post-template + grid blocks (#593) - #603
Merged
Merged
Conversation
Adds masonry as a first-class layout choice on the existing `artisanpack/post-template` and `artisanpack/grid` blocks (no new blocks). Native CSS Grid masonry (`grid-template-rows: masonry`) lights up automatically where the browser supports it via @supports; otherwise the public frontend runs a shared JS bootstrap that packs items shortest-column-first, with support for resize, mutation, image load, and `gridColumnSpan` for wider items. Implementation: - `post-template` gains `masonry` in the `layout` enum and a new toolbar button; columns control gains a masonry-specific help line. - `grid` gains a `layoutMode` (`fixed` | `masonry`) attribute exposed via a ToggleControl. The grid block publishes its layout mode to grid-item children via `providesContext`, and grid-items disable the row-span control with an explanatory hint when the parent is masonry. - Shared masonry module under `_shared/masonry/`: CSS (native @supports rule + JS-fallback positioning context) and `initMasonry()` controller with renderer-agnostic shortest-column-first packing. - Blade renderer ships `frontend/masonry.css` + `frontend/masonry- fallback.js` (plain-JS port of the controller) and wires both through `<x-ve-blocks-styles />`. React and Vue renderers emit the same `is-layout-masonry` / `ap-grid-layout-masonry` classes + `data-ap-cols` attribute so the bootstrap can target them. - Masonry layers `is-layout-grid` underneath `is-layout-masonry` so Gutenberg's bundled layout baseline provides `display: grid` inside the editor canvas iframe and the post-template grid CSS sets the per-breakpoint columns-N tracks. Tested: - 2253 JS tests pass (block.json shape, JS fallback packing / span / clamping / height / relayout / destroy / observers, React + Vue parity for both blocks). - 1323 PHP tests pass (Blade render output for both blocks + `<x-ve-blocks-styles />` wiring). - Renderer-parity script clean. Trade-off documented in the editor's edit components: the JS fallback is not run inside Gutenberg's canvas because freshly-inserted grid-items are empty (height 0) and the post-template streams content across frames via `BlockContextProvider`. Non-native browsers see a regular columned grid in the canvas as a preview approximation; Chrome 142+ shows true masonry packing via @supports. The public frontend's bootstrap packs items for every browser, so the published page is always masonry. Closes #593
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds ChangesMasonry Layout Feature
Sequence Diagram(s)sequenceDiagram
participant GridEdit
participant BlockContext
participant GridItemEdit
participant Renderer
participant Browser
participant initMasonry
GridEdit->>GridEdit: layoutMode set to masonry
GridEdit->>BlockContext: providesContext artisanpack/gridLayoutMode=masonry
BlockContext->>GridItemEdit: usesContext artisanpack/gridLayoutMode
GridItemEdit->>GridItemEdit: isMasonryParent=true, disable row-span
GridEdit->>Renderer: render div.ap-grid-layout-masonry data-ap-cols=N
Renderer->>Browser: emit HTML with masonry classes and data attributes
Browser->>initMasonry: DOMContentLoaded triggers bootstrap()
initMasonry->>initMasonry: supportsNativeMasonry() checks CSS.supports (cached)
alt native support available
Browser->>Browser: CSS grid-template-rows: masonry applies
else fallback needed
initMasonry->>Browser: add .ap-masonry-js-fallback class
initMasonry->>Browser: position items absolutely via findSlot()
initMasonry->>Browser: wire ResizeObserver and MutationObserver
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/visual-editor-renderer-blade/resources/assets/frontend/masonry-fallback.js`:
- Around line 298-314: The destroy() function cleans up event listeners,
ResizeObserver, and MutationObserver, but it fails to remove the
__apMasonryController marker that was added to the container element during
initialization. This causes subsequent bootstrap() calls to skip
reinitialization because they detect the stale marker and assume the controller
already exists. At the end of the destroy() function, after removing classes
from the container and before the function exits, delete the
__apMasonryController property from the container element to ensure the marker
is properly cleared.
- Around line 169-173: The `relayout()` function always reads columns from a
single data attribute without considering the current viewport breakpoint. To
fix this, enhance the column reading logic to detect the current breakpoint and
read the appropriate breakpoint-specific column value from the container's data
attributes instead of always using readColumnsFromAttr with the default value.
This ensures the masonry fallback respects responsive column overrides that the
renderer provides for different screen sizes.
In `@packages/visual-editor-renderer-react/src/blocks/core/query.tsx`:
- Around line 65-66: The columns variable assignment needs to validate and
constrain the numeric input to a reasonable range, as it currently accepts
invalid values like NaN, Infinity, or fractional numbers that produce incorrect
layout classes and unreliable masonry behavior. Modify the columns assignment to
clamp the value to a valid integer range (for example, between 1 and some
reasonable maximum like 12) after the type check, ensuring that the columns
value used in the "columns-*" class names and "data-ap-cols" attribute on the
subsequent lines is always a valid, whole number within acceptable bounds.
In `@packages/visual-editor-renderer-vue/src/blocks/core/query.ts`:
- Around line 66-67: The columns assignment on line 66 does not validate that
the numeric value is finite and within acceptable bounds, which can result in
invalid CSS class names and data attributes when the value is NaN, Infinity, or
a fractional number. Add normalization logic to the columns variable assignment
that ensures the value is a finite integer within a reasonable range (e.g.,
between 1 and some maximum like 12). Apply the same normalization logic wherever
columns is used for class or data attribute generation, particularly in the
areas around line 86 where data-ap-cols is set, to ensure consistent and valid
output across all usages.
In
`@resources/js/visual-editor/blocks/_shared/masonry/__tests__/fallback.test.ts`:
- Around line 80-82: The afterEach hook currently only restores mocks but does
not clean up fallback-controller instances that are created during tests,
leaving attached observers and window listeners alive across test runs. Maintain
a collection (such as an array) to track all fallback-controller instances
created throughout the test suite, and in the afterEach hook after calling
vi.restoreAllMocks(), iterate through this collection and call destroy() on each
controller instance to properly clean up their observers and listeners, then
clear the collection for the next test.
In `@resources/js/visual-editor/blocks/_shared/masonry/fallback.ts`:
- Around line 231-234: In the loop that iterates through placements and restores
styles for removed elements (when not in liveSet), you need to unobserve the
removed elements from the ResizeObserver to prevent accumulation of detached
targets. After calling restoreStyle on the placement.el element that is not in
liveSet, add a corresponding unobserve call to the ResizeObserver instance for
that element. Apply this fix in all three locations mentioned (the initial block
around lines 231-234, and the similar patterns at lines 297-308 and 364-365) to
ensure removed elements are properly cleaned up from observation and don't
continue consuming observer resources.
In `@resources/js/visual-editor/blocks/_shared/masonry/masonry.css`:
- Around line 49-52: The gap property defined for
.wp-block-post-template.is-layout-masonry at line 49 is restricted to only apply
when the .ap-masonry-js-fallback class is not present (via the
:not(.ap-masonry-js-fallback) selector). When the fallback is activated and this
class is added, the gap declaration no longer applies, causing readGapPx() in
fallback.ts to return 0 and resulting in no spacing. Remove the
:not(.ap-masonry-js-fallback) negation from the selector so the gap property
applies in both the normal and fallback states, or ensure the gap is also
defined in the fallback CSS block around line 76-85 to maintain consistent
spacing regardless of fallback activation state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 76221e54-1a64-43f3-83c5-210aa38b8508
📒 Files selected for processing (28)
packages/visual-editor-renderer-blade/resources/assets/frontend/masonry-fallback.jspackages/visual-editor-renderer-blade/resources/assets/frontend/masonry.csspackages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/grid.blade.phppackages/visual-editor-renderer-blade/resources/views/blocks/core/post-template.blade.phppackages/visual-editor-renderer-blade/resources/views/components/blocks-styles.blade.phppackages/visual-editor-renderer-blade/src/View/Components/BlocksStylesComponent.phppackages/visual-editor-renderer-blade/tests/Feature/MasonryLayoutRenderTest.phppackages/visual-editor-renderer-react/src/blocks/artisanpack/grid.tsxpackages/visual-editor-renderer-react/src/blocks/core/query.tsxpackages/visual-editor-renderer-react/tests/BlockTree.test.tsxpackages/visual-editor-renderer-react/tests/PostTemplateMasonry.test.tsxpackages/visual-editor-renderer-vue/src/blocks/artisanpack/grid.tspackages/visual-editor-renderer-vue/src/blocks/core/query.tspackages/visual-editor-renderer-vue/tests/BlockTree.test.tspackages/visual-editor-renderer-vue/tests/PostTemplateMasonry.test.tsresources/js/visual-editor/blocks/__tests__/masonry-layout.test.tsresources/js/visual-editor/blocks/_shared/masonry/__tests__/fallback.test.tsresources/js/visual-editor/blocks/_shared/masonry/fallback.tsresources/js/visual-editor/blocks/_shared/masonry/index.tsresources/js/visual-editor/blocks/_shared/masonry/masonry.cssresources/js/visual-editor/blocks/grid-item/block.jsonresources/js/visual-editor/blocks/grid-item/edit.tsxresources/js/visual-editor/blocks/grid/block.jsonresources/js/visual-editor/blocks/grid/edit.tsxresources/js/visual-editor/blocks/grid/index.tsresources/js/visual-editor/blocks/post-template/block.jsonresources/js/visual-editor/blocks/post-template/edit.tsxresources/js/visual-editor/blocks/post-template/index.ts
- Clamp `columns` in the React + Vue post-template renderers so
NaN / Infinity / fractional values can't poison the emitted
`columns-N` class and `data-ap-cols` attribute.
- Preserve `gap` on `.ap-masonry-js-fallback` so `readGapPx()` reads
the right spacing in the fallback path (the `:not(...)` baseline
rule dropped it).
- Clear the `__apMasonryController` marker in `destroy()` so SPA /
Livewire re-bootstraps don't skip reinitialization on a recycled
container.
- Unobserve removed items from the ResizeObserver in both the TS
fallback and the JS bootstrap so long-running pages don't
accumulate detached targets.
- Track every controller spawned in the fallback test suite and
`destroy()` each one in `afterEach` so observers + listeners don't
leak across runs.
- Honor per-breakpoint column counts: renderers stamp
`data-ap-cols-{bp}` for each responsive override, and the
fallback picks the active breakpoint via `window.innerWidth`
against the same Tailwind min-widths the editor uses (640 / 768
/ 1024 / 1280 / 1536).
- Tests: 4 new (2 JS responsive picker, 2 PHP responsive emission).
Full suite: 2255 JS + 1325 PHP, renderer-parity script clean.
Out of scope (flagged but deferred):
- CodeRabbit's docstring-coverage warning is sub-threshold across
the changeset, but the existing project convention is brief block
comments rather than per-function docstrings — the new code
follows that. No action.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/visual-editor-renderer-vue/src/blocks/core/query.ts`:
- Around line 25-27: The clampColumns function currently rejects numeric string
values like "4" because it checks if the value is a number type before checking
if it's finite, causing it to fall back to the default value. Modify the type
check in clampColumns to first attempt to coerce the value to a number using
Number() or parseFloat() if the value is a string, then validate that the
coerced result is finite. This will allow numeric strings to be accepted and
properly clamped instead of being rejected and falling back to the default,
ensuring that Line 86 emits the correct columns-N and data-ap-cols values when
upstream serializes numeric attributes as strings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 54db0ddb-fae4-4f88-96c7-d3ed7ab15381
📒 Files selected for processing (11)
packages/visual-editor-renderer-blade/resources/assets/frontend/masonry-fallback.jspackages/visual-editor-renderer-blade/resources/assets/frontend/masonry.csspackages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/grid.blade.phppackages/visual-editor-renderer-blade/tests/Feature/MasonryLayoutRenderTest.phppackages/visual-editor-renderer-react/src/blocks/artisanpack/grid.tsxpackages/visual-editor-renderer-react/src/blocks/core/query.tsxpackages/visual-editor-renderer-vue/src/blocks/artisanpack/grid.tspackages/visual-editor-renderer-vue/src/blocks/core/query.tsresources/js/visual-editor/blocks/_shared/masonry/__tests__/fallback.test.tsresources/js/visual-editor/blocks/_shared/masonry/fallback.tsresources/js/visual-editor/blocks/_shared/masonry/masonry.css
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/visual-editor-renderer-react/src/blocks/core/query.tsx
- packages/visual-editor-renderer-vue/src/blocks/artisanpack/grid.ts
- resources/js/visual-editor/blocks/_shared/masonry/fallback.ts
- resources/js/visual-editor/blocks/_shared/masonry/masonry.css
- packages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/grid.blade.php
- resources/js/visual-editor/blocks/_shared/masonry/tests/fallback.test.ts
- packages/visual-editor-renderer-blade/resources/assets/frontend/masonry.css
- packages/visual-editor-renderer-blade/resources/assets/frontend/masonry-fallback.js
CodeRabbit flagged that `clampColumns` in the React + Vue post-template renderers rejects numeric-string `columns` values like "4" — but the project's `attrInt` helper accepts them, and hosts that round-trip block attributes as strings would otherwise fall back to the default 3 columns. Delegate to `attrInt` so numeric strings survive, NaN / Infinity / unparseable strings still fall through to the caller's fallback, and the result is clamped to the stylesheet's [1, 12] range. Adds 4 tests (2 React, 2 Vue) covering numeric-string acceptance and unparseable-value fallback. Closes the open CodeRabbit comment on #603.
github-project-automation
Bot
moved this from In Review
to Done
in ArtisanPack UI Overview
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.
Description
Adds
masonryas a first-class layout choice on the existingartisanpack/post-templateandartisanpack/gridblocks (no new blocks). Native CSS Grid masonry (grid-template-rows: masonry) lights up automatically where the browser supports it via@supports; otherwise the public frontend runs a shared JS bootstrap that packs items shortest-column-first, with support for resize, mutation, image load, andgridColumnSpanfor wider items.Closes: #593
Type of Change
Related Issue
Issue: #593
Motivation and Context
Both the Query Loop (via post-template) and the standalone Grid block today expose only fixed-row grid as their multi-column layout. There's no native masonry option — the Pinterest-style packed-tile layout where items have variable heights and pack into columns left-to-right without leaving vertical whitespace. Users currently work around this with custom CSS or third-party JS. This change offers masonry as a first-class layout option, sharing the implementation between the two host blocks.
Changes Made
post-template: addsmasonryto thelayoutenum, plus a Masonry toolbar button alongside List and Grid. Columns control gains a masonry-specific help line.grid: adds a newlayoutModeattribute (fixed|masonry) exposed via aToggleControl. Publishes the layout mode to grid-item children viaprovidesContext.grid-item: subscribes to the parent grid's layout mode and disables the row-span control with an explanatory hint when the parent is masonry (gridColumnSpanstill applies per the spec).resources/js/visual-editor/blocks/_shared/masonry/:masonry.css— native@supports (grid-template-rows: masonry)rule + JS-fallback positioning context.fallback.ts—initMasonry(container, options)controller with renderer-agnostic shortest-column-first packing, native-support feature detection,data-ap-col-spanhonoring, resize/mutation/image-load observers, and clean teardown.frontend/masonry.css+frontend/masonry-fallback.js(plain-JS port of the controller) and wires both through<x-ve-blocks-styles />.is-layout-masonry/ap-grid-layout-masonryclasses anddata-ap-colsattribute so the bootstrap can target them.is-layout-gridunderneathis-layout-masonryso Gutenberg's bundled layout baseline providesdisplay: gridinside the editor canvas iframe and the post-template grid CSS sets the per-breakpoint columns-N tracks.How Has This Been Tested?
Testing Environment:
Tests Performed:
is-layout-masonry+is-layout-grid+columns-N+data-ap-cols, and the masonry-fallback bootstrap packs items into a packed layout.gridColumnSpan) still applies in masonry mode — items configured with span 2 render twice as wide.Accessibility Tests Run
__()-localised labels + help text)label; range/toggle inherit Gutenberg defaults)Details: Masonry preview is purely layout — no interactive content was added. All controls use Gutenberg's accessible
ToolbarButton,RangeControl, andToggleControlprimitives with localised labels and help text.Tests Added
Test details:
resources/js/visual-editor/blocks/__tests__/masonry-layout.test.ts— block.json shape (6 tests).resources/js/visual-editor/blocks/_shared/masonry/__tests__/fallback.test.ts— JS fallback unit tests (14 tests: feature detection caching, shortest-column-first packing, span handling, column clamping, container height, relayout, destroy + style restoration).packages/visual-editor-renderer-react/tests/PostTemplateMasonry.test.tsx+BlockTree.test.tsxupdates — React renderer parity (5 tests).packages/visual-editor-renderer-vue/tests/PostTemplateMasonry.test.ts+BlockTree.test.tsupdates — Vue renderer parity (5 tests).packages/visual-editor-renderer-blade/tests/Feature/MasonryLayoutRenderTest.php— Blade output +<x-ve-blocks-styles />wiring (7 tests).Totals (full suite run):
Documentation
Documentation details: Inline comments on edit components and the masonry CSS / JS fallback explain:
is-layout-gridunderneathis-layout-masonry).BlockContextProvider).@supports (grid-template-rows: masonry)is used as the feature gate.Pre-Submission Checklist
Trade-off Notes
Editor canvas preview vs. public frontend:
@supports.The acceptance criterion "editor canvas uses the JS fallback so preview is consistent" turned out to be unworkable in practice given Gutenberg's canvas behavior — freshly-inserted grid-items are empty (height 0) so the fallback collapses the wrapper, and post-template iterations stream content across frames via
BlockContextProviderso heights can't be measured reliably. The compromise (column-grid preview + frontend-packed output) is documented in the editor edit components.Once Safari + Firefox ship native masonry support, the canvas preview will light up automatically via the
@supportsrule — no code change needed.Summary by CodeRabbit
Release Notes