Repository navigation
feat: add BreadcrumbsResolver for artisanpack/breadcrumbs (#565) - #575
ViewFromTheBox merged 2 commits into
Conversation
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>
|
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 (1)
📝 WalkthroughWalkthroughServer-side BreadcrumbsResolver stamps a normalized ChangesBreadcrumbs Resolver and Component Integration
Sequence DiagramsequenceDiagram
participant BlocksComponent
participant BreadcrumbsResolver
participant PostModel
participant TrailFilter
participant BladeRenderer
BlocksComponent->>BreadcrumbsResolver: stampTree(blockTree, post)
BreadcrumbsResolver->>PostModel: collectAncestors(post) / parentPost lookup
BreadcrumbsResolver->>TrailFilter: apply ap.visual-editor.breadcrumbs.trail (if available)
BreadcrumbsResolver->>BreadcrumbsResolver: finalizeTrail(entries)
BreadcrumbsResolver->>BlocksComponent: return stamped tree (_resolvedTrail)
BlocksComponent->>BladeRenderer: render blocks (breadcrumb block reads _resolvedTrail)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/visual-editor-renderer-blade/src/Resolvers/BreadcrumbsResolver.php (1)
175-181: 📐 Maintainability & Code Quality | 💤 Low valueOptional: Simplify redundant conditional.
The
onHomepagecheck at line 175 and the subsequentnull === $postcheck at line 179 both return the same$trailvalue. Since anynullpost results in a Home-only trail (which the finalizer marks as current), theonHomepageguard doesn't gate different behavior.♻️ Suggested simplification
- // Homepage — when no post is in scope and the request URL matches - // the configured home URL, the trail collapses to a single Home - // entry. The finalizer marks it `current: true` so the renderer - // drops the link. - if ( null === $post && $this->onHomepage( $homeUrl ) ) { - return $trail; - } - if ( null === $post ) { return $trail; }🤖 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 175 - 181, The early-return block contains a redundant branch: remove the specific onHomepage($homeUrl) check and consolidate to a single null === $post guard that returns $trail; in the BreadcrumbsResolver (method handling $post/$trail) keep the null check (if ( null === $post ) return $trail;) and delete the preceding if ( null === $post && $this->onHomepage( $homeUrl ) ) branch so behavior is unchanged but the conditional is simplified.
🤖 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.
Nitpick comments:
In `@packages/visual-editor-renderer-blade/src/Resolvers/BreadcrumbsResolver.php`:
- Around line 175-181: The early-return block contains a redundant branch:
remove the specific onHomepage($homeUrl) check and consolidate to a single null
=== $post guard that returns $trail; in the BreadcrumbsResolver (method handling
$post/$trail) keep the null check (if ( null === $post ) return $trail;) and
delete the preceding if ( null === $post && $this->onHomepage( $homeUrl ) )
branch so behavior is unchanged but the conditional is simplified.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1624e946-2d8f-46ca-913b-add58534d57e
📒 Files selected for processing (10)
config/visual-editor.phppackages/visual-editor-renderer-blade/resources/assets/frontend/breadcrumbs.csspackages/visual-editor-renderer-blade/resources/views/components/blocks-styles.blade.phppackages/visual-editor-renderer-blade/src/Resolvers/BreadcrumbsResolver.phppackages/visual-editor-renderer-blade/src/View/Components/BlocksComponent.phppackages/visual-editor-renderer-blade/src/View/Components/BlocksStylesComponent.phppackages/visual-editor-renderer-blade/src/VisualEditorRendererBladeServiceProvider.phppackages/visual-editor-renderer-blade/tests/Feature/BlocksStylesComponentTest.phppackages/visual-editor-renderer-blade/tests/Feature/BreadcrumbsResolverTest.phpresources/js/visual-editor/blocks/breadcrumbs/breadcrumbs.css
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>
63bff5a
into
feature/495-port-crosswinds-blocks
Description
Adds a
BreadcrumbsResolverto the Blade renderer that stamps_resolvedTrailon everyartisanpack/breadcrumbsblock in the saved tree, so the existing Blade / React / Vue renderers (which already consume_resolvedTrail) emit a populated<ol>on the public frontend instead of an empty shell.The default trail is
Home → …ancestors → current. The Home hop reads fromconfig('artisanpack.visual-editor.breadcrumbs.home_url')(falls back tourl('/')); the last entry is auto-markedcurrent: trueso the renderer drops the link and emitsaria-current="page". Hierarchical posts (pages withparent/parent_id) walk the ancestor chain bounded to 32 levels with cycle detection.Hosts customize the trail through the
ap.visual-editor.breadcrumbs.trailfilter (e.g. to insert a 'Category' hop between Home and a blog post) without having to subclass the resolver. A pre-stamped_resolvedTrailon the saved tree wins over the resolver fallback, matchingPostResolver's contract.Also ships
breadcrumbs.cssto 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.Closes: #565
Type of Change
Related Issue
Issue: #565
Motivation and Context
CW0 (#496) ported
crosswinds-blocks/breadcrumbs→artisanpack/breadcrumbswith full Blade + React + Vue renderers that consume a_resolvedTrailattribute, but nothing stamped that attribute server-side — so the block rendered an empty<ol>on the public frontend. This PR is the resolver layer (analogous toPostResolverfor thepost-*family).Changes Made
BreadcrumbsResolverinpackages/visual-editor-renderer-blade/src/Resolvers/walking the block tree and stamping_resolvedTrailon everyartisanpack/breadcrumbsblock.BlocksComponent(runs afterPostResolver, gated behind:resolve-breadcrumbs="false"opt-out) and registered it as scoped in the service provider (matchesLoginoutResolver's lifetime story for Octane / queue workers).breadcrumbssection to the package config withhome_url/home_labeloverrides.breadcrumbs.cssto the frontend asset bundle; wired into<x-ve-blocks-styles />(independent of\$emitInteractive, same asgrid/marquee/social-icons).How Has This Been Tested?
Testing Environment:
Tests Performed:
Accessibility Tests Run
Details:
The renderer emits `aria-label="Breadcrumb"` (configurable via the block's `ariaLabel` attribute) on the wrapping `
` and `aria-current="page"` on the current entry. Schema.org `BreadcrumbList` itemtype annotations are gated on the `breadcrumbsSchema` attribute (default on) — verified both states render correctly.Tests Added
Test details:
Documentation
Documentation details:
`BreadcrumbsResolver` ships PHPDoc on the class, every public method, and the filter hook semantics. The package `config/visual-editor.php` has a documented `breadcrumbs` section describing the override knobs and the filter hook.
Pre-Submission Checklist
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Style