Repository navigation
feat: port crosswinds blocks under artisanpack/* (#495) - #577
Merged
Merged
Conversation
…readcrumbs Forks the upstream crosswinds-blocks breadcrumbs block into the visual-editor package and establishes the pattern for CW1–CW7 to follow: - Editor scaffold under `resources/js/visual-editor/blocks/breadcrumbs/` (block.json, edit, save, index, inserter-icon, separators, css). Drops the upstream `breadrcumbsSchema` typo for `breadcrumbsSchema` since the fresh namespace has no legacy markup to preserve. - Blade + React + Vue renderers reading a server-stamped `_resolvedTrail` attribute, emitting `<nav><ol>…</ol></nav>` with the selected chevron separator and optional schema.org BreadcrumbList microdata. URLs run through the existing UrlSanitizer / safeUrl helpers so unsafe schemes are dropped. - Three-renderer parity tests (12 new assertions per renderer) covering schema-on/schema-off, separator fallback, empty trail, and unsafe-URL rejection. Manifest in `packages/renderer-parity.json` updated. No transforms — upstream did not define any for breadcrumbs. Follow-ups filed under umbrella #495: - #565 — BreadcrumbsResolver to stamp `_resolvedTrail` on the server - #566 — Editor canvas iframe pipeline for custom block CSS Closes #496
- Only mark the breadcrumb span with `ap-breadcrumbs__current` when the trail entry is actually `current`. Previously every non-link branch carried the class — so an entry whose URL was sanitized away rendered as 'current' even when it wasn't the trail's final crumb. Mirrored across all three renderers (Blade, React, Vue). - React: use the loop index for the `<li>` key instead of a position+label composite — labels can collide and the array index is already stable for this list. - Add an assertion to the unsafe-URL parity tests confirming exactly one `ap-breadcrumbs__current` occurrence in the rendered output.
…adcrumbs feat: CW0 pilot — port crosswinds-blocks/breadcrumbs as artisanpack/breadcrumbs
Adds the artisanpack/accordions, accordion, accordion-title, accordion-body, tabs, and tab-section blocks plus three-renderer parity (Blade + React + Vue) and renderer-parity tests. Accordion family: - accordions parent owns the wrapper; accordion child owns the panelId + panelIcon, auto-generated from clientId on mount. - accordion-title / accordion-body are content-only — the accordion renderer walks its children and re-wraps them with the matching id / aria-controls / aria-labelledby / hidden wiring, so authors only ever set the panel id once. Tabs family: - tab-section owns its own label + tabId (both auto-fill on mount). - tabs renderer walks innerBlocks to build the tablist AND re-wraps each section's grandchildren with the resolved tabId, so trigger aria-controls always lines up with panel id. Adds positional fallback for missing tabIds and dedup for duplicates. Front-end: - Per-family CSS + a vanilla interactivity script ship from visual-editor-renderer-blade under public/vendor/.../frontend/, wired into the new <x-ve-blocks-styles /> emission. The interactivity script handles accordion toggle (click + Enter / Space) and tab switching (click + arrow keys + Home / End). - Editor canvas iframe also picks up the styles via canvas-styles with editor-only overrides that keep every panel / section expanded for editing. Renderer infrastructure: - BlockRenderer.php now passes the raw innerBlocks array to Blade partials (alongside the pre-rendered innerBlocksHtml) so partials can walk children when they need to derive parent state from them. Closes #497
- Lowercase the `currentcolor` keyword in accordion + tabs CSS to satisfy stylelint's value-keyword-case rule. - Slugify $tabId in the standalone tab-section partial so emitted id / aria-labelledby attributes are always valid HTML ids even when the persisted slug carries characters outside [a-z0-9-].
…-tabs feat: port accordion + tabs block families
Forks `crosswinds-blocks/basic-grid` and `basic-grid-item` into the
artisanpack/* namespace with the "basic-" prefix dropped from block
names, attributes, classes, and labels.
Grid family:
- artisanpack/grid (parent) carries a single `numColumns` attribute,
enrolled in artisanpackResponsive.attributes so the per-breakpoint
HOC routes overrides into `responsive.numColumns.{bp}`. Gap is
surfaced through the standard spacing.blockGap support with
horizontal/vertical sides.
- artisanpack/grid-item (child) carries `gridColumnSpan`, `gridRowSpan`,
and an `innerLayout` enum (normal/equal/center/bottom/last-bottom).
Spans are enrolled in artisanpackResponsive.attributes; the
column-span max is clamped to the grid's `numColumns` via block
context.
Renderers (Blade + React + Vue):
- Per-breakpoint values encode as static classes:
`ap-grid-has-N-{base|sm|md|lg|xl|2xl}-columns`
`ap-grid-item-span-N-{bp}-{columns|row}`
- grid.css ships the matching @media-keyed rules; mobile-first cascade
picks the active breakpoint's class at runtime. Class-driven (not
per-instance @media via the accumulator) so the front-end works
regardless of where `<x-ve-blocks-styles>` lands.
Editor canvas:
- canvas-styles.ts injects grid.css into the iframe so authors get an
accurate preview as they switch breakpoints.
Tests:
- Renderer-parity tests cover the family in all three renderers
(Unit) plus a Feature test that verifies the per-breakpoint class
emission survives through `<x-ve-blocks>`.
Closes #498
- Move grid CSS link outside `$emitInteractive` gate — grid is a pure
layout family with no companion interactivity script, so it must
ship even when hosts opt out of accordion/tabs interactivity.
- Fix `.ap-grid-item-layout-bottom`: with `flex-direction: column` the
main axis is vertical, so `justify-content: flex-end` (not
`align-items: flex-end`) is what pushes children to the bottom.
- Validate raw `responsive.*` overrides as finite numbers before
clamping in React + Vue renderers — previously non-numeric values
silently snapped to the clamp min (1) and emitted bogus
`ap-grid-*-1-{bp}-*` classes. Regression tests cover the fix.
…ds-blocks-cw2-grid-cluster feat: port crosswinds-blocks grid + grid-item families (CW2)
Codifies the rule that blocks ported from the legacy upstream into the artisanpack/* namespace must NOT carry any "CW#" / legacy-project-name attribution in their code, comments, asset filenames, or commit messages. Any Claude session opening this repo on any machine reads the rule from here, so it is no longer dependent on per-machine agent memory.
… blocks Adds two adjacent-post container blocks that wrap inner post-* children against the resolved neighbor in the same post type. PostResolver looks up the adjacent post via the existing accessor candidates, recursively re-stamps the inner-block subtree against the neighbor, and sets a `_resolvedHasAdjacent` flag so renderers can collapse to empty markup when no neighbor exists. Also exposes the existing `isLink` / `linkTarget` / `rel` attributes on `artisanpack/post-title` through a Settings sidebar panel so the titles inside these wrappers can be made clickable — the Blade / React / Vue renderers already honored the attributes; only the editor UI was missing. Closes #499
…-post-blocks feat: artisanpack/next-post + artisanpack/previous-post container blocks
…me blocks
Adds three site-wide chrome blocks under the artisanpack namespace:
- copyright (dynamic) — © + configurable text + current year, with three
display modes (icon-text / icon-only / text-only). Year is read at
render time on every renderer so the line stays accurate without a
rebuild.
- marquee (static) — RichText body wrapped in a sized `<div>`. The
renderers emit an inline `animation: ap-marquee-scroll Ns linear
infinite` declaration that drives the keyframe shipped in the new
`frontend/marquee.css` asset; the editor preview drops the animation
so authors can edit the text without it scrolling under their cursor.
- comments-number (dynamic) — `<count> <singular|plural>` line.
PostResolver stamps `_resolvedCommentCount` (reusing the existing
branch shared with post-comments-count), and the renderers pick the
singular or plural label by count.
Wires the marquee front-end stylesheet through `<x-ve-blocks-styles />`
(new `marqueeStyleHref` + `data-ve-marquee` link tag) so consumers ship
the keyframes alongside the existing accordion / tabs / grid bundles.
Closes #500
- Honor prefers-reduced-motion on the marquee animation in both the editor preview and the published frontend stylesheet — the keyframe is suppressed and the off-screen transform is reset so opted-out visitors see static text instead of an endless scroll. - Extract the marquee width / speed clamp helpers into a shared `clamp.ts` module so edit.tsx and save.tsx can't drift in their validation ranges or defaults. - Freeze the system clock with `vi.useFakeTimers()` in the React + Vue copyright renderer specs so the year assertions stay deterministic across a Dec 31 → Jan 1 boundary. - Assert both `postId` AND `postType` in the comments-number usesContext contract test so a future removal of `postType` trips the suite.
…locks feat: artisanpack/copyright + marquee + comments-number site-chrome blocks
…icons + social-share-content blocks Ports the single-post content cluster under the artisanpack/* namespace with full Blade + React + Vue renderer parity. - single-content: container block scoped to one chosen entry. QueryInliner resolves the post via QueryResolverContract and re-stamps inner blocks via PostResolver, falling back to the host post when postId is 0. - related-posts: loop block over N taxonomy-related posts. QueryInliner derives the taxonomy term IDs from the host post (now threaded through inline()) and clones the inner-block tree once per result with PostResolver stamps. - author-social-icons: PostResolver stamps _resolvedAuthorSocialLinks from the author's stored profile URLs. Email bare addresses normalize to mailto: URLs. - social-share-content: PostResolver builds per-platform share URLs from the host post's permalink, title, and featured image and stamps _resolvedShareLinks. Renderer-parity manifest, three-renderer parity fixtures, block.json contract test, PostResolver social-icon test, and QueryInliner single- post-content test added. CLAUDE.md carries a temporary in-progress section so the work can be picked up from another machine. Refs #501 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Manual-testing follow-ups on the single-post content cluster:
- single-content: replace the post-id text input with a searchable
post dropdown backed by /visual-editor/api/{plural}, plus inserter
variations registered per cms-framework content type via a new
data-content-types attribute on the mount element. Variation slugs
are singular (post/page) to match the WP entity convention; the
paired plural drives the REST URL. QueryInliner singularizes any
legacy plural postType so blocks saved before this pass still
resolve.
- related-posts: mirror the query block's attribute shape (query
object + queryId + displayLayout) and inspector UX (perPage /
columns / offset / orderBy controls). The canvas now fetches and
renders N-1 read-only preview cards below the editable inner
template so authors can see the actual count.
- social-share-content + author-social-icons: icon color, hover icon
color, icon background color, and hover icon background color via
PanelColorSettings; renderers emit only CSS custom properties so
the :hover swap actually applies (inline color would have beaten
the :hover rule on specificity). Switched glyph paths to Simple
Icons (CC0) for a polished brand look; email + website are
matching hand-drawn UI glyphs since Simple Icons is brands-only.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Escape the content-types JSON in editor.blade.php + mount.blade.php via JSON_HEX_* so attribute injection can never break out of the data-content-types value. - Swap the hand-rolled singularizer in VisualEditorComponent + routes/web.php for Str::singular so legitimate stems like `types` or `classes` aren't mangled. - Drop dangerouslySetInnerHTML from the related-posts editor preview; excerpts get tag-stripped client-side before render. - Validate the single-content collection slug against [a-z0-9_-] and fall back to `posts` when it isn't recognised + isn't slug-shaped, so a tampered `postType` can't compose a path-traversal URL. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Set aria-label on every social-share + author-social chip anchor so icon-only modes still expose a discernable name to screen readers. Vue inserts the attribute after `style` to keep React/Vue parity on the byte-compared HTML the renderer-parity harness checks. - Switch the CSS `fill: currentColor` declarations to lowercase `currentcolor` in both copies of social-icons.css to satisfy the project's stylelint value-keyword-case rule. - Replace the leftover hand-rolled plural-strip in QueryInliner::expandSingleContent with Str::singular so the back-compat path uses the same inflector as the variations registry (avoids the `types` -> `typ` class of mistakes). - Capitalize "Mac mini" in CLAUDE.md per Apple style. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ontent-cluster feat(#501): single-post content cluster — single-content, related-posts, social-share + author-social icons
Forks six search-family blocks under `artisanpack/*` with full Blade +
React + Vue parity: `search-field`, `search-filters`,
`search-filters-buttons`, `search-filters-taxonomy`,
`post-types-search-results`, `single-post-types-search-results`. Each
block ships block.json + dynamic save, edit-side preview, inserter icon,
and renderer partials/components. Inter-block query state flows through
the standard GET form (hidden `post_type` + `name="s"` + `?{taxonomy}=`
selects) so authors can wire the cluster end-to-end with no extra
JavaScript. Renderer-side request state arrives via `_resolved*`
attribute stamps; Blade falls back to the live request and cms-framework
`PostCategory` / `PostTag` models when the host has not pre-resolved.
Adds 17 new family-contract tests + 7 React/Vue parity fixtures (full
suites: 2018 JS + 1078 PHP). Renderer-parity manifest extended with the
six new names.
Also fixes two pre-existing bugs the new templates surfaced:
- `__('Pagination')` in `query-pagination.blade.php` matched Laravel's
bundled `lang/en/pagination.php`, returning the file's `previous` /
`next` array and crashing `e()` downstream. Renamed the aria-label
key to `'Pagination navigation'` with an inline note explaining the
collision so a future contributor does not reintroduce it.
- Blade's directive lexer does not recognize `@endif` when it sits
directly after a non-whitespace character (`selected@endif`), so the
literal `@endif` leaked into the compiled output and unbalanced the
`<?php if(...): ?>`. Replaced the inline-attribute pattern with a
`@php`-block flag plus `{!! $flag ? ' selected' : '' !!}`.
Closes #502
- Sync `query-pagination` aria-label across React + Vue renderers (both were still emitting "Pagination" while Blade was renamed to "Pagination navigation" to dodge the lang/en/pagination.php collision). - Localize `search-filters-buttons` Blade fallbacks via `__()`. - Derive `search-filters-taxonomy` `$taxonomyName` from the sanitized slug when the author has not supplied one (`post_tag` → `Post Tag`). - Suffix `search-filters-taxonomy` `$selectId` with an attributes hash so two instances of the same taxonomy on one page get distinct ids. - Normalize the incoming `?post_type=` value in `single-post-types-search-results` the same way the saved attribute is sanitized so `?post_type=Post` matches a `post` section. - Use React `useId()` for the preview ids in `search-field/edit.tsx` and `search-filters-taxonomy/edit.tsx` so multiple instances no longer collide on duplicate DOM ids.
…uster feat: port crosswinds search cluster (#502)
Static progress-bar block that visualises a skill level as a horizontal filled bar. Three-renderer parity (Blade + React + Vue) with shared clamp helpers, ARIA progressbar semantics, and a strict allow-list for inline CSS colour values on the Blade side. No `skill` CPT dependency — the block ships as a manual-input control (skill level, bar height, colours, accessible label), so authors can render it without registering an upstream post type. Closes #503
Mirror the Blade-side colour allow-list in the React + Vue renderers so the same saved block renders identical colours across all three. Renderers now reject anything that isn't a hex / rgb / rgba / hsl / hsla value, matching the Blade contract.
…ider feat: add artisanpack/skills-slider block
Stamps `_resolvedTrail` on every `artisanpack/breadcrumbs` block in the
saved tree so the Blade / React / Vue renderers (which already consume
the attribute) emit a populated `<ol>` on the public frontend.
The default trail is `Home → …ancestors → current`. The Home hop reads
from `config('artisanpack.visual-editor.breadcrumbs.home_url')` (falls
back to `url('/')`); the last entry is auto-marked `current: true` so
the renderer drops the link and emits `aria-current="page"`. Hierarchical
posts (pages with `parent`/`parent_id`) walk the ancestor chain bounded
to 32 levels with cycle detection.
Hosts customize the trail through the `ap.visual-editor.breadcrumbs.trail`
filter (e.g. to insert a "Category" hop between Home and a blog post)
without subclassing. A pre-stamped `_resolvedTrail` on the saved tree
wins over the resolver fallback, matching PostResolver's contract.
Also ships `breadcrumbs.css` to the frontend asset bundle and wires it
into `<x-ve-blocks-styles />` so the inline-flex layout + chevron
separator applies on the public site without each host re-implementing
the rules. Tightens the inter-item gap from 0.5rem to 0.25rem.
Closes #565
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Remove the redundant `onHomepage()` short-circuit in `BreadcrumbsResolver::defaultTrail()` — both the homepage-specific and the generic `null === $post` branches returned the same trail, so the extra check (and its `onHomepage()` helper + `Illuminate\Http\Request` import) were dead surface. Behavior unchanged: a null post still collapses to a single Home entry that the finalizer marks `current: true`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…esolver feat: add BreadcrumbsResolver for artisanpack/breadcrumbs (#565)
The sandboxed BlockCanvas iframe never saw the per-block stylesheets
imported via side effect from each block's index.ts, so blocks with
their own CSS (callout, breadcrumbs, and future blocks) rendered
unstyled in the editor preview.
Resolve every blocks/*/*.css file as inline CSS via import.meta.glob
and append the discovered sheets to canvasStyles, replacing the five
per-family manual imports that used to force an edit to canvas-styles
every time a new custom block landed. Entries are sorted by path so
the cascade is deterministic across builds.
Drop the inline style={} workaround from blocks/breadcrumbs/edit.tsx
now that breadcrumbs.css reaches the iframe, and extend the canvas-
styles test suite to assert the glob actually discovers the cited
sheets (callout, breadcrumbs, accordion, tabs, grid, marquee, shared
social-icons) rather than snapshotting a single file count.
Closes #566
Drop the identity `.map((path) => path)` when constructing the blockStyleStrings Set in the canvas-styles test — the array is already a string[], so it can flow straight into `new Set(...)`.
…-custom-block-css feat: auto-inject custom block CSS into editor canvas iframe (#566)
Move every first-party block out of the catch-all `artisanpack` inserter
category into the standard WP categories (text/media/design/widgets/theme)
plus a new `search` category for the six search-cluster blocks. The
`artisanpack` category was never going to feel native next to `core/*`
blocks — the new homes match how authors actually scan the inserter.
Category moves (25 blocks):
search (new) search-field, search-filters, *-filters-buttons,
*-filters-taxonomy, post-types-search-results,
single-post-types-search-results
design accordion, accordion-body, accordion-title, accordions,
tabs, tab-section, grid, grid-item, callout, marquee
theme breadcrumbs, comments-number, copyright, single-content,
related-posts, author-social-icons, social-share-content
widgets form, skills-slider
Editor wiring:
- editor/custom-blocks.ts adds SEARCH_CATEGORY_SLUG +
ensureSearchCategory() and calls it from registerCustomBlocks().
ensureArtisanpackCategory() is retained for downstream apps that
still ship blocks under the `artisanpack` slug but is no longer
called at registration time.
- 8 block index.ts files (accordion x4, tabs x2, grid x2) now wire
their new inserter-icon module through to registerBlockType.
Inserter icons — switch all 25 first-party blocks to Font Awesome Free
Solid SVGs (path data copied verbatim from the bundled FA SVGs under
resources/icons/font-awesome/fas, CC BY 4.0). Replaces the hand-drawn
SVGs that were ~close to the Gutenberg core glyphs but consistently
read as amateur next to them:
accordion -> square-caret-down
accordion-body -> align-justify
accordion-title -> heading
accordions -> layer-group
author-social-icons -> id-card-clip
breadcrumbs -> signs-post
callout -> lightbulb
comments-number -> comment-dots
copyright -> copyright
form -> clipboard-list
grid -> table-cells
grid-item -> square
marquee -> bullhorn
post-types-search-results -> magnifying-glass-arrow-right
related-posts -> newspaper
search-field -> magnifying-glass
search-filters -> filter
search-filters-buttons -> sliders
search-filters-taxonomy-> tags
single-content -> file-lines
single-post-types-search-results -> note-sticky
skills-slider -> bars-progress
social-share-content -> share-nodes
tab-section -> window-maximize
tabs -> window-restore
Tests:
- search-cluster-family / single-post-content-family / skills-slider
assert their new categories.
- site-chrome-family becomes per-block (copyright=theme,
marquee=design, comments-number=theme).
- custom-blocks.test.tsx adds an ensureSearchCategory suite mirroring
the existing ensureArtisanpackCategory one; the callout block.json
+ registerCustomBlocks assertions move to the `design` category.
2032/2032 JS tests pass; renderer parity OK.
Add NOTICE.md at the package root with full Font Awesome attribution for the bundled SVG paths now used by every first-party inserter icon: credits Fonticons, Inc., links to the FA Free license and CC BY 4.0 license text, and lists which icon each of the 25 blocks pulls from. The 25 per-file inserter-icon.tsx comments are updated in turn to reference NOTICE.md and the CC BY 4.0 license URL so each consuming file carries its own attribution trail. Skipped CodeRabbit's other findings (one per CR pass): each claimed the SVG path data for `share-nodes` / `magnifying-glass` was wrong and should be swapped for the FA v6 path with viewBox "0 0 448 512". This package ships FA v7.2.0 (viewBox "0 0 512 512") — the paths in the inserter components match the bundled `.svg` source files under `resources/icons/font-awesome/fas/` byte-for-byte. The suggestions were CR confusing a different FA major version for the canonical one. Tests: 2032/2032 JS tests still pass.
|
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 (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR ports multiple ChangesArtisanPack block port
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
ViewFromTheBox
marked this pull request as ready for review
June 13, 2026 01:55
CI surfaced this when the umbrella PR ran the full suite for the first time (the per-cluster PRs only ran CodeRabbit). The assertion still expected `artisanpack/callout` to live under the `artisanpack` inserter category, but the icon + recategorise commit moved it to `design` alongside the rest of the design-cluster blocks.
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (14)
packages/visual-editor-renderer-vue/src/blocks/artisanpack/searchCluster.ts-72-74 (1)
72-74:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse an instance-unique seed for generated control IDs.
Line 72 and Line 178 derive IDs from attribute content only, so two identical block instances can emit the same
id. That can breaklabel[for]targeting and produce invalid duplicate IDs in the same document. Please include a per-instance unique seed (block/node identity) when composing these IDs.Also applies to: 178-179
🤖 Prompt for 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. In `@packages/visual-editor-renderer-vue/src/blocks/artisanpack/searchCluster.ts` around lines 72 - 74, The generated control IDs (e.g., inputId produced via shortHash(`${label}|${placeholder}|${searchValue}`)) only use attribute content so identical blocks collide; include a per-instance unique seed when composing these IDs (for example `${instanceSeed}|${label}|${placeholder}|${searchValue}`) and persist that seed across renders (use a stable node/block id if available or generate-and-store one on mount/useRef) so IDs remain unique and stable; apply this change to the inputId construction shown and the analogous ID generation at the other site (the occurrence referenced at lines 178-179) so both places use the same per-instance seed.packages/visual-editor-renderer-vue/src/blocks/artisanpack/breadcrumbs.ts-2-2 (1)
2-2:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove internal phase-label wording from the header comment.
Line 2 includes an internal phase label in source comments; this should be removed to match repository porting conventions.
Suggested edit
- * Vue renderer for the `artisanpack/breadcrumbs` block (CW0 pilot — `#496`). + * Vue renderer for the `artisanpack/breadcrumbs` block (`#496`).Based on learnings, internal phase labels should not appear in code/comments during these ports.
🤖 Prompt for 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. In `@packages/visual-editor-renderer-vue/src/blocks/artisanpack/breadcrumbs.ts` at line 2, The header comment for the Vue renderer of the `artisanpack/breadcrumbs` block contains an internal phase label ("CW0 pilot — `#496`"); remove that phrase so the comment reads simply "Vue renderer for the `artisanpack/breadcrumbs` block." Locate the top-of-file header comment in the breadcrumbs renderer and delete the internal phase-label wording, leaving the rest of the comment intact.Source: Learnings
packages/visual-editor-renderer-react/src/blocks/artisanpack/breadcrumbs.tsx-2-2 (1)
2-2:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove internal phase label from header comment.
Line 2 uses
CW0 pilot, which violates the repo rule against internal phase labels in code/comments.Suggested edit
- * React renderer for the `artisanpack/breadcrumbs` block (CW0 pilot — `#496`). + * React renderer for the `artisanpack/breadcrumbs` block (`#496`).Based on learnings: “Do NOT reference internal phase labels (CW0, CW1, CW2, etc.) … in code, comments, asset filenames, class/property names, CSS comments, test descriptions, or commit messages.”
🤖 Prompt for 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. In `@packages/visual-editor-renderer-react/src/blocks/artisanpack/breadcrumbs.tsx` at line 2, Remove the internal phase label "CW0 pilot" from the header comment in the artisanpack/breadcrumbs renderer; edit the header comment that currently reads "React renderer for the `artisanpack/breadcrumbs` block (CW0 pilot — `#496`)" to a neutral form such as "React renderer for the `artisanpack/breadcrumbs` block" so no internal phase labels remain; ensure no other occurrences of "CW0", "CW1", etc. remain in that header comment or nearby comments.Source: Learnings
packages/visual-editor-renderer-react/src/blocks/artisanpack/searchCluster.tsx-67-69 (1)
67-69:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEnsure control IDs are unique per block instance.
Line 67 and Line 145 derive IDs from repeatable values, so identical instances can emit duplicate IDs. That breaks label targeting and creates invalid DOM semantics on pages with multiple instances. Please include an instance-unique suffix for both IDs (resolver/client instance identifier), not only value-derived hashes.
Also applies to: 145-146
🤖 Prompt for 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. In `@packages/visual-editor-renderer-react/src/blocks/artisanpack/searchCluster.tsx` around lines 67 - 69, The IDs computed from shortHash (e.g., the inputId constant created with shortHash(`${label}|${placeholder}|${searchValue}`)) are only value-derived and can collide across repeated block instances; include a per-instance unique suffix (for example a renderer/instance identifier passed as a prop or obtained from context like instanceId or rendererInstanceId) when constructing these IDs so they are unique per block instance. Update the inputId expression to append or incorporate that instance identifier (use the same instance identifier consistently wherever related IDs are computed), and make the same change to the other ID(s) derived from values later in the file (the ID at the 145–146 site mentioned in the review) so label/aria relationships remain correct across multiple instances. Ensure the instance identifier is required/validated or generated once per block instance and reused for all IDs in this component.packages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/author-social-icons.blade.php-124-124 (1)
124-124:⚠️ Potential issue | 🟡 MinorLocalize the ARIA label using a translatable %s placeholder
Line 124 hardcodes'Author on ' . $chip['label']; the repo’s dynamic i18n strings usesprintf( __( '...%s' ), ... )rather than__(..., [ ... ]).💡 Suggested change
- <a class="ap-author-social-icons__chip {{ $chip['slug'] }}" href="{{ $chip['url'] }}" aria-label="{{ 'Author on ' . $chip['label'] }}"{!! $styleAttr !!}> + <a class="ap-author-social-icons__chip {{ $chip['slug'] }}" href="{{ $chip['url'] }}" aria-label="{{ sprintf( /* translators: %s: author label. */ __( 'Author on %s' ), $chip['label'] ) }}"{!! $styleAttr !!}>🤖 Prompt for 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. In `@packages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/author-social-icons.blade.php` at line 124, Replace the hardcoded concatenation used for the aria-label on the anchor with a translatable sprintf-style string: change the aria-label expression on the ap-author-social-icons__chip anchor to use sprintf( __( 'Author on %s' ), $chip['label'] ) (inside Blade echoing) so the label becomes translatable via the %s placeholder while preserving the existing $chip['label'] and $styleAttr.packages/visual-editor-renderer-blade/src/Resolvers/BreadcrumbsResolver.php-232-236 (1)
232-236:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse strict boolean normalization for
currentin trail entries.
!empty()makes string values like"false"truthy, which can incorrectly mark breadcrumbs as current when filters pass scalar strings.Suggested fix
- $normalized[] = [ + $currentRaw = $entry['current'] ?? false; + $current = is_bool( $currentRaw ) + ? $currentRaw + : filter_var( $currentRaw, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE ); + + $normalized[] = [ 'label' => $label, 'url' => '' === $url ? null : $url, - 'current' => ! empty( $entry['current'] ), + 'current' => true === $current, ];🤖 Prompt for 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. In `@packages/visual-editor-renderer-blade/src/Resolvers/BreadcrumbsResolver.php` around lines 232 - 236, The 'current' normalization uses "! empty( $entry['current'] )" which treats strings like "false" as truthy; update the BreadcrumbsResolver normalized array assignment so the 'current' value is strictly coerced to boolean (e.g. use filter_var($entry['current'], FILTER_VALIDATE_BOOLEAN) or an explicit boolean check) instead of !empty(), replacing the existing 'current' => ! empty( $entry['current'] ) expression in the block that builds $normalized[].packages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/accordion.blade.php-6-20 (1)
6-20:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNormalize
panelIdto a valid HTML id token before wiring ARIA/data attributes.Escaping protects against injection, but it does not prevent invalid id characters (for example spaces), which can break control linkage and selector-based interactivity.
Suggested fix
- $panelId = isset( $attributes['panelId'] ) ? (string) $attributes['panelId'] : ''; + $panelId = isset( $attributes['panelId'] ) ? trim( (string) $attributes['panelId'] ) : ''; + $panelId = preg_replace( '/[^A-Za-z0-9_.:-]+/', '-', $panelId ) ?? ''; + $panelId = trim( $panelId, '-.' );packages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/search-filters.blade.php-14-16 (1)
14-16:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRe-apply safe defaults when sanitized slugs become empty.
After regex sanitization, an all-invalid input can become
''; current code then emits empty query keys/values instead of a usable default.
packages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/search-filters.blade.php#L14-L16: if$postType === ''after sanitize, reset to'post'before rendering the hidden input.packages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/search-filters-taxonomy.blade.php#L19-L21: if$taxonomy === ''after sanitize, reset to'category'before using it for request lookup and<select name>.🤖 Prompt for 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. In `@packages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/search-filters.blade.php` around lines 14 - 16, In search-filters.blade.php (lines 14-16) the sanitized $postType can become empty; after the preg_replace call, check if $postType === '' and if so set it back to 'post' before rendering the hidden input. In search-filters-taxonomy.blade.php (lines 19-21) do the same for $taxonomy: after sanitization check if $taxonomy === '' and if so reset it to 'category' before using it for request lookups and as the <select name> value; no other changes are required at the sibling site.src/View/Components/VisualEditorComponent.php-66-69 (1)
66-69:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPHPDoc missing
pluralkey in content-type shape.The property type declares
array{slug: string, label: string}, butresolveContentTypes()(lines 138–145) returnsarray{slug: string, plural: string, label: string}. Update both PHPDoc blocks to match the actual shape.📝 Proposed fix
/** - * `@var` array<int, array{slug: string, label: string}> + * `@var` array<int, array{slug: string, plural: string, label: string}> */ public array $contentTypes;and
/** * Lists the registered cms-framework content types so the editor can * surface variations of `artisanpack/single-content` (one per type) * and any other block that wants to enumerate available resources. * - * `@return` array<int, array{slug: string, label: string}> + * `@return` array<int, array{slug: string, plural: string, label: string}> * * `@since` 1.0.0 */🤖 Prompt for 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. In `@src/View/Components/VisualEditorComponent.php` around lines 66 - 69, The PHPDoc for the property $contentTypes and the PHPDoc for the return shape of resolveContentTypes() are out of sync with actual data (they omit the "plural" key); update both PHPDoc annotations to use array<int, array{slug: string, plural: string, label: string}> so the declared shape matches the array returned by resolveContentTypes().resources/js/visual-editor/blocks/single-content/variations.ts-23-24 (1)
23-24:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize variation labels/descriptions before shipping.
These inserter-visible strings are hardcoded in English, so they won’t translate in localized editor builds.
Suggested fix
import type { BlockConfiguration } from '`@wordpress/blocks`'; +import { __, sprintf } from '`@wordpress/i18n`'; import { getContentTypes } from '../../editor/content-type-registry'; +import { TEXT_DOMAIN } from '../../vendor/i18n'; @@ name: `single-${type.slug}`, - title: `Single ${type.label}`, - description: `Render a single ${type.label.toLowerCase()} and scope the inner blocks to it.`, + title: sprintf(__('Single %s', TEXT_DOMAIN), type.label), + description: sprintf( + __('Render a single %s and scope the inner blocks to it.', TEXT_DOMAIN), + type.label.toLowerCase() + ),🤖 Prompt for 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. In `@resources/js/visual-editor/blocks/single-content/variations.ts` around lines 23 - 24, Localize the inserter-visible strings in variations.ts by replacing the hardcoded title and description with i18n calls: use sprintf( __( 'Single %s', 'your-text-domain' ), type.label ) for title and sprintf( __( 'Render a single %s and scope the inner blocks to it.', 'your-text-domain' ), type.label.toLowerCase() ) for description (or use __ with concatenation if preferred), and ensure you import/wp.i18n functions (__, sprintf) at the top of the file; update the title and description properties accordingly so they are translatable.resources/js/visual-editor/blocks/skills-slider/edit.tsx-131-135 (1)
131-135:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign the ARIA range with the actual control bounds.
skillLevelis clamped to 1–100 and the inspector only allows 1–100, butaria-valueminstill advertises 0. That makes the announced range inconsistent with the value users can actually set.♻️ Proposed fix
- aria-valuemin={0} + aria-valuemin={1}🤖 Prompt for 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. In `@resources/js/visual-editor/blocks/skills-slider/edit.tsx` around lines 131 - 135, The ARIA range advertised by the progressbar is inconsistent: aria-valuemin is set to 0 while the control clamps skillLevel to 1–100 (safeLevel) and the inspector only allows 1–100; update the aria-valuemin attribute to match the actual minimum (set aria-valuemin={1} or use the existing min constant/prop if one exists) in the element that renders role="progressbar" (the same JSX that sets aria-valuenow={safeLevel} and aria-label={ariaLabel || __('Skill level', TEXT_DOMAIN)}), so the announced range aligns with the real control bounds.resources/js/visual-editor/blocks/breadcrumbs/index.ts-5-6 (1)
5-6:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove internal phase/provenance wording from this docblock.
Line 5 still includes internal rollout/provenance labeling that should not ship in source comments.
Based on learnings: internal phase labels and legacy provenance naming should be excluded from code/comments during this porting work.
🤖 Prompt for 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. In `@resources/js/visual-editor/blocks/breadcrumbs/index.ts` around lines 5 - 6, Remove the internal rollout/provenance wording from the top docblock that mentions `@wordpress/blocks.registerBlockType`—edit the comment to strip any internal phase labels (e.g., "CW0 pilot", "provenance", or similar rollout/legacy tags) so only neutral, public-facing documentation remains; leave the factual description of the block and its behavior but remove internal provenance phrases.Source: Learnings
resources/js/visual-editor/blocks/breadcrumbs/edit.tsx-73-76 (1)
73-76:⚠️ Potential issue | 🟡 MinorUse literal strings in
__()for SelectControl option labels
Inresources/js/visual-editor/blocks/breadcrumbs/edit.tsxandresources/js/visual-editor/blocks/copyright/edit.tsx,__()is called withoption.label(a computed/member value), which i18n extraction won’t pick up reliably. Build theoptionsso the first argument to__()is a static string literal (e.g.,__('<label literal>', TEXT_DOMAIN)), not__({}[key], TEXT_DOMAIN).🤖 Prompt for 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. In `@resources/js/visual-editor/blocks/breadcrumbs/edit.tsx` around lines 73 - 76, The SelectControl options use __() with a computed value (option.label) so i18n extraction will miss strings; update the options construction so the first argument to __() is a static literal for each label instead of __((option.label), TEXT_DOMAIN). Concretely, replace the current SEPARATOR_OPTIONS.map(...) usage in the options prop with either (a) prebuild SEPARATOR_OPTIONS so each entry's label is already the result of a literal call like __('Slash', TEXT_DOMAIN) and then use that label directly, or (b) map using a switch/lookup that passes a string literal into __() for each known option (e.g., __('Slash', TEXT_DOMAIN), __('Arrow', TEXT_DOMAIN), etc.), keeping value as option.value; ensure the code references SEPARATOR_OPTIONS and TEXT_DOMAIN so the correct symbols are updated.resources/js/visual-editor/blocks/search-filters/edit.tsx-62-69 (1)
62-69:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep the default
postoption selectable.The fallback only runs when the registry is empty. If the registry contains other types but not
post, this control renders withvalue="post"and no matching option, so the editor shows a blank selection.♻️ Proposed fix
- if (options.length === 0) { - options.push({ value: 'post', label: __('Posts', TEXT_DOMAIN) }); - } + if (!options.some(({ value }) => value === 'post')) { + options.unshift({ value: 'post', label: __('Posts', TEXT_DOMAIN) }); + }🤖 Prompt for 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. In `@resources/js/visual-editor/blocks/search-filters/edit.tsx` around lines 62 - 69, The options array built from getContentTypes() can lack a 'post' entry while the UI still uses value='post', causing an unmatched blank selection; update the logic that creates options (the options variable) to ensure a selectable default by checking if options.some(o => o.value === 'post') and if false add { value: 'post', label: __('Posts', TEXT_DOMAIN) } (use push or unshift as desired) so 'post' is present whenever missing.
🧹 Nitpick comments (4)
packages/visual-editor-renderer-vue/tests/BlockTree.test.ts (1)
576-577: ⚡ Quick winRemove internal phase-label wording from the test title.
Line 576 includes
"(I4 fork)", which conflicts with the repository’s naming guidance for tests/comments. Please use a neutral description instead.Suggested edit
- it('carries the `#338` a11y fix forward to artisanpack/search (I4 fork)', () => { + it('carries the `#338` a11y fix forward to artisanpack/search', () => {Based on learnings: “Do NOT reference internal phase labels ... in ... test descriptions.”
🤖 Prompt for 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. In `@packages/visual-editor-renderer-vue/tests/BlockTree.test.ts` around lines 576 - 577, Update the test title string in the it(...) call that currently reads "carries the `#338` a11y fix forward to artisanpack/search (I4 fork)" by removing the internal phase label "(I4 fork)" and replacing it with a neutral description (e.g., "carries the `#338` a11y fix forward to artisanpack/search"); leave the test body and makeBlock(...) invocation unchanged.Source: Learnings
packages/visual-editor-renderer-blade/tests/Feature/BlocksStylesComponentTest.php (1)
24-42: ⚡ Quick winAdd an explicit assertion that
interactivity.jsis not emitted when:interactive="false".Current expectations verify independent CSS emission but do not protect the negative contract for script emission.
💡 Suggested test addition
expect( $rendered ) ->toContain( '/vendor/visual-editor-renderer-blade/frontend/grid.css' ) ->toContain( 'data-ve-grid' ) ->toContain( '/vendor/visual-editor-renderer-blade/frontend/marquee.css' ) ->toContain( 'data-ve-marquee' ) + ->not->toContain( '/vendor/visual-editor-renderer-blade/frontend/interactivity.js' ) ->not->toContain( 'data-ve-accordion' ) ->not->toContain( 'data-ve-tabs' );🤖 Prompt for 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. In `@packages/visual-editor-renderer-blade/tests/Feature/BlocksStylesComponentTest.php` around lines 24 - 42, Add a negative assertion to the existing tests that render Blade::render('<x-ve-blocks-styles :interactive="false" />') (e.g., the "emits the grid + marquee..." and/or "emits the breadcrumbs..." examples) to assert that the interactivity script is not emitted; specifically, add expect($rendered)->not->toContain(...) for the interactivity asset path (interactivity.js) and/or the interactivity marker (e.g., 'data-ve-interactivity') so the test verifies the script is omitted when :interactive="false".packages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/author-social-icons.blade.php (1)
6-120: 🏗️ Heavy liftExtract shared social-chip preparation into a single helper to avoid divergence.
This block duplicates almost the entire normalize/filter/sanitize/style pipeline that also exists in
packages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/social-share-content.blade.php. Keeping this logic in one place will prevent drift in validation/sanitization behavior.🤖 Prompt for 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. In `@packages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/author-social-icons.blade.php` around lines 6 - 120, The template duplicates the normalize/filter/sanitize/style pipeline (the code that builds $selected/$selectedSlugs, iterates $linksRaw, calls SocialIconRegistry::author and UrlSanitizer::safe, builds $chips, $styleParts, $styleAttr, $showIcon and $showLabel); extract that logic into a single reusable helper (e.g., a function prepare_author_social_icons(array $attributes): array or a class method) that accepts the block $attributes and returns an array with keys chips, styleAttr, showIcon, showLabel (and any other derived values like radius if needed); implement the helper somewhere reusable (a view helper or service class), move the pipeline code into it (preserving all validation/sanitization rules and variable names like SocialIconRegistry::author and UrlSanitizer::safe), and replace the duplicated blocks in each template with a call to the new helper and assignment from its return value.resources/js/visual-editor/blocks/_shared/social-icons.ts (1)
60-75: ⚡ Quick winAvoid positional coupling in
SHARE_PLATFORMS.Using
AUTHOR_PLATFORMS[0/1/2/5]makes this brittle; reorderingAUTHOR_PLATFORMScan silently map the wrong icons. Prefer slug-based lookup.Refactor sketch
+function requireAuthorPlatform(slug: string): SocialIconDefinition { + const platform = AUTHOR_PLATFORMS.find((entry) => entry.slug === slug); + if (!platform) { + throw new Error(`Unknown author platform slug: ${slug}`); + } + return platform; +} + const SHARE_PLATFORMS: ReadonlyArray<SocialIconDefinition> = [ - AUTHOR_PLATFORMS[0], // facebook - AUTHOR_PLATFORMS[1], // twitter / X - AUTHOR_PLATFORMS[2], // mastodon + requireAuthorPlatform('facebook'), + requireAuthorPlatform('twitter'), + requireAuthorPlatform('mastodon'), { slug: 'reddit', // ... }, { slug: 'pinterest', // ... }, - AUTHOR_PLATFORMS[5], // email + requireAuthorPlatform('email'), ];🤖 Prompt for 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. In `@resources/js/visual-editor/blocks/_shared/social-icons.ts` around lines 60 - 75, SHARE_PLATFORMS is brittle because it references AUTHOR_PLATFORMS by numeric indices; change it to build the array by slug lookup (e.g., findPlatform('facebook'), findPlatform('twitter'), findPlatform('mastodon'), findPlatform('email')) instead of AUTHOR_PLATFORMS[0/1/2/5]; implement a small helper (e.g., getAuthorPlatformBySlug(slug): SocialIconDefinition | undefined) that returns the matching entry from AUTHOR_PLATFORMS and use it when constructing SHARE_PLATFORMS, providing the inline literal definitions (reddit, pinterest) unchanged and falling back to a safe default or excluding missing entries if find returns undefined.
🤖 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 `@config/visual-editor.php`:
- Around line 334-337: The breadcrumbs resolver is added under the 'breadcrumbs'
config key but the inserter allow-list 'enabled_blocks' still omits the block;
open the same config file and add 'artisanpack/breadcrumbs' to the
enabled_blocks array (the allow-list that controls which blocks are insertable)
so the resolver becomes insertable via the editor.
In
`@packages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/marquee.blade.php`:
- Line 32: The template currently emits raw HTML for the marquee with {!!
$content !!}, which can cause stored XSS if the marquee content is
user-controlled; change the rendering to either escape the content (use Blade's
escaped echo for $content) or run it through a trusted sanitizer/HTML whitelist
function before echoing, ensuring you still allow safe markup if needed; update
the ap-marquee__text output to use the sanitized/escaped value (replace raw {!!
$content !!} with the escaped/sanitized variant) so $content/marqueeContent is
never emitted unsanitized.
In
`@packages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/search-field.blade.php`:
- Around line 29-32: Both templates currently build form-control IDs only from
json_encoded attributes which can duplicate across instances; update each site
to append a per-instance identifier from the render context to the computed ID
(keep the existing md5(attributes) hash as a secondary stabilizer). In
packages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/search-field.blade.php
(lines 29-32) change the $inputId construction to include a per-instance token
(for example append the block's runtime/client id available in the render
context such as $attributes['id'] or a $clientId/$blockId or uniqid()), e.g.
combine substr(md5(json_encode($attributes)),0,8) with that token; in
packages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/search-filters-taxonomy.blade.php
(lines 90-94) make the same change for $selectId so each block instance yields a
unique ID while preserving the existing hash as a secondary stabilizer.
In `@packages/visual-editor-renderer-blade/src/BlockRenderer.php`:
- Around line 109-116: The dynamic rendering path drops raw child blocks when
falling back to static, so update the call and implementations to propagate
innerBlocks: change the call in BlockRenderer (currently renderDynamic($name,
$attributes, $innerBlocksHtml)) to pass the normalized $innerBlocks as a fourth
argument (renderDynamic($name, $attributes, $innerBlocksHtml, $innerBlocks)),
then update the renderDynamic method signature and its internal fallback to call
renderStatic($name, $attributes, $innerBlocksHtml, $innerBlocks) (instead of
passing an empty array) so any fallback static rendering receives the full
child-block tree.
In `@resources/js/visual-editor/blocks/tab-section/save.tsx`:
- Around line 7-9: Both save entrypoints currently return null which prevents
nested blocks from serializing; update the TabSectionSave
(resources/js/visual-editor/blocks/tab-section/save.tsx lines 7-9) to render the
section's saved inner blocks (use InnerBlocks.Content) instead of null, and
likewise update the TabsSave (resources/js/visual-editor/blocks/tabs/save.tsx
lines 7-9) to render the parent's saved inner blocks (use InnerBlocks.Content);
ensure the appropriate InnerBlocks import (from `@wordpress/block-editor` or
wp.blockEditor) is present at the top of each file so the rendered content is
serialized.
In `@resources/js/visual-editor/editor/content-type-registry.ts`:
- Around line 63-67: parseFromElement currently accepts any non-empty plural and
caches it, which allows malformed values to later break
SingleContentEdit.resolveCollection when used in `${API_BASE}/${collection}`;
update parseFromElement (the code handling rawPlural -> plural) to validate and
normalize plurals before caching by trimming, lowercasing, and only accepting a
URL-safe path segment (e.g. match /^[a-z0-9-_]+$/i); if the rawPlural fails
validation, fall back to the safe default `${slug}s`; this ensures the cached
plural (variable plural) is always a safe path segment so resolveCollection and
API calls won’t produce broken URLs.
In `@src/Resources/PostResolver.php`:
- Around line 159-167: The adjacent-post container currently calls
adjacentPost() which relies on queryAdjacentPost() fallback that only compares
published_at and can return a different post type or the same row on ties;
update queryAdjacentPost() (used by adjacentPost()) to restrict fallback queries
to the same post type (e.g., WHERE post_type = $post->post_type) and to exclude
the current post id (WHERE ID != $post->ID), and add a deterministic tiebreaker
in the ordering (e.g., ORDER BY published_at ASC/DESC, ID ASC/DESC) so equal
timestamps return the true neighbor rather than a different type or the same
row. Ensure any call sites that expect adjacentPost() behavior need no change.
- Around line 866-979: The authorSocialUrl function currently returns raw
author-provided URLs and can allow dangerous schemes; update authorSocialUrl to
validate and allowlist schemes before returning. After computing $value (the
chosen candidate), trim it, then if it starts with '/' or '#' treat it as a safe
relative URL and return it; otherwise parse the scheme via parse_url($value,
PHP_URL_SCHEME) (lowercased) and only allow a small whitelist such as 'http',
'https', 'mailto', and 'tel'; for 'email' keep the existing mailto:
normalization but only after validation; if the scheme is missing but the value
looks like an email address allow mailto:, and if the scheme is not in the
allowlist return an empty string. Apply these checks in authorSocialUrl to
ensure only safe link targets are stamped.
In `@src/Resources/QueryInliner.php`:
- Around line 339-350: The related-posts query currently hardcodes 'category' in
the $queryAttrs (see where $queryAttrs is built) causing tag-only relatedness to
fail; change the logic to determine taxonomy and term IDs from the host post by
adding a helper like hostRelatedTerms() that returns [taxonomy, termIds] (using
hostTermIds() or extractTermIds($post->categories/$post->tags) as shown in the
proposed snippet), then replace the hardcoded 'taxonomy' => 'category' and
'terms' => $termIds with 'taxonomy' => $taxonomy and 'terms' => $termIds (where
$taxonomy and $termIds come from hostRelatedTerms($host)); apply the same
replacement at the second site where a similar $queryAttrs block is built (the
block around lines 442-460) so both related-post queries use the correct
taxonomy.
---
Minor comments:
In
`@packages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/author-social-icons.blade.php`:
- Line 124: Replace the hardcoded concatenation used for the aria-label on the
anchor with a translatable sprintf-style string: change the aria-label
expression on the ap-author-social-icons__chip anchor to use sprintf( __(
'Author on %s' ), $chip['label'] ) (inside Blade echoing) so the label becomes
translatable via the %s placeholder while preserving the existing $chip['label']
and $styleAttr.
In
`@packages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/search-filters.blade.php`:
- Around line 14-16: In search-filters.blade.php (lines 14-16) the sanitized
$postType can become empty; after the preg_replace call, check if $postType ===
'' and if so set it back to 'post' before rendering the hidden input. In
search-filters-taxonomy.blade.php (lines 19-21) do the same for $taxonomy: after
sanitization check if $taxonomy === '' and if so reset it to 'category' before
using it for request lookups and as the <select name> value; no other changes
are required at the sibling site.
In `@packages/visual-editor-renderer-blade/src/Resolvers/BreadcrumbsResolver.php`:
- Around line 232-236: The 'current' normalization uses "! empty(
$entry['current'] )" which treats strings like "false" as truthy; update the
BreadcrumbsResolver normalized array assignment so the 'current' value is
strictly coerced to boolean (e.g. use filter_var($entry['current'],
FILTER_VALIDATE_BOOLEAN) or an explicit boolean check) instead of !empty(),
replacing the existing 'current' => ! empty( $entry['current'] ) expression in
the block that builds $normalized[].
In
`@packages/visual-editor-renderer-react/src/blocks/artisanpack/breadcrumbs.tsx`:
- Line 2: Remove the internal phase label "CW0 pilot" from the header comment in
the artisanpack/breadcrumbs renderer; edit the header comment that currently
reads "React renderer for the `artisanpack/breadcrumbs` block (CW0 pilot —
`#496`)" to a neutral form such as "React renderer for the
`artisanpack/breadcrumbs` block" so no internal phase labels remain; ensure no
other occurrences of "CW0", "CW1", etc. remain in that header comment or nearby
comments.
In
`@packages/visual-editor-renderer-react/src/blocks/artisanpack/searchCluster.tsx`:
- Around line 67-69: The IDs computed from shortHash (e.g., the inputId constant
created with shortHash(`${label}|${placeholder}|${searchValue}`)) are only
value-derived and can collide across repeated block instances; include a
per-instance unique suffix (for example a renderer/instance identifier passed as
a prop or obtained from context like instanceId or rendererInstanceId) when
constructing these IDs so they are unique per block instance. Update the inputId
expression to append or incorporate that instance identifier (use the same
instance identifier consistently wherever related IDs are computed), and make
the same change to the other ID(s) derived from values later in the file (the ID
at the 145–146 site mentioned in the review) so label/aria relationships remain
correct across multiple instances. Ensure the instance identifier is
required/validated or generated once per block instance and reused for all IDs
in this component.
In `@packages/visual-editor-renderer-vue/src/blocks/artisanpack/breadcrumbs.ts`:
- Line 2: The header comment for the Vue renderer of the
`artisanpack/breadcrumbs` block contains an internal phase label ("CW0 pilot —
`#496`"); remove that phrase so the comment reads simply "Vue renderer for the
`artisanpack/breadcrumbs` block." Locate the top-of-file header comment in the
breadcrumbs renderer and delete the internal phase-label wording, leaving the
rest of the comment intact.
In `@packages/visual-editor-renderer-vue/src/blocks/artisanpack/searchCluster.ts`:
- Around line 72-74: The generated control IDs (e.g., inputId produced via
shortHash(`${label}|${placeholder}|${searchValue}`)) only use attribute content
so identical blocks collide; include a per-instance unique seed when composing
these IDs (for example `${instanceSeed}|${label}|${placeholder}|${searchValue}`)
and persist that seed across renders (use a stable node/block id if available or
generate-and-store one on mount/useRef) so IDs remain unique and stable; apply
this change to the inputId construction shown and the analogous ID generation at
the other site (the occurrence referenced at lines 178-179) so both places use
the same per-instance seed.
In `@resources/js/visual-editor/blocks/breadcrumbs/edit.tsx`:
- Around line 73-76: The SelectControl options use __() with a computed value
(option.label) so i18n extraction will miss strings; update the options
construction so the first argument to __() is a static literal for each label
instead of __((option.label), TEXT_DOMAIN). Concretely, replace the current
SEPARATOR_OPTIONS.map(...) usage in the options prop with either (a) prebuild
SEPARATOR_OPTIONS so each entry's label is already the result of a literal call
like __('Slash', TEXT_DOMAIN) and then use that label directly, or (b) map using
a switch/lookup that passes a string literal into __() for each known option
(e.g., __('Slash', TEXT_DOMAIN), __('Arrow', TEXT_DOMAIN), etc.), keeping value
as option.value; ensure the code references SEPARATOR_OPTIONS and TEXT_DOMAIN so
the correct symbols are updated.
In `@resources/js/visual-editor/blocks/breadcrumbs/index.ts`:
- Around line 5-6: Remove the internal rollout/provenance wording from the top
docblock that mentions `@wordpress/blocks.registerBlockType`—edit the comment to
strip any internal phase labels (e.g., "CW0 pilot", "provenance", or similar
rollout/legacy tags) so only neutral, public-facing documentation remains; leave
the factual description of the block and its behavior but remove internal
provenance phrases.
In `@resources/js/visual-editor/blocks/search-filters/edit.tsx`:
- Around line 62-69: The options array built from getContentTypes() can lack a
'post' entry while the UI still uses value='post', causing an unmatched blank
selection; update the logic that creates options (the options variable) to
ensure a selectable default by checking if options.some(o => o.value === 'post')
and if false add { value: 'post', label: __('Posts', TEXT_DOMAIN) } (use push or
unshift as desired) so 'post' is present whenever missing.
In `@resources/js/visual-editor/blocks/single-content/variations.ts`:
- Around line 23-24: Localize the inserter-visible strings in variations.ts by
replacing the hardcoded title and description with i18n calls: use sprintf( __(
'Single %s', 'your-text-domain' ), type.label ) for title and sprintf( __(
'Render a single %s and scope the inner blocks to it.', 'your-text-domain' ),
type.label.toLowerCase() ) for description (or use __ with concatenation if
preferred), and ensure you import/wp.i18n functions (__, sprintf) at the top of
the file; update the title and description properties accordingly so they are
translatable.
In `@resources/js/visual-editor/blocks/skills-slider/edit.tsx`:
- Around line 131-135: The ARIA range advertised by the progressbar is
inconsistent: aria-valuemin is set to 0 while the control clamps skillLevel to
1–100 (safeLevel) and the inspector only allows 1–100; update the aria-valuemin
attribute to match the actual minimum (set aria-valuemin={1} or use the existing
min constant/prop if one exists) in the element that renders role="progressbar"
(the same JSX that sets aria-valuenow={safeLevel} and aria-label={ariaLabel ||
__('Skill level', TEXT_DOMAIN)}), so the announced range aligns with the real
control bounds.
In `@src/View/Components/VisualEditorComponent.php`:
- Around line 66-69: The PHPDoc for the property $contentTypes and the PHPDoc
for the return shape of resolveContentTypes() are out of sync with actual data
(they omit the "plural" key); update both PHPDoc annotations to use array<int,
array{slug: string, plural: string, label: string}> so the declared shape
matches the array returned by resolveContentTypes().
---
Nitpick comments:
In
`@packages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/author-social-icons.blade.php`:
- Around line 6-120: The template duplicates the normalize/filter/sanitize/style
pipeline (the code that builds $selected/$selectedSlugs, iterates $linksRaw,
calls SocialIconRegistry::author and UrlSanitizer::safe, builds $chips,
$styleParts, $styleAttr, $showIcon and $showLabel); extract that logic into a
single reusable helper (e.g., a function prepare_author_social_icons(array
$attributes): array or a class method) that accepts the block $attributes and
returns an array with keys chips, styleAttr, showIcon, showLabel (and any other
derived values like radius if needed); implement the helper somewhere reusable
(a view helper or service class), move the pipeline code into it (preserving all
validation/sanitization rules and variable names like SocialIconRegistry::author
and UrlSanitizer::safe), and replace the duplicated blocks in each template with
a call to the new helper and assignment from its return value.
In
`@packages/visual-editor-renderer-blade/tests/Feature/BlocksStylesComponentTest.php`:
- Around line 24-42: Add a negative assertion to the existing tests that render
Blade::render('<x-ve-blocks-styles :interactive="false" />') (e.g., the "emits
the grid + marquee..." and/or "emits the breadcrumbs..." examples) to assert
that the interactivity script is not emitted; specifically, add
expect($rendered)->not->toContain(...) for the interactivity asset path
(interactivity.js) and/or the interactivity marker (e.g.,
'data-ve-interactivity') so the test verifies the script is omitted when
:interactive="false".
In `@packages/visual-editor-renderer-vue/tests/BlockTree.test.ts`:
- Around line 576-577: Update the test title string in the it(...) call that
currently reads "carries the `#338` a11y fix forward to artisanpack/search (I4
fork)" by removing the internal phase label "(I4 fork)" and replacing it with a
neutral description (e.g., "carries the `#338` a11y fix forward to
artisanpack/search"); leave the test body and makeBlock(...) invocation
unchanged.
In `@resources/js/visual-editor/blocks/_shared/social-icons.ts`:
- Around line 60-75: SHARE_PLATFORMS is brittle because it references
AUTHOR_PLATFORMS by numeric indices; change it to build the array by slug lookup
(e.g., findPlatform('facebook'), findPlatform('twitter'),
findPlatform('mastodon'), findPlatform('email')) instead of
AUTHOR_PLATFORMS[0/1/2/5]; implement a small helper (e.g.,
getAuthorPlatformBySlug(slug): SocialIconDefinition | undefined) that returns
the matching entry from AUTHOR_PLATFORMS and use it when constructing
SHARE_PLATFORMS, providing the inline literal definitions (reddit, pinterest)
unchanged and falling back to a safe default or excluding missing entries if
find returns undefined.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| $textStyle = 'animation: ap-marquee-scroll ' . $speed . 's linear infinite;'; | ||
| @endphp | ||
| <div{!! $classAttr . $styleAttr . $idAttr !!}> | ||
| <p class="ap-marquee__text" style="{{ $textStyle }}">{!! $content !!}</p> |
There was a problem hiding this comment.
Escape or sanitize marquee content before rendering.
Line 32 outputs $content with raw Blade echo ({!! !!}), which can introduce stored XSS if marqueeContent is user-controlled at any point in the pipeline.
🔧 Proposed fix
- <p class="ap-marquee__text" style="{{ $textStyle }}">{!! $content !!}</p>
+ <p class="ap-marquee__text" style="{{ $textStyle }}">{{ $content }}</p>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <p class="ap-marquee__text" style="{{ $textStyle }}">{!! $content !!}</p> | |
| <p class="ap-marquee__text" style="{{ $textStyle }}">{{ $content }}</p> |
🤖 Prompt for 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.
In
`@packages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/marquee.blade.php`
at line 32, The template currently emits raw HTML for the marquee with {!!
$content !!}, which can cause stored XSS if the marquee content is
user-controlled; change the rendering to either escape the content (use Blade's
escaped echo for $content) or run it through a trusted sanitizer/HTML whitelist
function before echoing, ensuring you still allow safe markup if needed; update
the ap-marquee__text output to use the sanitized/escaped value (replace raw {!!
$content !!} with the escaped/sanitized variant) so $content/marqueeContent is
never emitted unsanitized.
Comment on lines
+7
to
+9
| export default function TabSectionSave(): null { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Persist the inner-block tree in both save entrypoints. Both save components currently return null, so the nested tab sections never serialize and the block tree cannot round-trip.
resources/js/visual-editor/blocks/tab-section/save.tsx#L7-L9: emit the section’s saved inner blocks here.resources/js/visual-editor/blocks/tabs/save.tsx#L7-L9: emit the parent’s saved inner blocks here.
📍 Affects 2 files
resources/js/visual-editor/blocks/tab-section/save.tsx#L7-L9(this comment)resources/js/visual-editor/blocks/tabs/save.tsx#L7-L9
🤖 Prompt for 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.
In `@resources/js/visual-editor/blocks/tab-section/save.tsx` around lines 7 - 9,
Both save entrypoints currently return null which prevents nested blocks from
serializing; update the TabSectionSave
(resources/js/visual-editor/blocks/tab-section/save.tsx lines 7-9) to render the
section's saved inner blocks (use InnerBlocks.Content) instead of null, and
likewise update the TabsSave (resources/js/visual-editor/blocks/tabs/save.tsx
lines 7-9) to render the parent's saved inner blocks (use InnerBlocks.Content);
ensure the appropriate InnerBlocks import (from `@wordpress/block-editor` or
wp.blockEditor) is present at the top of each file so the rendered content is
serialized.
CR's auto-review on #577 surfaced nine inline findings against the crosswinds-port cumulative diff. Seven are addressed below; the remaining two are documented at the bottom with the reason for skipping. config/visual-editor.php — enabled_blocks CR flagged `artisanpack/breadcrumbs` as configured (resolver section present) but missing from the inserter allow-list. The same gap applied to every other CW0–CW7 block — none of the 25 first-party ports had been added. Listing them all in cluster order so the inserter surfaces them and the EnabledBlockNamesTest V1 snapshot stays the source of truth. packages/visual-editor-renderer-blade/src/BlockRenderer.php Propagate the normalized `innerBlocks` tree through both fallback paths in `renderDynamic` — missing dynamic handler and a thrown exception — so static partials downstream still see the full child tree. Also adds a per-render `$renderIndex` counter (resets in `render()`, increments in `renderBlock`, threaded into `renderStatic` and the partial's `$data`) so templates that need a per-instance suffix can stay unique even when sibling blocks carry identical attributes. packages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/{search-field,search-filters-taxonomy}.blade.php Mix `$renderIndex` into the form-control `id`. The attribute-hash suffix stays as a secondary stabilizer so the id is still readable in the single-instance case, but two identical-attribute siblings no longer collide. The label `for=` binding can no longer target the wrong control. resources/js/visual-editor/editor/content-type-registry.ts Constrain `plural` (later interpolated into `${API_BASE}/${plural}` by `SingleContentEdit.resolveCollection()`) to the same `[a-z0-9_-]` shape as the slug, falling back to `${slug}s` for anything malformed. A registry entry with `plural: "../admin"` or similar can no longer produce a broken / traversal-prone REST URL. src/Resources/PostResolver.php — queryAdjacentPost The Eloquent fallback now scopes to the host's `post_type`, excludes the current post id, and adds an id-based tiebreaker on the same `published_at`. Adjacent containers can no longer surface a different post type as the "next post" or — on equal timestamps — return the host post itself. src/Resources/PostResolver.php — authorSocialUrl Validate the email candidate via `FILTER_VALIDATE_EMAIL` before the `mailto:` normalization and reject anything outside an `http`/`https`/`mailto`/`tel` scheme allow-list. Author profile fields can be host-editable; a `javascript:` value there would have rendered as the `href` of a chip on the public frontend. src/Resources/QueryInliner.php — related-posts taxonomy detection Add `hostRelatedTerms()` that returns `[taxonomy, termIds]` by preferring `$post->categories`, falling back to `$post->tags` (`post_tag`), and finally degrading to the legacy `hostTermIds()` scoped to `category` for hosts that only model a generic `terms` collection. The hard-coded `'taxonomy' => 'category'` in `expandRelatedPosts()` is replaced with the resolved taxonomy so tag-only posts now get a related-posts query that exercises the right index. Skipped (documented): marquee.blade.php — `{!! $content !!}` XSS finding. The marquee template follows the same `source: 'html'` / `{!! $content !!}` pattern as 20+ other rich-text blocks (paragraph, heading, quote, pullquote, callout, list, …). The underlying concern is real but the right fix is a package-wide HTML sanitizer at the rendering boundary across all three renderers — not a one-off escape on marquee that would lose the formatted-text round-trip and silently diverge from the rest of the rich-text pipeline. Filing as its own follow-up. tabs/tab-section save returning `null`. CR read this as a round-trip bug. It isn't: tabs/tab-section follow the same dynamic-container pattern as accordions / accordion / accordion-title / accordion-body / grid / grid-item. Gutenberg's serializer preserves the inner-block tree as nested comment delimiters regardless of what `save` returns; the renderers walk the parsed tree, not the saved HTML, so the round-trip is intact.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@config/visual-editor.php`:
- Around line 203-205: Replace the legacy-source and phase-label wording in the
allow-list comments with neutral phrasing that does not mention CW phase labels
or crosswinds terminology; specifically, in config/visual-editor.php (lines
203-205) update the comment to neutral language such as referencing the upstream
change/issue only (e.g., "First-party blocks ported from upstream (see `#495`);
child blocks are also listed so the inserter allow-list does not filter them out
of their parent's template."), and in
tests/Unit/VisualEditor/EnabledBlockNamesTest.php (lines 87-90) make the same
replacement so the test docs use the identical neutral phrasing referencing only
the upstream/issue number.
In `@resources/js/visual-editor/editor/content-type-registry.ts`:
- Around line 68-81: The fallback plural currently trusts the raw slug (`slug`)
which is only trimmed; when `rawPlural` is invalid you must produce a safe
plural using the same constraints as `SAFE_PLURAL_PATTERN` instead of
`${slug}s`. Change the fallback to first normalize/sanitize the slug (e.g.
lower-case, trim, and replace or strip characters that don't match
SAFE_PLURAL_PATTERN) into a new `normalizedSlug`, then set `plural` to
`${normalizedSlug}s` only if `SAFE_PLURAL_PATTERN.test(normalizedSlug)`; if that
still fails, use a safe hardcoded fallback (e.g. "items") so `plural` is
guaranteed to match the safe pattern. Ensure you update the code paths that
compute `normalizedPlural`/`plural` so the `slug`-based fallback runs the same
validation/sanitization.
🪄 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: ed35a0f0-14f5-46ae-a387-7af10b281a3b
📒 Files selected for processing (8)
config/visual-editor.phppackages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/search-field.blade.phppackages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/search-filters-taxonomy.blade.phppackages/visual-editor-renderer-blade/src/BlockRenderer.phpresources/js/visual-editor/editor/content-type-registry.tssrc/Resources/PostResolver.phpsrc/Resources/QueryInliner.phptests/Unit/VisualEditor/EnabledBlockNamesTest.php
🚧 Files skipped from review as they are similar to previous changes (3)
- src/Resources/PostResolver.php
- src/Resources/QueryInliner.php
- packages/visual-editor-renderer-blade/resources/views/blocks/artisanpack/search-filters-taxonomy.blade.php
config/visual-editor.php + tests/Unit/VisualEditor/EnabledBlockNamesTest.php
Rewrite the two allow-list comments I added in the previous fix
commit to drop the legacy-source name and phase labels. The
repo's convention (see CLAUDE.md) is that only the issue number
is acceptable; the rest is noise that leaks rebrand work into the
in-code commentary.
resources/js/visual-editor/editor/content-type-registry.ts
Tighten the plural fallback. The previous round constrained
`plural` against `SAFE_PLURAL_PATTERN` but the fallback path —
`${slug}s` — still trusted an unvalidated slug, so a registry
entry like `{slug: "../admin", label: "X"}` would slip through
and produce a malformed REST collection segment. Lowercase the
slug, run it through the same pattern, and drop the entry
entirely if it fails. The `${slug}s` fallback is now guaranteed
safe because the slug has been validated by the time we get
there.
github-project-automation
Bot
moved this from In Review
to Done
in ArtisanPack UI Overview
16 of 22 tasks
This was referenced Jul 14, 2026
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
Closes umbrella #495 — ports the legacy crosswinds-blocks set into the visual-editor package as first-party
artisanpack/*blocks, mirroring the core-block fork pattern from #331. 25 blocks across 8 clusters (CW0–CW7), all with three-renderer parity (Blade + React + Vue) and renderer parity tests, plus two cross-cutting follow-ups (#565, #566) and a final inserter-icon + category cleanup pass.Closes: #495, #496, #497, #498, #499, #500, #501, #502, #503, #565, #566
Type of Change
Related Issue
Issue: #495
Motivation and Context
The crosswinds-blocks set was a useful starting point but never going to ship under its own name — bringing the blocks in-house as first-party
artisanpack/*blocks gets them the parity guarantees, the renderer-bridge integration, the inserter polish, and the maintenance lifecycle the rest of the visual-editor package already has. Doing the port phase-by-phase (one cluster PR per CW#) kept reviews tractable; this umbrella PR is just the final merge of all of that into release/1.1 plus the inserter-icon + category cleanup we did at the end.Changes Made
CW0 — pilot (#496, merged via #567)
artisanpack/breadcrumbsCW1 — interactive cluster (#497, merged via #568)
artisanpack/accordions,artisanpack/accordion,artisanpack/accordion-title,artisanpack/accordion-bodyartisanpack/tabs,artisanpack/tab-sectionCW2 — grid cluster (#498, merged via #569)
artisanpack/grid(renamed frombasic-grid)artisanpack/grid-item(renamed frombasic-grid-item)CW3 — post navigation cluster (#499, merged via #570)
artisanpack/next-post,artisanpack/previous-postCW4 — site chrome cluster (#500, merged via #571)
artisanpack/copyright,artisanpack/marquee,artisanpack/comments-numberCW5 — single-post content cluster (#501, merged via #572)
artisanpack/single-content,artisanpack/related-postsartisanpack/author-social-icons,artisanpack/social-share-contentCW6 — generic search cluster (#502, merged via #573)
artisanpack/search-field,artisanpack/search-filtersartisanpack/search-filters-buttons,artisanpack/search-filters-taxonomyartisanpack/post-types-search-results,artisanpack/single-post-types-search-resultsCW7 — skills cluster (#503, merged via #574)
artisanpack/skills-sliderCross-cutting follow-ups
_resolvedTrailfor artisanpack/breadcrumbs #565 (merged via feat: add BreadcrumbsResolver for artisanpack/breadcrumbs (#565) #575):BreadcrumbsResolver— stamps_resolvedTrailon the server so the public frontend renders the trail.Final cleanup (this PR's only direct commits)
resources/icons/font-awesome/fas/, FA v7.2.0, CC BY 4.0). The hand-drawn approximations they replaced consistently read as amateur next to the Gutenberg core glyphs.artisanpackinserter category into a real WP one — text/media/design/widgets/theme — plus a newsearchcategory for the six search-cluster blocks. The 25 moves:search(NEW): the 6 search-cluster blocksdesign: accordion family (4), tabs family (2), grid + grid-item, callout, marqueetheme: breadcrumbs, comments-number, copyright, single-content, related-posts, author-social-icons, social-share-contentwidgets: form, skills-slidereditor/custom-blocks.tsaddsSEARCH_CATEGORY_SLUG+ensureSearchCategory().ensureArtisanpackCategory()is retained for downstream apps but is no longer called at registration time.inserter-icon.tsxheader now references NOTICE.md + the CC BY 4.0 license URL.How Has This Been Tested?
Testing Environment:
Tests Performed:
npm test— full vitest run, all 2032 tests across 240 test files pass.npm run verify:parity— renderer parity script clean (151 blocks registered across Blade, React, and Vue).npm run build— production build succeeds./admin/packages/visual-editor/standalone/editor), confirmed:Searchinserter category appears with the 6 search-cluster blocksAccessibility Tests Run
Details: Accessibility checks were run per cluster as part of the CW0–CW7 PR cycle. Each inserter-icon SVG carries
aria-hidden="true"+focusable="false"; the block titles supply the inserter's accessible name. No focus-management or contrast regressions surfaced in this final cleanup.Tests Added
Test details:
ensureSearchCategorysuite ineditor/__tests__/custom-blocks.test.tsxmirrors the existingensureArtisanpackCategoryone.search-cluster-family.test.ts(nowsearch),single-post-content-family.test.ts(nowtheme),skills-slider.test.ts(nowwidgets),site-chrome-family.test.ts(now per-block: copyright=theme, marquee=design, comments-number=theme).artisanpack/calloutassertions incustom-blocks.test.tsxmove fromartisanpacktodesign.Documentation
Documentation details: Inline comments updated on touched files. The new
NOTICE.mddocuments the FA attribution. README / wiki updates for the new blocks will follow in a docs-only PR.Screenshots
The 25 updated inserter icons can be spot-checked in the dev app at
/admin/packages/visual-editor/standalone/editor— open the inserter and scroll through theSearch/Design/Theme/Widgetscategories.Pre-Submission Checklist
Summary by CodeRabbit
New Features
Style / Interactivity
Documentation
Tests