feat: block breadcrumb #225
Conversation
WalkthroughAdds a new weDocs Breadcrumbs Gutenberg block (editor UI, preview, styles, server render), registers it, replaces the monolithic built data store with modular src/data docs/settings stores and bootstrapping, updates multiple built asset manifests and CSS utilities, removes a debug log, and updates .gitignore to ignore Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (18)
src/blocks/Breadcrumb/style.scss (1)
1-3: Avoid duplicating Tailwind base/components in block-local CSS.Emitting base/components per-block can duplicate preflight and increase CSS. Prefer using only utilities here (or ensure base is emitted once globally).
-@tailwind base; -@tailwind components; @tailwind utilities;assets/build/index.css (1)
3069-3079: Scoped utilities look good; minor polish.
- Nice: all utilities are scoped under .wedocs-document to avoid leakage.
- Nits: consider using theme tokens (e.g., primary) instead of hard-coded bg-blue-500; add focus-visible variants where you added hover-only text changes for a11y.
Also applies to: 3224-3227, 3812-3814, 4283-4286, 4726-4728, 5471-5474, 5499-5501
src/blocks/Breadcrumb/render.php (5)
396-398: Separator color doesn’t resolve WP preset tokensWhen set to a preset (e.g., var:preset|color|accent-3), the style becomes invalid. Use get_color_value().
- <span class="mx-2" style="<?php if ($separator_color): ?>color: <?php echo esc_attr($separator_color); ?>;<?php endif; ?>"> + <span class="mx-2" style="<?php if ($separator_color): ?>color: <?php echo esc_attr(get_color_value($separator_color)); ?>;<?php endif; ?>">
238-251: Page ancestors likely rendered in reverse (child→parent) orderget_post_ancestors() returns closest parent first. Breadcrumbs usually need top-level→current. Reverse before iterating.
- $ancestors = get_post_ancestors($post->ID); + $ancestors = array_reverse(get_post_ancestors($post->ID));
123-128: Border width should add units when numericRaw numeric values (e.g., "1") become invalid in CSS. Append "px" if no unit present.
- function get_border_width_value($width_value) { + function get_border_width_value($width_value) { if (empty($width_value)) { return ''; } - return $width_value; + // If numeric or all-digits string, default to px + if (is_numeric($width_value)) { + return rtrim((string)$width_value) . 'px'; + } + return $width_value; }
43-45: Unsafe nested array access can emit PHP notices$elements['link']['color']['text'] may not exist; direct indexing can raise notices. Use wp_array_get() or isset-chain.
-$elements = $style['elements'] ?? []; -$link_color = $elements['link']['color']['text'] ?? ''; +$elements = $style['elements'] ?? []; +$link_color = function_exists('wp_array_get') + ? wp_array_get($elements, ['link', 'color', 'text'], '') + : (isset($elements['link']['color']['text']) ? $elements['link']['color']['text'] : '');
299-307: Search/404 cases not handledis_search() and is_404() currently fall through, yielding only Home. Consider explicit labels (e.g., “Search results for ...”, “404 Not Found”).
wedocs.php (1)
171-175: Typo“registeer our widget” in the comment. Harmless, but easy win to fix.
src/blocks/Breadcrumb/Inspector.js (3)
41-58: Use consistent text domain ("wedocs")The plugin’s text domain is "wedocs". Using "wedocs-blocks" will block translations.
- <PanelBody title={__("Settings", "wedocs-blocks")}> + <PanelBody title={__("Settings", "wedocs")}> ... - label={__("Separator Icon", "wedocs-blocks")} + label={__("Separator Icon", "wedocs")} ... - <ToggleControl - label={__("Hide Home Icon", "wedocs-blocks")} + <ToggleControl + label={__("Hide Home Icon", "wedocs")} ... - <PanelBody - title={__("Breadcrumb Separator", "wedocs-blocks")} + <PanelBody + title={__("Breadcrumb Separator", "wedocs")} ... - {__("Color", "wedocs-blocks")} + {__("Color", "wedocs")} ... - name: __("Theme", "wedocs-blocks"), + name: __("Theme", "wedocs"),Also applies to: 60-69, 74-75, 91-92, 127-131
100-116: Button label reads “Separator Icon” in Color pickerThis control sets color; label should say “Separator Color”.
- <ColorIndicator colorValue={breadcrumbSeparator.color} /> - <Text>{__("Separator Icon", "wedocs-blocks")}</Text> + <ColorIndicator colorValue={breadcrumbSeparator.color} /> + <Text>{__("Separator Color", "wedocs")}</Text>Also applies to: 112-114
28-34: Remove unused themeGradientsFetched but unused.
- const { themeColors, themeGradients } = useSelect((select) => { + const { themeColors } = useSelect((select) => { const editorSettings = select("core/block-editor").getSettings(); return { themeColors: editorSettings.colors, - themeGradients: editorSettings.gradients, }; });src/blocks/Breadcrumb/style.css (2)
66-68: Style the current crumb regardless of element type.The current item is a
<span aria-current="page">in PHP. Your CSS only targets links. Add a generic selector.Apply this diff:
-.wp-block-wedocs-breadcrumbs a[aria-current="page"] { - @apply text-gray-900 font-medium; -} +.wp-block-wedocs-breadcrumbs [aria-current="page"] { + @apply text-gray-900 font-medium; +}
43-49: Avoid relying on raw Tailwind utilities in SSR markup unless guaranteed in the built CSS.Classes like
space-x-4,hover:opacity-80,sr-onlywon’t be included by Purge/JIT unless referenced in build-time sources. Prefer block‑scoped CSS (using@apply) for SSR output, or safelist these utilities in the Tailwind config.src/blocks/Breadcrumb/edit.js (1)
268-276: Double spacing between items (space-x-4 + ml-4).You already add
space-x-4on the<ol>. Drop the item-levelml-4to avoid compounded gaps.Apply this diff:
- className={`${index === 0 ? '' : 'ml-4'} text-sm font-medium hover:opacity-80`} + className="text-sm font-medium hover:opacity-80"- className={`${index === 0 ? '' : 'ml-4'} text-sm font-medium`} + className="text-sm font-medium"Also applies to: 279-287
assets/build/blocks/Breadcrumb/render.php (4)
239-251: Page ancestor order is reversed.
get_post_ancestors()returns from closest parent up. Breadcrumbs should show topmost → current. Reverse the array before iterating.Apply this diff:
-$ancestors = get_post_ancestors($post->ID); +$ancestors = array_reverse( get_post_ancestors( $post->ID ) );
396-398: Separator color needs token-to-CSS conversion.
$separator_colormay be a preset token (var:preset|color|...) or a slug. Convert viaget_color_value().Apply this diff:
-<span class="mx-2" style="<?php if ($separator_color): ?>color: <?php echo esc_attr($separator_color); ?>;<?php endif; ?>"> +<span class="mx-2" style="<?php if ($separator_color): ?>color: <?php echo esc_attr( get_color_value( $separator_color ) ); ?>;<?php endif; ?>">
350-371: Class construction for color/border/link may be ineffective.Slug-prefixed classes like
has-*-border-colorexpect slugs, not raw tokens/hex. You’re usingsanitize_key($effective_border_color)without strippingvar:preset|color|. For links you strip the prefix, for borders you don’t. Standardize or rely on inline styles.
420-431: Tailwind utilities in SSR markup may be purged.Classes like
hover:opacity-80,ml-4,space-x-4,sr-onlymay not ship in production CSS. Prefer block-scoped CSS rules (e.g.,.wp-block-wedocs-breadcrumbs a { @apply ... }) or safelist utilities in Tailwind config.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
.gitignore(1 hunks)assets/build/block.asset.php(1 hunks)assets/build/blocks/Breadcrumb/block.json(1 hunks)assets/build/blocks/Breadcrumb/render.php(1 hunks)assets/build/frontend.asset.php(1 hunks)assets/build/index.asset.php(1 hunks)assets/build/index.css(7 hunks)assets/build/print.asset.php(1 hunks)assets/build/store.asset.php(1 hunks)src/blocks/Breadcrumb/Inspector.js(1 hunks)src/blocks/Breadcrumb/block.json(1 hunks)src/blocks/Breadcrumb/edit.js(1 hunks)src/blocks/Breadcrumb/index.js(1 hunks)src/blocks/Breadcrumb/render.php(1 hunks)src/blocks/Breadcrumb/style.css(1 hunks)src/blocks/Breadcrumb/style.scss(1 hunks)src/blocks/Search/edit.js(0 hunks)src/blocks/index.js(1 hunks)wedocs.php(1 hunks)
💤 Files with no reviewable changes (1)
- src/blocks/Search/edit.js
🧰 Additional context used
🧬 Code graph analysis (5)
src/blocks/Breadcrumb/index.js (1)
src/blocks/Breadcrumb/edit.js (1)
Edit(6-297)
src/blocks/Breadcrumb/Inspector.js (1)
src/blocks/Breadcrumb/edit.js (1)
attributes(42-42)
src/blocks/Breadcrumb/edit.js (1)
src/blocks/Breadcrumb/Inspector.js (2)
attributes(21-25)Inspector(20-148)
assets/build/blocks/Breadcrumb/render.php (2)
src/blocks/Breadcrumb/edit.js (8)
attributes(42-42)spacing(149-149)border(150-150)typography(151-151)padding(153-153)margin(154-154)shadow(152-152)breadcrumbs(185-198)src/blocks/Breadcrumb/Inspector.js (1)
attributes(21-25)
src/blocks/Breadcrumb/render.php (3)
src/blocks/Breadcrumb/edit.js (8)
attributes(42-42)spacing(149-149)border(150-150)typography(151-151)padding(153-153)margin(154-154)shadow(152-152)breadcrumbs(185-198)src/blocks/Breadcrumb/Inspector.js (1)
attributes(21-25)assets/build/blocks/Breadcrumb/render.php (9)
get_spacing_value(52-83)get_color_value(88-105)get_border_radius_value(110-119)get_border_width_value(124-129)get_typography_value(134-157)get_shadow_value(162-179)get_separator_icon(184-197)get_breadcrumb_items(202-319)get_doc_ancestors(323-338)
🔇 Additional comments (16)
assets/build/index.asset.php (1)
1-1: Verify WP min-version supports 'react' and 'react-dom' handles.If you support older WP, these handles may be missing; relying on wp-element alone is safer. Please confirm target WP versions.
assets/build/frontend.asset.php (1)
1-1: LGTM: version bump only.assets/build/print.asset.php (1)
1-1: LGTM: version bump only.assets/build/store.asset.php (1)
1-1: LGTM: dependencies consistent with data/store usage.assets/build/block.asset.php (1)
1-1: Good addition of wp-data for editor store usage..gitignore (2)
20-20: PR objective mismatch — linked issue references anasyncbump but this PR doesn't change dependenciespackage-lock.json contains
asynconly as a transitive dep (archiver ->async^3.2.4) and neither package.json nor package-lock.json are modified in this PR. Either include the dependency bump (package.json + lockfile) or update the PR description/linked issue to match the actual changes.
20-20: Don't ignore built assets — remove /assets/build/ or ensure CI builds & uploads them
- Found "/assets/build/" in .gitignore (line 20).
- Either remove that ignore so built assets are tracked for distribution, or confirm/add a release workflow that runs the frontend build and uploads the built artifacts (upload-artifact / create-release).
- Verification script exited early; unable to confirm CI packaging — confirm the build-and-attach workflow exists.
src/blocks/index.js (1)
3-3: Breadcrumb import: LGTMKeeps bundle registration consistent with other blocks.
src/blocks/Breadcrumb/index.js (1)
6-9: Block registration: LGTMMetadata registration with dynamic save is correct.
Confirm block.json’s "render": "file:./render.php" is present in the built path (assets/build/blocks/Breadcrumb) so PHP render is picked up in production.
wedocs.php (2)
186-189: Block registration path: LGTM; verify asset presenceRegistering via assets/build/blocks/Breadcrumb relies on block.json to include render.php. Ensure assets/build is shipped with releases.
186-189: Linked issue seems unrelatedPR links issue “Bump async from 2.6.3 to 3.2.2”, which doesn’t match these changes. Avoid cross‑repo noise and update linkage if accidental.
src/blocks/Breadcrumb/Inspector.js (1)
8-17: Heavy reliance on __experimental componentsThese props/components can change. Ensure minimum WP/Gutenberg version is documented, or provide fallbacks.
Also applies to: 39-71, 72-146
src/blocks/Breadcrumb/edit.js (1)
200-226: Inline style mapping looks solid.Good translation of WP style tokens (spacing, border, typography, shadow) to CSS variables for preview.
src/blocks/Breadcrumb/block.json (2)
35-77: Check WordPress minimum version for experimental support keys.You’re using
__experimentalBorderand several__experimental*typography flags. Ensure your minimum WP version supports these (or migrate to stable keys where possible).
1-34: Manifest reads well.Attributes and example align with the editor/renderer. No issues.
assets/build/blocks/Breadcrumb/block.json (1)
1-79: Built manifest mirrors the source manifest correctly.Looks consistent. Keep source of truth in
src/and ensure build is regenerated on changes.
| if (!function_exists('get_spacing_value')) { | ||
| function get_spacing_value($spacing_value) { | ||
| if (empty($spacing_value)) { | ||
| return ''; | ||
| } | ||
|
|
||
| // Handle object with individual sides | ||
| if (is_array($spacing_value)) { | ||
| $sides = ['top', 'right', 'bottom', 'left']; | ||
| $values = []; | ||
| foreach ($sides as $side) { | ||
| $value = $spacing_value[$side] ?? '0'; | ||
| if (strpos($value, 'var:preset|spacing|') === 0) { | ||
| $spacing_name = str_replace('var:preset|spacing|', '', $value); | ||
| $values[] = "var(--wp--preset--spacing--{$spacing_name})"; | ||
| } else { | ||
| $values[] = $value; | ||
| } | ||
| } | ||
| return implode(' ', $values); | ||
| } | ||
|
|
||
| // Handle string values | ||
| if (is_string($spacing_value)) { | ||
| if (strpos($spacing_value, 'var:preset|spacing|') === 0) { | ||
| $spacing_name = str_replace('var:preset|spacing|', '', $spacing_value); | ||
| return "var(--wp--preset--spacing--{$spacing_name})"; | ||
| } | ||
| return $spacing_value; | ||
| } | ||
|
|
||
| return ''; | ||
| } | ||
| } | ||
|
|
||
| // Helper function to convert WordPress color to CSS | ||
| if (!function_exists('get_color_value')) { | ||
| function get_color_value($color) { | ||
| if (empty($color)) { | ||
| return ''; | ||
| } | ||
|
|
||
| // Handle preset color format: var:preset|color|accent-3 | ||
| if (strpos($color, 'var:preset|color|') === 0) { | ||
| $color_name = str_replace('var:preset|color|', '', $color); | ||
| return "var(--wp--preset--color--{$color_name})"; | ||
| } | ||
|
|
||
| // Handle direct preset color names: accent-3, primary, etc. | ||
| if (preg_match('/^[a-z0-9-]+$/', $color) && !strpos($color, '#') && !strpos($color, 'rgb')) { | ||
| return "var(--wp--preset--color--{$color})"; | ||
| } | ||
|
|
||
| return $color; | ||
| } | ||
| } | ||
|
|
||
| // Helper function to convert WordPress border radius to CSS | ||
| if (!function_exists('get_border_radius_value')) { | ||
| function get_border_radius_value($radius_value) { | ||
| if (empty($radius_value)) { | ||
| return ''; | ||
| } | ||
| if (strpos($radius_value, 'var:preset|spacing|') === 0) { | ||
| $radius_name = str_replace('var:preset|spacing|', '', $radius_value); | ||
| return "var(--wp--preset--spacing--{$radius_name})"; | ||
| } | ||
| return $radius_value; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prefix helper function names to avoid global collisions.
Names like get_spacing_value, get_color_value, get_breadcrumb_items are very generic. If another plugin defines them, your guards will bind to the other implementation with unpredictable results. Prefix with wedocs_ and update local calls.
🤖 Prompt for AI Agents
In assets/build/blocks/Breadcrumb/render.php around lines 51 to 121, the helper
functions use generic global names (get_spacing_value, get_color_value,
get_border_radius_value) which risk collisions; rename them to prefixed versions
(e.g. wedocs_get_spacing_value, wedocs_get_color_value,
wedocs_get_border_radius_value) and update any local calls within this file to
use the new names, keep the same logic and function_exists guards updated to the
new names to avoid breaking other plugins.
| ?> | ||
| <div class="wedocs-document"> | ||
| <nav aria-label="Breadcrumb" class="flex"> | ||
| <?php |
There was a problem hiding this comment.
Missing block wrapper attributes breaks supports and scoped CSS.
Wrap the block with get_block_wrapper_attributes() so WP adds wp-block-wedocs-breadcrumbs, custom className, and style-support classes. This also fixes the .wp-block-wedocs-breadcrumbs CSS not applying.
Apply this diff:
-<?php
-?>
-<div class="wedocs-document">
+<?php
+$wrapper_attributes = function_exists( 'get_block_wrapper_attributes' )
+ ? get_block_wrapper_attributes( 'wedocs-document' )
+ : 'class="wedocs-document"';
+?>
+<div <?php echo $wrapper_attributes; ?>>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In assets/build/blocks/Breadcrumb/render.php around lines 345–348, the root
block wrapper is a plain <div class="wedocs-document"> which prevents WordPress
from adding the wp-block-wedocs-breadcrumbs, user className and style-support
classes; replace that static opening div with a div that echoes
get_block_wrapper_attributes() (passing the existing wedocs-document class via
the class attribute) so WP injects block, custom className and style-support
classes, preserving the original wedocs-document class.
| <span <?php echo $index === 0 ? '' : 'class="ml-4"'; ?> aria-current="page" class="text-sm font-medium text-gray-500 hover:text-gray-700" style="<?php if ($font_size_from_style): ?>font-size: <?php echo esc_attr(get_typography_value($font_size_from_style)); ?>;<?php elseif ($font_size): ?>font-size: <?php echo esc_attr(get_typography_value($font_size)); ?>;<?php endif; ?>"> | ||
| <?php echo esc_html($breadcrumb['title']); ?> | ||
| </span> |
There was a problem hiding this comment.
Duplicate class attributes on the current-page span.
You output class twice; the first is overridden. Consolidate into one.
Apply this diff:
-<span <?php echo $index === 0 ? '' : 'class="ml-4"'; ?> aria-current="page" class="text-sm font-medium text-gray-500 hover:text-gray-700" style="<?php if ($font_size_from_style): ?>font-size: <?php echo esc_attr(get_typography_value($font_size_from_style)); ?>;<?php elseif ($font_size): ?>font-size: <?php echo esc_attr(get_typography_value($font_size)); ?>;<?php endif; ?>">
+<span class="<?php echo $index === 0 ? '' : 'ml-4 '; ?>text-sm font-medium text-gray-500 hover:text-gray-700" aria-current="page" style="<?php if ($font_size_from_style): ?>font-size: <?php echo esc_attr(get_typography_value($font_size_from_style)); ?>;<?php elseif ($font_size): ?>font-size: <?php echo esc_attr(get_typography_value($font_size)); ?>;<?php endif; ?>">📝 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.
| <span <?php echo $index === 0 ? '' : 'class="ml-4"'; ?> aria-current="page" class="text-sm font-medium text-gray-500 hover:text-gray-700" style="<?php if ($font_size_from_style): ?>font-size: <?php echo esc_attr(get_typography_value($font_size_from_style)); ?>;<?php elseif ($font_size): ?>font-size: <?php echo esc_attr(get_typography_value($font_size)); ?>;<?php endif; ?>"> | |
| <?php echo esc_html($breadcrumb['title']); ?> | |
| </span> | |
| <span class="<?php echo $index === 0 ? '' : 'ml-4 '; ?>text-sm font-medium text-gray-500 hover:text-gray-700" aria-current="page" style="<?php if ($font_size_from_style): ?>font-size: <?php echo esc_attr(get_typography_value($font_size_from_style)); ?>;<?php elseif ($font_size): ?>font-size: <?php echo esc_attr(get_typography_value($font_size)); ?>;<?php endif; ?>"> | |
| <?php echo esc_html($breadcrumb['title']); ?> | |
| </span> |
🤖 Prompt for AI Agents
In assets/build/blocks/Breadcrumb/render.php around lines 435 to 437, the <span>
for the current page has duplicate class attributes (one conditional for 'ml-4'
and one for text styling) causing the first to be ignored; consolidate into a
single class attribute that conditionally includes "ml-4" when $index !== 0
alongside the existing classes ("text-sm font-medium text-gray-500
hover:text-gray-700") and remove the duplicated attribute so only one class
attribute remains; leave the aria-current and inline style logic intact.
| const blockProps = useBlockProps(); | ||
| const { separator, hideHomeIcon, breadcrumbSeparator } = attributes; | ||
|
|
||
|
|
There was a problem hiding this comment.
Use block wrapper props so supports and block class apply in the editor.
useBlockProps() is declared but not used. This drops the wp-block-wedocs-breadcrumbs class, breaking scoped styles and support classes.
Apply this diff:
-const blockProps = useBlockProps();
+const blockProps = useBlockProps({ className: "wedocs-document" });And in the JSX:
-<div className="wedocs-document">
+<div {...blockProps}>📝 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.
| const blockProps = useBlockProps(); | |
| const { separator, hideHomeIcon, breadcrumbSeparator } = attributes; | |
| const blockProps = useBlockProps({ className: "wedocs-document" }); | |
| const { separator, hideHomeIcon, breadcrumbSeparator } = attributes; |
🤖 Prompt for AI Agents
In src/blocks/Breadcrumb/edit.js around lines 41 to 44, useBlockProps() is
called but its return value isn't applied to the block's root JSX wrapper, which
removes the wp-block-wedocs-breadcrumbs class and breaks scoped styles and
supports; fix by spreading the blockProps returned by useBlockProps() onto the
top-level wrapper element in the edit component (ensure you merge/append any
existing className rather than overriding it) so block wrapper props and support
classes are applied in the editor.
| if (!function_exists('get_spacing_value')) { | ||
| function get_spacing_value($spacing_value) { | ||
| if (empty($spacing_value)) { | ||
| return ''; | ||
| } | ||
|
|
||
| // Handle object with individual sides | ||
| if (is_array($spacing_value)) { | ||
| $sides = ['top', 'right', 'bottom', 'left']; | ||
| $values = []; | ||
| foreach ($sides as $side) { | ||
| $value = $spacing_value[$side] ?? '0'; | ||
| if (strpos($value, 'var:preset|spacing|') === 0) { | ||
| $spacing_name = str_replace('var:preset|spacing|', '', $value); | ||
| $values[] = "var(--wp--preset--spacing--{$spacing_name})"; | ||
| } else { | ||
| $values[] = $value; | ||
| } | ||
| } | ||
| return implode(' ', $values); | ||
| } | ||
|
|
||
| // Handle string values | ||
| if (is_string($spacing_value)) { | ||
| if (strpos($spacing_value, 'var:preset|spacing|') === 0) { | ||
| $spacing_name = str_replace('var:preset|spacing|', '', $spacing_value); | ||
| return "var(--wp--preset--spacing--{$spacing_name})"; | ||
| } | ||
| return $spacing_value; | ||
| } | ||
|
|
||
| return ''; | ||
| } | ||
| } | ||
|
|
||
| // Helper function to convert WordPress color to CSS | ||
| if (!function_exists('get_color_value')) { | ||
| function get_color_value($color) { | ||
| if (empty($color)) { | ||
| return ''; | ||
| } | ||
|
|
||
| // Handle preset color format: var:preset|color|accent-3 | ||
| if (strpos($color, 'var:preset|color|') === 0) { | ||
| $color_name = str_replace('var:preset|color|', '', $color); | ||
| return "var(--wp--preset--color--{$color_name})"; | ||
| } | ||
|
|
||
| // Handle direct preset color names: accent-3, primary, etc. | ||
| if (preg_match('/^[a-z0-9-]+$/', $color) && !strpos($color, '#') && !strpos($color, 'rgb')) { | ||
| return "var(--wp--preset--color--{$color})"; | ||
| } | ||
|
|
||
| return $color; | ||
| } | ||
| } | ||
|
|
||
| // Helper function to convert WordPress border radius to CSS | ||
| if (!function_exists('get_border_radius_value')) { | ||
| function get_border_radius_value($radius_value) { | ||
| if (empty($radius_value)) { | ||
| return ''; | ||
| } | ||
| if (strpos($radius_value, 'var:preset|spacing|') === 0) { | ||
| $radius_name = str_replace('var:preset|spacing|', '', $radius_value); | ||
| return "var(--wp--preset--spacing--{$radius_name})"; | ||
| } | ||
| return $radius_value; | ||
| } | ||
| } | ||
|
|
||
| // Helper function to convert WordPress border width to CSS | ||
| if (!function_exists('get_border_width_value')) { | ||
| function get_border_width_value($width_value) { | ||
| if (empty($width_value)) { | ||
| return ''; | ||
| } | ||
| return $width_value; | ||
| } | ||
| } | ||
|
|
||
| // Helper function to convert WordPress typography to CSS | ||
| if (!function_exists('get_typography_value')) { | ||
| function get_typography_value($typography_value) { | ||
| if (empty($typography_value)) { | ||
| return ''; | ||
| } | ||
|
|
||
| // Handle preset font size format: var:preset|font-size|medium | ||
| if (strpos($typography_value, 'var:preset|font-size|') === 0) { | ||
| $font_size_name = str_replace('var:preset|font-size|', '', $typography_value); | ||
| return "var(--wp--preset--font-size--{$font_size_name})"; | ||
| } | ||
|
|
||
| // Handle preset font family format: var:preset|font-family|system-font | ||
| if (strpos($typography_value, 'var:preset|font-family|') === 0) { | ||
| $font_family_name = str_replace('var:preset|font-family|', '', $typography_value); | ||
| return "var(--wp--preset--font-family--{$font_family_name})"; | ||
| } | ||
|
|
||
| // Handle direct preset font size names: medium, large, x-large, xx-large, etc. | ||
| if (preg_match('/^(small|medium|large|x-large|xx-large|xxx-large)$/', $typography_value)) { | ||
| return "var(--wp--preset--font-size--{$typography_value})"; | ||
| } | ||
|
|
||
| return $typography_value; | ||
| } | ||
| } | ||
|
|
||
| // Helper function to convert WordPress shadow to CSS | ||
| if (!function_exists('get_shadow_value')) { | ||
| function get_shadow_value($shadow_value) { | ||
| if (empty($shadow_value)) { | ||
| return ''; | ||
| } | ||
|
|
||
| // Handle preset shadow format: var:preset|shadow|natural | ||
| if (strpos($shadow_value, 'var:preset|shadow|') === 0) { | ||
| $shadow_name = str_replace('var:preset|shadow|', '', $shadow_value); | ||
| return "var(--wp--preset--shadow--{$shadow_name})"; | ||
| } | ||
|
|
||
| // Handle direct preset shadow names: natural, deep, sharp, outlined, crisp | ||
| if (preg_match('/^(natural|deep|sharp|outlined|crisp)$/', $shadow_value)) { | ||
| return "var(--wp--preset--shadow--{$shadow_value})"; | ||
| } | ||
|
|
||
| return $shadow_value; | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Prefix global helpers to avoid collisions
Functions like get_spacing_value(), get_color_value(), get_breadcrumb_items(), etc., are globally scoped and generic. Another plugin can define them, altering behavior. Prefix with wedocs_ and update references.
Also applies to: 182-201, 322-339
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf "Searching repository for global helper definitions/usages...\n\n"
functions=(get_spacing_value get_color_value get_breadcrumb_items get_border_radius_value get_border_width_value get_typography_value get_shadow_value)
for f in "${functions[@]}"; do
printf "===== DEFINITIONS for: %s =====\n" "$f"
if command -v rg >/dev/null 2>&1; then
rg -n --hidden --no-ignore -S "function\s+$f\s*\(" || true
else
grep -nR --binary-files=without-match -E "function\s+$f\s*\(" || true
fi
printf "\n"
printf "===== USAGES for: %s =====\n" "$f"
if command -v rg >/dev/null 2>&1; then
rg -n --hidden --no-ignore -S "\b$f\s*\(" || true
else
grep -nR --binary-files=without-match -E "\b$f\s*\(" || true
fi
printf "\n\n"
done
printf "===== Files named render.php =====\n"
find . -type f -name 'render.php' -print || true
printf "\n"
# Show the Breadcrumb render file if it exists
candidate="src/blocks/Breadcrumb/render.php"
if [ -f "$candidate" ]; then
printf "=== Showing %s (1-400) ===\n" "$candidate"
sed -n '1,400p' "$candidate" || true
else
printf "%s not found; showing first matching render.php (if any):\n" "$candidate"
first=$(find . -type f -name 'render.php' | head -n1 || true)
if [ -n "$first" ]; then
printf "=== %s (1-400) ===\n" "$first"
sed -n '1,400p' "$first" || true
else
printf "No render.php files found in repo.\n"
fi
fiLength of output: 23270
Prefix global helper functions with wedocs_ to avoid collisions
Rename the globally-scoped helpers defined in src/blocks/Breadcrumb/render.php (and mirrored in assets/build/blocks/Breadcrumb/render.php) and update all call sites: get_spacing_value → wedocs_get_spacing_value; get_color_value → wedocs_get_color_value; get_breadcrumb_items → wedocs_get_breadcrumb_items; get_border_radius_value → wedocs_get_border_radius_value; get_border_width_value → wedocs_get_border_width_value; get_typography_value → wedocs_get_typography_value; get_shadow_value → wedocs_get_shadow_value; get_separator_icon → wedocs_get_separator_icon; get_doc_ancestors → wedocs_get_doc_ancestors. Change the source (src/...) and regenerate assets/build rather than only editing built files.
🤖 Prompt for AI Agents
In src/blocks/Breadcrumb/render.php around lines 51 to 180 the review asks to
avoid global helper collisions by renaming all globally-scoped helper functions
with a wedocs_ prefix and updating their call sites; rename get_spacing_value →
wedocs_get_spacing_value, get_color_value → wedocs_get_color_value,
get_breadcrumb_items → wedocs_get_breadcrumb_items, get_border_radius_value →
wedocs_get_border_radius_value, get_border_width_value →
wedocs_get_border_width_value, get_typography_value →
wedocs_get_typography_value, get_shadow_value → wedocs_get_shadow_value,
get_separator_icon → wedocs_get_separator_icon, and get_doc_ancestors →
wedocs_get_doc_ancestors in this source file, then update every place in the
codebase that calls these functions to use the new names, and finally regenerate
the built file assets/build/blocks/Breadcrumb/render.php from the updated src
files so the built output reflects the renames (do not only edit built files).
| // Add color classes | ||
| if ($background_color) { | ||
| $ol_classes[] = 'has-background'; | ||
| $ol_classes[] = 'has-' . sanitize_key($background_color) . '-background-color'; | ||
| } | ||
| if ($text_color) { | ||
| $ol_classes[] = 'has-text-color'; | ||
| $ol_classes[] = 'has-' . sanitize_key($text_color) . '-color'; | ||
| } | ||
| // Use border color from style system if available, fallback to attribute | ||
| $effective_border_color = $border_color_from_style ?: $border_color; | ||
| if ($effective_border_color) { | ||
| $ol_classes[] = 'has-border-color'; | ||
| $ol_classes[] = 'has-' . sanitize_key($effective_border_color) . '-border-color'; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Color classes may be wrong for preset tokens
For background/text/border, if attributes contain "var:preset|color|slug", sanitize_key() yields invalid class names (e.g., has-varpreset...). Normalize to the slug first.
+ // Helper: map color attr to class slug
+ $to_color_slug = function($color) {
+ if (strpos($color, 'var:preset|color|') === 0) {
+ return sanitize_key(str_replace('var:preset|color|', '', $color));
+ }
+ return sanitize_key($color);
+ };
// Add color classes
if ($background_color) {
$ol_classes[] = 'has-background';
- $ol_classes[] = 'has-' . sanitize_key($background_color) . '-background-color';
+ $ol_classes[] = 'has-' . $to_color_slug($background_color) . '-background-color';
}
if ($text_color) {
$ol_classes[] = 'has-text-color';
- $ol_classes[] = 'has-' . sanitize_key($text_color) . '-color';
+ $ol_classes[] = 'has-' . $to_color_slug($text_color) . '-color';
}
// Use border color from style system if available, fallback to attribute
$effective_border_color = $border_color_from_style ?: $border_color;
if ($effective_border_color) {
$ol_classes[] = 'has-border-color';
- $ol_classes[] = 'has-' . sanitize_key($effective_border_color) . '-border-color';
+ $ol_classes[] = 'has-' . $to_color_slug($effective_border_color) . '-border-color';
}📝 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.
| // Add color classes | |
| if ($background_color) { | |
| $ol_classes[] = 'has-background'; | |
| $ol_classes[] = 'has-' . sanitize_key($background_color) . '-background-color'; | |
| } | |
| if ($text_color) { | |
| $ol_classes[] = 'has-text-color'; | |
| $ol_classes[] = 'has-' . sanitize_key($text_color) . '-color'; | |
| } | |
| // Use border color from style system if available, fallback to attribute | |
| $effective_border_color = $border_color_from_style ?: $border_color; | |
| if ($effective_border_color) { | |
| $ol_classes[] = 'has-border-color'; | |
| $ol_classes[] = 'has-' . sanitize_key($effective_border_color) . '-border-color'; | |
| } | |
| // Helper: map color attr to class slug | |
| $to_color_slug = function($color) { | |
| if (strpos($color, 'var:preset|color|') === 0) { | |
| return sanitize_key(str_replace('var:preset|color|', '', $color)); | |
| } | |
| return sanitize_key($color); | |
| }; | |
| // Add color classes | |
| if ($background_color) { | |
| $ol_classes[] = 'has-background'; | |
| $ol_classes[] = 'has-' . $to_color_slug($background_color) . '-background-color'; | |
| } | |
| if ($text_color) { | |
| $ol_classes[] = 'has-text-color'; | |
| $ol_classes[] = 'has-' . $to_color_slug($text_color) . '-color'; | |
| } | |
| // Use border color from style system if available, fallback to attribute | |
| $effective_border_color = $border_color_from_style ?: $border_color; | |
| if ($effective_border_color) { | |
| $ol_classes[] = 'has-border-color'; | |
| $ol_classes[] = 'has-' . $to_color_slug($effective_border_color) . '-border-color'; | |
| } |
🤖 Prompt for AI Agents
In src/blocks/Breadcrumb/render.php around lines 352 to 366, the code builds
color class names by calling sanitize_key() directly on attribute values which
can be preset tokens like "var:preset|color|slug" (resulting in invalid class
names such as has-varpreset...), so change the logic to detect preset token
values and normalize them to the actual slug before sanitizing: if the value
starts with "var:preset|", extract the last segment (the slug) or parse after
the final '|' and use that slug for sanitize_key(); otherwise fall back to the
original value; apply this normalization to background_color, text_color, and
effective_border_color before composing the class names.
src/blocks/Breadcrumb/render.php
Outdated
| <span <?php echo $index === 0 ? '' : 'class="ml-4"'; ?> aria-current="page" class="text-sm font-medium text-gray-500 hover:text-gray-700" style="<?php if ($font_size_from_style): ?>font-size: <?php echo esc_attr(get_typography_value($font_size_from_style)); ?>;<?php elseif ($font_size): ?>font-size: <?php echo esc_attr(get_typography_value($font_size)); ?>;<?php endif; ?>"> | ||
| <?php echo esc_html($breadcrumb['title']); ?> | ||
| </span> |
There was a problem hiding this comment.
Duplicate class attribute drops "ml-4" on current item
The span has two class attributes; the first (conditional "ml-4") is ignored. Merge classes to avoid losing spacing.
- <span <?php echo $index === 0 ? '' : 'class="ml-4"'; ?> aria-current="page" class="text-sm font-medium text-gray-500 hover:text-gray-700" style="<?php if ($font_size_from_style): ?>font-size: <?php echo esc_attr(get_typography_value($font_size_from_style)); ?>;<?php elseif ($font_size): ?>font-size: <?php echo esc_attr(get_typography_value($font_size)); ?>;<?php endif; ?>">
+ <span class="<?php echo $index === 0 ? '' : 'ml-4 '; ?>text-sm font-medium text-gray-500 hover:text-gray-700" aria-current="page" style="<?php if ($font_size_from_style): ?>font-size: <?php echo esc_attr(get_typography_value($font_size_from_style)); ?>;<?php elseif ($font_size): ?>font-size: <?php echo esc_attr(get_typography_value($font_size)); ?>;<?php endif; ?>">📝 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.
| <span <?php echo $index === 0 ? '' : 'class="ml-4"'; ?> aria-current="page" class="text-sm font-medium text-gray-500 hover:text-gray-700" style="<?php if ($font_size_from_style): ?>font-size: <?php echo esc_attr(get_typography_value($font_size_from_style)); ?>;<?php elseif ($font_size): ?>font-size: <?php echo esc_attr(get_typography_value($font_size)); ?>;<?php endif; ?>"> | |
| <?php echo esc_html($breadcrumb['title']); ?> | |
| </span> | |
| <span class="<?php echo $index === 0 ? '' : 'ml-4 '; ?>text-sm font-medium text-gray-500 hover:text-gray-700" aria-current="page" style="<?php if ($font_size_from_style): ?>font-size: <?php echo esc_attr(get_typography_value($font_size_from_style)); ?>;<?php elseif ($font_size): ?>font-size: <?php echo esc_attr(get_typography_value($font_size)); ?>;<?php endif; ?>"> | |
| <?php echo esc_html($breadcrumb['title']); ?> | |
| </span> |
🤖 Prompt for AI Agents
In src/blocks/Breadcrumb/render.php around lines 435 to 437, the <span> has two
class attributes so the conditional 'class="ml-4"' is ignored; merge the
conditional "ml-4" into the single class attribute instead. Modify the code so
the class attribute builds a single string that includes 'ml-4' when $index !==
0 along with the existing "text-sm font-medium text-gray-500
hover:text-gray-700" classes; keep the conditional inline style logic unchanged
and ensure values are properly escaped.
| .wp-block-wedocs-breadcrumbs { | ||
| /* Ensure TailwindCSS classes work in block editor */ | ||
| @apply block; | ||
| } |
There was a problem hiding this comment.
Frontend styles won’t apply without the block wrapper class.
All selectors are scoped to .wp-block-wedocs-breadcrumbs, but the server render wraps output in .wedocs-document (no block wrapper). Either add the wrapper via get_block_wrapper_attributes() in render.php and use it in edit.js, or mirror these selectors under .wedocs-document. Prefer fixing render/edit to use the standard wrapper.
🤖 Prompt for AI Agents
In src/blocks/Breadcrumb/style.css lines 6-9, the CSS targets
.wp-block-wedocs-breadcrumbs but the server-rendered HTML is wrapped in
.wedocs-document, so frontend styles never apply; update render.php to include
the standard block wrapper by calling get_block_wrapper_attributes() on the
block container output so the .wp-block-wedocs-breadcrumbs class is present, and
update edit.js to use the same wrapper (e.g., useBlockProps or adding the
wrapper attributes) so editor markup matches the frontend; alternatively, if you
cannot change render/edit now, duplicate the CSS selectors under
.wedocs-document to restore styling, but prefer implementing
get_block_wrapper_attributes() in render.php and making edit.js use the wrapper
class.
|
Some options are still missing as like in the design, @sapayth vai
|
|
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (3)
assets/build/frontend.js (3)
28-28: Same: source maps off in prod.Ensure frontend.js.map is only emitted for non-production builds.
1-16: Same: share the webpack runtime.Apply the runtimeChunk: 'single' optimization noted on print.js.
1-28: Same: remove build artifacts from VCS.Same rationale as print.js — please drop assets/build files from the PR.
🧹 Nitpick comments (12)
assets/build/print.js (2)
28-28: Gate source maps for production.The file references print.js.map. Ensure maps are excluded/minified in production builds to avoid size/leakage, and only enabled in dev.
1-16: Deduplicate webpack runtime across bundles.Both print.js and frontend.js embed identical bootstrap. Consider optimization.runtimeChunk: 'single' to shrink bundles.
Apply in webpack config:
module.exports = { // ... + optimization: { + runtimeChunk: 'single' + } }assets/build/index.css (1)
3816-3817: Note:shrink-0duplicates existingflex-shrink-0.Harmless, but consider removing one alias to keep the surface lean.
assets/build/blocks/Breadcrumb/block.json (4)
1-9: Addtextdomainfor i18n and verify the linked issue.
- Include
"textdomain": "wedocs"so strings are translatable.- The PR links issue “Bump async to 3.2.2,” but this file introduces a Breadcrumb block. Confirm the linkage or update references.
"icon": "arrow-right-alt", + "textdomain": "wedocs",
10-29: Model missing controls in attributes; add layout and separator spacing.Design feedback calls out alignment (present), layout selector, and separator margin/padding. Add structured attributes to support them and constrain
separatorvia enum."attributes": { "separator": { - "type": "string", - "default": "slash" + "type": "string", + "enum": ["slash","chevron","dot","pipe"], + "default": "slash" }, "hideHomeIcon": { "type": "boolean", "default": false }, "alignment": { "type": "string", "default": "left" }, + "layout": { + "type": "string", + "enum": ["inline","collapsed"], + "default": "inline" + }, + "separatorSpacing": { + "type": "object", + "default": { "marginInline": "0.5rem", "paddingInline": "0" } + }, "breadcrumbSeparator": { "type": "object", "default": { "color": "" } } },
40-82: Replace deprecated__experimental*supports with stable keys; add text alignment support.Use stable supports to integrate with Global Styles and avoid editor warnings. Also prefer built‑in text alignment support over a custom
alignmentattribute (you can still keep the attribute for backward compat)."supports": { "html": false, "color": { "background": true, "text": true, "link": true, "border": true }, "shadow": true, - "spacing": { - "margin": true, - "padding": true, - "__experimentalDefaultControls": { "margin": true, "padding": true } - }, - "__experimentalBorder": { - "color": true, "radius": true, "style": true, "width": true, - "__experimentalDefaultControls": { "color": true, "radius": true, "style": true, "width": true } - }, - "typography": { - "fontSize": true, - "lineHeight": true, - "__experimentalFontFamily": true, - "__experimentalFontWeight": true, - "__experimentalFontStyle": true, - "__experimentalTextTransform": true, - "__experimentalTextDecoration": true, - "__experimentalLetterSpacing": true, - "__experimentalDefaultControls": { "fontSize": true } - } + "spacing": { "margin": true, "padding": true }, + "border": { "color": true, "radius": true, "style": true, "width": true }, + "typography": { + "fontSize": true, + "lineHeight": true, + "fontFamily": true, + "fontWeight": true, + "fontStyle": true, + "textTransform": true, + "textDecoration": true, + "letterSpacing": true, + "textAlign": true + }, + "align": false },Note: if you migrate to
typography.textAlign, wire it in the editor/render and consider deprecating the customalignmentattribute.
83-84: A11y: ensure server render outputs proper breadcrumb semantics.Verify
render.phpuses<nav aria-label="Breadcrumb">with an ordered list<ol>andaria-current="page"on the last item; ensure separators are decorative (CSS/pseudo) and not announced.I can draft the render snippet if desired.
assets/build/store.js (5)
332-336: De‑duplicate userDocIds to prevent growth and redundant IDs.Merging/pushing can introduce duplicates.
Apply in source (src/data/docs/reducer.js):
- case 'SET_USER_DOC_IDS': - return { - ...state, - userDocIds: [...state.userDocIds, ...action.userDocIds] - }; + case 'SET_USER_DOC_IDS': { + const merged = [...state.userDocIds, ...action.userDocIds]; + return { ...state, userDocIds: Array.from(new Set(merged)) }; + } - case 'SET_USER_DOC_ID': - return { - ...state, - userDocIds: [...state.userDocIds, action.userDocId] - }; + case 'SET_USER_DOC_ID': + return state.userDocIds.includes(action.userDocId) + ? state + : { ...state, userDocIds: [...state.userDocIds, action.userDocId] };Also applies to: 338-341
444-450: Don’t mutate the fetched docs when derivinghelpfulDocs.
docs.sort(...)mutates the array you just used to set state (even if reducer copies, this is brittle). Sort a copy.Apply in source (src/data/docs/resolvers.js):
- const helpfulDocs = docs.sort((a, b) => helpfulDocIds.indexOf(a.id) - helpfulDocIds.indexOf(b.id)).filter(doc => helpfulDocIds?.includes(doc?.id)); + const helpfulDocs = [...docs] + .sort((a, b) => helpfulDocIds.indexOf(a.id) - helpfulDocIds.indexOf(b.id)) + .filter(doc => helpfulDocIds?.includes(doc?.id));
556-563: Remove redundant reverse() and avoid double work in sorting children.
reverse()is unnecessary before sorting; also avoid optional chaining on arrays from filter.Apply in source (src/data/docs/selectors.js):
- const childrens = docs?.filter(doc => doc.parent === id)?.reverse(); - const sortableChildrens = childrens?.sort((a, b) => a.menu_order - b.menu_order); - return sortableChildrens; + const children = docs.filter(doc => doc.parent === id); + return children.sort((a, b) => a.menu_order - b.menu_order);
477-482: Selectors return internal arrays by reference; consider defensive copies or memoization.Callers could mutate results; repeated sorts in selectors can be costly. Consider
reselect-style memoization or returning shallow copies in hot paths.Also applies to: 483-488, 495-500, 525-532, 533-547, 548-555, 564-569, 570-581
1-1228: Built artifacts committed; confirm repo policy.assets/build/ is usually ignored per your .gitignore update. If builds are tracked for distribution, fine; otherwise, drop build files from PR to reduce churn.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
assets/build/block.asset.php(1 hunks)assets/build/blocks/Breadcrumb/block.json(1 hunks)assets/build/blocks/Breadcrumb/render.php(1 hunks)assets/build/frontend.css(2 hunks)assets/build/frontend.js(1 hunks)assets/build/index.asset.php(1 hunks)assets/build/index.css(10 hunks)assets/build/print.css(2 hunks)assets/build/print.js(1 hunks)assets/build/store.js(1 hunks)src/blocks/Breadcrumb/Inspector.js(1 hunks)src/blocks/Breadcrumb/block.json(1 hunks)src/blocks/Breadcrumb/edit.js(1 hunks)src/blocks/Breadcrumb/render.php(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- assets/build/print.css
- assets/build/frontend.css
🚧 Files skipped from review as they are similar to previous changes (7)
- src/blocks/Breadcrumb/edit.js
- src/blocks/Breadcrumb/Inspector.js
- assets/build/index.asset.php
- src/blocks/Breadcrumb/block.json
- assets/build/block.asset.php
- src/blocks/Breadcrumb/render.php
- assets/build/blocks/Breadcrumb/render.php
🧰 Additional context used
🪛 Biome (2.1.2)
assets/build/store.js
[error] 321-324: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 325-325: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 375-375: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
[error] 376-376: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
assets/build/index.css
[error] 5888-5888: Unexpected value or character.
Expected one of:
(parse)
[error] 5888-5888: Unexpected value or character.
Expected one of:
(parse)
[error] 5888-5888: Unexpected value or character.
Expected one of:
(parse)
[error] 5888-5888: Unexpected value or character.
Expected one of:
(parse)
[error] 5888-5888: Unexpected value or character.
Expected one of:
(parse)
[error] 5888-5888: Unexpected shorthand property flex-flow after flex-direction
(lint/suspicious/noShorthandPropertyOverrides)
[error] 5888-5888: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
display is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 5888-5888: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
display is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 5888-5888: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
transition is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
🔇 Additional comments (15)
assets/build/print.js (2)
10-15: Runtime marker looks fine.webpack_require.r guards Symbol and sets __esModule correctly. No action needed.
1-28: Upgrade confirmed — async upgraded to 3.2.4; no older versions found. package.json has no direct async; package-lock.json contains "async": "^3.2.4" (resolved to async-3.2.4.tgz); no other lockfiles with older async were found; .gitignore includes /assets/build/.assets/build/index.css (8)
3072-3082: A11y helpersr-onlyis correct and safely scoped.Utility looks good and is correctly namespaced under
.wedocs-document.
3228-3230: Utility spacingmx-2addition looks good.Safe, localized extension.
3915-3916:justify-startaddition LGTM.
4290-4292:bg-blue-500utility is fine and consistent with--tw-bg-opacity.
4733-4734:uppercaseutility addition LGTM.
5478-5480: Hover color utility is fine.
5506-5507: Hover opacity utility is fine.
5888-5888: Silence Biome on vendor CSS or it will keep failing.Errors reported (unexpected chars, duplicate properties, shorthand after longhand) come from minified vendor rules (
-ms-box-orient, duplicateddisplay,transition, etc.). Treat as vendor exemptions rather than fixing compiled output.Add the ignore and re-run CI to confirm the violations disappear for built assets.
assets/build/store.js (5)
653-662: Assume API response shape cautiously inupdateSettings.Code expects
{ data }and thenresponse.data. If the controller returns the settings directly, this will fail.Please confirm the server response contract for
/wp/v2/docs/settings. If it returns settings directly, assignresponseinstead ofresponse.data.
958-971: Turnstile site key nested update looks correct.Ensure any settings persistence path also saves this nested value.
726-734: Early return on promo notice when Pro is loaded is sensible.
1222-1224: Store registration LGTM.Registering both stores via
wp.data.registeris correct.
175-182: Clarify: hard delete vs Trash — add ?force=true for permanent deletion
WordPress REST requires ?force=true to permanently delete; DELETE without it moves trashable items to Trash (HTTP 410 if already trashed; 501 for non‑trashable types).
If permanent deletion is intended, change the path to '/wp/v2/docs/{docId}?force=true' and only call actions.removeDoc(docId) after confirming the permanent delete; otherwise keep current behavior to move to Trash.
File: assets/build/store.js (lines 175-182)
| !*** css ./node_modules/css-loader/dist/cjs.js??ruleSet[1].rules[2].use[1]!./node_modules/postcss-loader/dist/cjs.js??ruleSet[1].rules[2].use[2]!./node_modules/react-responsive-carousel/lib/styles/carousel.min.css ***! | ||
| \************************************************************************************************************************************************************************************************************************/ | ||
| .carousel .control-arrow,.carousel.carousel-slider .control-arrow{transition:all .25s ease-in;opacity:.4;filter:alpha(opacity=40);position:absolute;z-index:2;top:20px;background:none;border:0;font-size:32px;cursor:pointer}.carousel .control-arrow:focus,.carousel .control-arrow:hover{opacity:1;filter:alpha(opacity=100)}.carousel .control-arrow:before,.carousel.carousel-slider .control-arrow:before{margin:0 5px;display:inline-block;border-top:8px solid transparent;border-bottom:8px solid transparent;content:''}.carousel .control-disabled.control-arrow{opacity:0;filter:alpha(opacity=0);cursor:inherit;display:none}.carousel .control-prev.control-arrow{left:0}.carousel .control-prev.control-arrow:before{border-right:8px solid #fff}.carousel .control-next.control-arrow{right:0}.carousel .control-next.control-arrow:before{border-left:8px solid #fff}.carousel-root{outline:none}.carousel{position:relative;width:100%}.carousel *{box-sizing:border-box}.carousel img{width:100%;display:inline-block;pointer-events:none}.carousel .carousel{position:relative}.carousel .control-arrow{outline:0;border:0;background:none;top:50%;margin-top:-13px;font-size:18px}.carousel .thumbs-wrapper{margin:20px;overflow:hidden}.carousel .thumbs{transition:all .15s ease-in;transform:translate3d(0, 0, 0);position:relative;list-style:none;white-space:nowrap}.carousel .thumb{transition:border .15s ease-in;display:inline-block;margin-right:6px;white-space:nowrap;overflow:hidden;border:3px solid #fff;padding:2px}.carousel .thumb:focus{border:3px solid #ccc;outline:none}.carousel .thumb.selected,.carousel .thumb:hover{border:3px solid #333}.carousel .thumb img{vertical-align:top}.carousel.carousel-slider{position:relative;margin:0;overflow:hidden}.carousel.carousel-slider .control-arrow{top:0;color:#fff;font-size:26px;bottom:0;margin-top:0;padding:5px}.carousel.carousel-slider .control-arrow:hover{background:rgba(0,0,0,0.2)}.carousel .slider-wrapper{overflow:hidden;margin:auto;width:100%;transition:height .15s ease-in}.carousel .slider-wrapper.axis-horizontal .slider{-ms-box-orient:horizontal;display:-moz-flex;display:flex}.carousel .slider-wrapper.axis-horizontal .slider .slide{flex-direction:column;flex-flow:column}.carousel .slider-wrapper.axis-vertical{-ms-box-orient:horizontal;display:-moz-flex;display:flex}.carousel .slider-wrapper.axis-vertical .slider{flex-direction:column}.carousel .slider{margin:0;padding:0;position:relative;list-style:none;width:100%}.carousel .slider.animated{transition:all .35s ease-in-out}.carousel .slide{min-width:100%;margin:0;position:relative;text-align:center}.carousel .slide img{width:100%;vertical-align:top;border:0}.carousel .slide iframe{display:inline-block;width:calc(100% - 80px);margin:0 40px 40px;border:0}.carousel .slide .legend{transition:all .5s ease-in-out;position:absolute;bottom:40px;left:50%;margin-left:-45%;width:90%;border-radius:10px;background:#000;color:#fff;padding:10px;font-size:12px;text-align:center;opacity:0.25;transition:opacity .35s ease-in-out}.carousel .control-dots{position:absolute;bottom:0;margin:10px 0;padding:0;text-align:center;width:100%;z-index:1}@media (min-width: 960px){.carousel .control-dots{bottom:0}}.carousel .control-dots .dot{transition:opacity .25s ease-in;opacity:.3;filter:alpha(opacity=30);box-shadow:1px 1px 2px rgba(0,0,0,0.9);background:#fff;border-radius:50%;width:8px;height:8px;cursor:pointer;display:inline-block;margin:0 8px}.carousel .control-dots .dot.selected,.carousel .control-dots .dot:hover{opacity:1;filter:alpha(opacity=100)}.carousel .carousel-status{position:absolute;top:0;right:0;padding:5px;font-size:10px;text-shadow:1px 1px 1px rgba(0,0,0,0.9);color:#fff}.carousel:hover .slide .legend{opacity:1} |
There was a problem hiding this comment.
Don’t inline third‑party carousel CSS into the global index bundle.
Inlining react-responsive-carousel CSS here bloats every page and triggers Biome errors on vendor syntax. Extract it to a separate CSS chunk and enqueue only where the carousel is used (likely admin/settings), or import it in that feature’s entry and mark assets/build/** as lint‑ignored.
Proposed actions:
- Stop importing
react-responsive-carousel/lib/styles/carousel.min.cssin the global CSS entry; import it in the specific admin/editor bundle instead. - Update linter config to ignore built/vendor CSS:
+// .biome.json
+{
+ "files": {
+ "ignore": ["assets/build/**", "**/node_modules/**"]
+ }
+}- If you must keep it, wrap the vendor import with stylelint/biome disable comments in the source, not the built file.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (2.1.2)
[error] 5888-5888: Unexpected value or character.
Expected one of:
(parse)
[error] 5888-5888: Unexpected value or character.
Expected one of:
(parse)
[error] 5888-5888: Unexpected value or character.
Expected one of:
(parse)
[error] 5888-5888: Unexpected value or character.
Expected one of:
(parse)
[error] 5888-5888: Unexpected value or character.
Expected one of:
(parse)
[error] 5888-5888: Unexpected shorthand property flex-flow after flex-direction
(lint/suspicious/noShorthandPropertyOverrides)
[error] 5888-5888: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
display is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 5888-5888: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
display is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 5888-5888: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
transition is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
🤖 Prompt for AI Agents
In assets/build/index.css lines 5886-5888 the minified third-party carousel CSS
from react-responsive-carousel is inlined into the global bundle; remove that
import from the global CSS/entry and instead import the carousel stylesheet from
the specific feature entry (e.g., admin/settings or editor) so it becomes a
separate chunk or lazy-loaded only when the carousel is used, then rebuild the
bundle; finally update project lint settings to ignore built/vendor CSS (or add
stylelint/biome disable comments in the original source file where the vendor
import lives, not in the generated assets) so vendor syntax doesn’t trigger lint
errors.
| /******/ (() => { // webpackBootstrap | ||
| /******/ "use strict"; | ||
| /******/ // The require scope | ||
| /******/ var __webpack_require__ = {}; | ||
| /******/ | ||
| /************************************************************************/ | ||
| /******/ /* webpack/runtime/make namespace object */ | ||
| /******/ (() => { | ||
| /******/ // define __esModule on exports | ||
| /******/ __webpack_require__.r = (exports) => { | ||
| /******/ if(typeof Symbol !== 'undefined' && Symbol.toStringTag) { | ||
| /******/ Object.defineProperty(exports, Symbol.toStringTag, { value: 'Module' }); | ||
| /******/ } | ||
| /******/ Object.defineProperty(exports, '__esModule', { value: true }); | ||
| /******/ }; | ||
| /******/ })(); | ||
| /******/ | ||
| /************************************************************************/ | ||
| var __webpack_exports__ = {}; | ||
| /*!************************************!*\ | ||
| !*** ./src/assets/less/print.less ***! | ||
| \************************************/ | ||
| __webpack_require__.r(__webpack_exports__); | ||
| // extracted by mini-css-extract-plugin | ||
|
|
||
| /******/ })() | ||
| ; | ||
| //# sourceMappingURL=print.js.map |
There was a problem hiding this comment.
Drop build artifacts from the PR; align with .gitignore.
These are compiled bundles. Given .gitignore now excludes /assets/build/, these should not be tracked. Please remove assets/build from this PR and rely on CI/release packaging (or keep bundles only on release/dist branches if WP.org distribution needs them).
🤖 Prompt for AI Agents
In assets/build/print.js around lines 1 to 28, this is a compiled bundle that
should not be tracked; remove the file from the PR and ensure assets/build is
ignored. To fix: delete the file from the branch (git rm assets/build/print.js
or remove the entire assets/build directory), commit the deletion, and if the
directory was previously tracked run git rm -r --cached assets/build then
commit; confirm .gitignore contains /assets/build/ and push the branch so
CI/release will produce bundles instead of committing them to the repo.
| const setDocState = { | ||
| ...state, | ||
| docs: [...state.docs, action.doc] | ||
| }; | ||
| const isNotInParent = !state.parents.some(parent => parent?.id === action?.doc?.id); | ||
| if (!action.doc.parent && isNotInParent) { | ||
| setDocState.parents = [{ | ||
| ...action.doc | ||
| }, ...state.parents]; | ||
| } | ||
| return setDocState; | ||
| case 'SET_USER_DOC_IDS': | ||
| return { | ||
| ...state, | ||
| userDocIds: [...state.userDocIds, ...action.userDocIds] | ||
| }; | ||
| case 'SET_USER_DOC_ID': | ||
| return { | ||
| ...state, | ||
| userDocIds: [...state.userDocIds, action.userDocId] | ||
| }; |
There was a problem hiding this comment.
Fix Biome “noSwitchDeclarations”: wrap case body in a block.
Variables declared in a switch case leak across clauses. Wrap this case in braces.
Apply in source (src/data/docs/reducer.js) then rebuild:
- case 'SET_DOC':
- const setDocState = {
+ case 'SET_DOC': {
+ const setDocState = {
...state,
docs: [...state.docs, action.doc]
- };
- const isNotInParent = !state.parents.some(parent => parent?.id === action?.doc?.id);
- if (!action.doc.parent && isNotInParent) {
- setDocState.parents = [{
- ...action.doc
- }, ...state.parents];
- }
- return setDocState;
+ };
+ const isNotInParent = !state.parents.some(parent => parent?.id === action?.doc?.id);
+ if (!action.doc.parent && isNotInParent) {
+ setDocState.parents = [{ ...action.doc }, ...state.parents];
+ }
+ return setDocState;
+ }📝 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.
| const setDocState = { | |
| ...state, | |
| docs: [...state.docs, action.doc] | |
| }; | |
| const isNotInParent = !state.parents.some(parent => parent?.id === action?.doc?.id); | |
| if (!action.doc.parent && isNotInParent) { | |
| setDocState.parents = [{ | |
| ...action.doc | |
| }, ...state.parents]; | |
| } | |
| return setDocState; | |
| case 'SET_USER_DOC_IDS': | |
| return { | |
| ...state, | |
| userDocIds: [...state.userDocIds, ...action.userDocIds] | |
| }; | |
| case 'SET_USER_DOC_ID': | |
| return { | |
| ...state, | |
| userDocIds: [...state.userDocIds, action.userDocId] | |
| }; | |
| case 'SET_DOC': { | |
| const setDocState = { | |
| ...state, | |
| docs: [...state.docs, action.doc] | |
| }; | |
| const isNotInParent = !state.parents.some(parent => parent?.id === action?.doc?.id); | |
| if (!action.doc.parent && isNotInParent) { | |
| setDocState.parents = [{ ...action.doc }, ...state.parents]; | |
| } | |
| return setDocState; | |
| } | |
| case 'SET_USER_DOC_IDS': | |
| return { | |
| ...state, | |
| userDocIds: [...state.userDocIds, ...action.userDocIds] | |
| }; | |
| case 'SET_USER_DOC_ID': | |
| return { | |
| ...state, | |
| userDocIds: [...state.userDocIds, action.userDocId] | |
| }; |
🧰 Tools
🪛 Biome (2.1.2)
[error] 321-324: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 325-325: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🤖 Prompt for AI Agents
In src/data/docs/reducer.js around the lines that correspond to
assets/build/store.js:321-341, the switch case for adding a doc declares local
variables (e.g. setDocState, isNotInParent) without a block, causing them to
leak into subsequent cases; wrap the entire case body in braces (e.g. change
`case 'ADD_DOC':` to `case 'ADD_DOC': {` and add a matching `}` before the next
case) so all variable declarations are block-scoped, keep the existing return
inside the new block, and then rebuild.
| case 'REMOVE_DOC': | ||
| return { | ||
| ...state, | ||
| docs: [...state.docs?.filter(doc => doc.id !== action.docId)], | ||
| parents: [...state.parents?.filter(parent => parent.id !== action.docId)] | ||
| }; |
There was a problem hiding this comment.
Unsafe optional chaining inside array spreads; also wrap case body.
Spreading the result of state.docs?.filter(...)/state.parents?.filter(...) can throw if the LHS is undefined. Use fallbacks and a block.
Apply in source (src/data/docs/reducer.js):
- case 'REMOVE_DOC':
- return {
- ...state,
- docs: [...state.docs?.filter(doc => doc.id !== action.docId)],
- parents: [...state.parents?.filter(parent => parent.id !== action.docId)]
- };
+ case 'REMOVE_DOC': {
+ const docs = Array.isArray(state.docs) ? state.docs : [];
+ const parents = Array.isArray(state.parents) ? state.parents : [];
+ return {
+ ...state,
+ docs: docs.filter(doc => doc.id !== action.docId),
+ parents: parents.filter(parent => parent.id !== action.docId),
+ };
+ }📝 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.
| case 'REMOVE_DOC': | |
| return { | |
| ...state, | |
| docs: [...state.docs?.filter(doc => doc.id !== action.docId)], | |
| parents: [...state.parents?.filter(parent => parent.id !== action.docId)] | |
| }; | |
| case 'REMOVE_DOC': { | |
| const docs = Array.isArray(state.docs) ? state.docs : []; | |
| const parents = Array.isArray(state.parents) ? state.parents : []; | |
| return { | |
| ...state, | |
| docs: docs.filter(doc => doc.id !== action.docId), | |
| parents: parents.filter(parent => parent.id !== action.docId), | |
| }; | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 375-375: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
[error] 376-376: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
🤖 Prompt for AI Agents
In assets/build/store.js around lines 372 to 377 (apply change in
src/data/docs/reducer.js), the case body uses optional chaining inside array
spreads which can throw if state.docs or state.parents is undefined; wrap the
case branch in a block and replace the spread of state.docs?.filter(...) and
state.parents?.filter(...) with safe fallbacks like (state.docs ||
[]).filter(...) and (state.parents || []).filter(...), then spread those results
into new arrays so the code never attempts to spread undefined.
| const getDocsPath = wp.hooks.applyFilters('wedocs_documentation_fetching_path', `/wp/v2/docs?per_page=-1&status=publish${typeof weDocsAdminVars !== 'undefined' ? ',draft' : ''}`); | ||
| const resolvers = { |
There was a problem hiding this comment.
🧩 Analysis chain
Verify usage of per_page=-1 in REST calls; WordPress caps per_page (commonly 100).
Using per_page=-1 can return a 400 error unless you’ve customized REST params. If not guaranteed, paginate or use a custom endpoint.
Would you confirm with docs that per_page=-1 is valid in your environment?
If pagination is required, consider switching controls to return full Response (apiFetch { parse: false }) and loop pages until empty, or expose a server endpoint that returns all docs safely.
Also applies to: 115-116, 184-186
🌐 Web query:
Does the WordPress REST API accept per_page=-1? What is the documented limit and recommended way to fetch all items?
💡 Result:
Short answer: No — per_page=-1 is not supported by the official REST API. The documented per_page range is 1–100 (default 10) and per_page is capped at 100. To retrieve everything, request pages (per_page up to 100) and combine results; use the X-WP-Total and X-WP-TotalPages response headers to know how many requests to make. [1][2][3]
Sources:
- REST API pagination docs (per_page 1–100, use multiple requests + X-WP-Total/X-WP-TotalPages). [1]
- Trac ticket discussing unbounded per_page=-1 (proposal was closed / not adopted). [2]
- Community examples / threads showing attempts/workarounds (customizing maximum or paging). [3]
Don't use per_page=-1 — paginate or add a server endpoint
per_page is capped at 1–100 in the WP REST API; per_page=-1 is unsupported and can cause 400 errors.
Action: Update calls in assets/build/store.js (lines 115–116, 184–186, 410–411) to fetch pages (use per_page=100 and loop page=1..N using X-WP-Total / X-WP-TotalPages — or call apiFetch({ parse: false }) to read headers), or expose a server endpoint that returns all docs.
🤖 Prompt for AI Agents
In assets/build/store.js around lines 115–116, 184–186 and 410–411, the code
uses per_page=-1 which WP REST API does not support and can cause 400 errors;
change the fetch logic to paginate: request per_page=100 and loop page=1..N
using the page query param, reading X-WP-TotalPages (or X-WP-Total) from
response headers to know when to stop — use apiFetch({ path, parse: false }) so
you can access response.headers, parse each page body and concat results;
alternatively, add a server-side endpoint that returns all docs and call that
endpoint instead of per_page=-1. Ensure the new implementation aggregates all
pages before returning the full docs list.
|
Hovering on list not refecting setted up color. @sapayth vai Screen.Recording.2025-09-19.at.2.57.29.PM.mov |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
assets/build/index.css (3)
1-3: Stop committing built assets; ignoreassets/build/.This is a compiled bundle. Keep sources in the PR and let CI build. Add/verify
.gitignoreand drop tracked build files.Apply:
+.gitignore +assets/build/Then untrack existing files:
git rm -r --cached assets/build
3815-3817: Aliasshrink-0duplicatesflex-shrink-0.Avoid parallel aliases unless required; they bloat CSS and can confuse authors.
Consider relying on
.flex-shrink-0only, or documenting why both are needed.
4877-4879: Addhover:no-underlineto override DaisyUI breadcrumb underline on hover.
.breadcrumbs > li > a:hoverforces underline; yourno-underlinewon’t prevent it. Provide a hover variant.Apply near the other hover utilities:
+.wedocs-document :is(.hover\:no-underline:hover) { + text-decoration-line: none; +}assets/build/blocks/Breadcrumb/render.php (1)
422-427: Verify separator color attribute usage matches frontend implementation.The separator uses
$separator_colordirectly but should use the helper function for consistency with other color processing.Apply this diff for consistency:
-<span class="mx-2" style="<?php if ($separator_color): ?>color: <?php echo esc_attr($separator_color); ?>;<?php endif; ?>"> +<span class="mx-2" style="<?php if ($separator_color): ?>color: <?php echo esc_attr(wedocs_get_color_value($separator_color)); ?>;<?php endif; ?>">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
assets/build/blocks/Breadcrumb/render.php(1 hunks)assets/build/index.asset.php(1 hunks)assets/build/index.css(11 hunks)src/blocks/Breadcrumb/render.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/blocks/Breadcrumb/render.php
- assets/build/index.asset.php
🧰 Additional context used
🧬 Code graph analysis (1)
assets/build/blocks/Breadcrumb/render.php (2)
src/blocks/Breadcrumb/Inspector.js (1)
attributes(21-26)src/blocks/Breadcrumb/edit.js (7)
attributes(42-42)spacing(149-149)border(150-150)typography(151-151)padding(153-153)margin(154-154)shadow(152-152)
🪛 Biome (2.1.2)
assets/build/index.css
[error] 5891-5891: Unexpected value or character.
Expected one of:
(parse)
[error] 5891-5891: Unexpected value or character.
Expected one of:
(parse)
[error] 5891-5891: Unexpected value or character.
Expected one of:
(parse)
[error] 5891-5891: Unexpected value or character.
Expected one of:
(parse)
[error] 5891-5891: Unexpected value or character.
Expected one of:
(parse)
[error] 5891-5891: Unexpected shorthand property flex-flow after flex-direction
(lint/suspicious/noShorthandPropertyOverrides)
[error] 5891-5891: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
display is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 5891-5891: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
display is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 5891-5891: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
transition is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
🔇 Additional comments (16)
assets/build/index.css (8)
3072-3082:sr-onlyutility looks good.
3227-3230: Addsmx-2under.wedocs-document— OK.
3914-3916:justify-startadded — aligns with the “Alignment option” fix.
4289-4292: Addsbg-blue-500— fine within the scoped utilities.
4732-4734:uppercaseutility — OK.
5480-5483: Hover color utility added — verify it fixes the reporter’s hover-color issue.Ensure the class is applied on the
<a>within breadcrumb items (not only on<li>), and that no higher-specificity rule overrides it.
5508-5510:hover:opacity-80— OK.
5888-5894: Don’t inlinereact-responsive-carouselCSS into the global bundle.This drags admin-only/vendor styles into every page and triggers Biome parse/duplicate-property errors.
Proposed actions:
- Move the import of
react-responsive-carousel/lib/styles/carousel.min.cssto the specific admin/editor entry where it’s used so it becomes a separate chunk.- Exclude built/vendor CSS from linters.
Add linter ignore:
+.biome.json +{ + "files": { "ignore": ["assets/build/**", "**/node_modules/**"] } +}Rebuild after relocating the import.
assets/build/blocks/Breadcrumb/render.php (8)
55-88: Use the prefixed function name to avoid global collisions.This addresses the existing feedback about global function name conflicts. The current generic name
get_spacing_valuecould conflict with other plugins.Apply this diff to add the prefix:
-if (!function_exists('get_spacing_value')) { - function get_spacing_value($spacing_value) { +if (!function_exists('wedocs_get_spacing_value')) { + function wedocs_get_spacing_value($spacing_value) {
91-110: Use the prefixed function name to avoid global collisions.This addresses the existing feedback about global function name conflicts. The current generic name
get_color_valuecould conflict with other plugins.Apply this diff to add the prefix:
-if (!function_exists('get_color_value')) { - function get_color_value($color) { +if (!function_exists('wedocs_get_color_value')) { + function wedocs_get_color_value($color) {
113-124: Use prefixed function names for all helper functions.All helper functions should be prefixed to avoid potential conflicts as mentioned in previous reviews.
Apply this diff to add prefixes:
-if (!function_exists('get_border_radius_value')) { - function get_border_radius_value($radius_value) { +if (!function_exists('wedocs_get_border_radius_value')) { + function wedocs_get_border_radius_value($radius_value) { -if (!function_exists('get_border_width_value')) { - function get_border_width_value($width_value) { +if (!function_exists('wedocs_get_border_width_value')) { + function wedocs_get_border_width_value($width_value) { -if (!function_exists('get_typography_value')) { - function get_typography_value($typography_value) { +if (!function_exists('wedocs_get_typography_value')) { + function wedocs_get_typography_value($typography_value) { -if (!function_exists('get_shadow_value')) { - function get_shadow_value($shadow_value) { +if (!function_exists('wedocs_get_shadow_value')) { + function wedocs_get_shadow_value($shadow_value) {Also applies to: 127-134, 137-162, 165-184
187-202: Use prefixed function names for remaining helper functions.Complete the prefixing for all remaining helper functions to maintain consistency and avoid conflicts.
Apply this diff to add prefixes:
-if (!function_exists('get_separator_icon')) { - function get_separator_icon($separator) { +if (!function_exists('wedocs_get_separator_icon')) { + function wedocs_get_separator_icon($separator) { -if (!function_exists('get_alignment_class')) { - function get_alignment_class($alignment) { +if (!function_exists('wedocs_get_alignment_class')) { + function wedocs_get_alignment_class($alignment) { -if (!function_exists('get_breadcrumb_items')) { - function get_breadcrumb_items() { +if (!function_exists('wedocs_get_breadcrumb_items')) { + function wedocs_get_breadcrumb_items() { -if (!function_exists('get_doc_ancestors')) { - function get_doc_ancestors($post) { +if (!function_exists('wedocs_get_doc_ancestors')) { + function wedocs_get_doc_ancestors($post) {Also applies to: 205-217, 220-339, 341-358
374-374: Add WordPress block wrapper attributes.This addresses the existing feedback about missing block wrapper attributes that prevents proper CSS application and WordPress block features.
Apply this diff to add proper wrapper attributes:
-<div class="wedocs-document"> +<?php +$wrapper_attributes = function_exists('get_block_wrapper_attributes') + ? get_block_wrapper_attributes(['class' => 'wedocs-document']) + : 'class="wedocs-document"'; +?> +<div <?php echo $wrapper_attributes; ?>>
459-461: Fix duplicate class attributes on current page span.This addresses the existing feedback about duplicate class attributes that cause the first one to be ignored.
Apply this diff to consolidate the class attributes:
-<span <?php echo $index === 0 ? '' : 'class=""'; ?> aria-current="page" class="" style="<?php if ($font_size_from_style): ?>font-size: <?php echo esc_attr(get_typography_value($font_size_from_style)); ?>;<?php elseif ($font_size): ?>font-size: <?php echo esc_attr(get_typography_value($font_size)); ?>;<?php endif; ?>"> +<span aria-current="page" style="<?php if ($font_size_from_style): ?>font-size: <?php echo esc_attr(get_typography_value($font_size_from_style)); ?>;<?php elseif ($font_size): ?>font-size: <?php echo esc_attr(get_typography_value($font_size)); ?>;<?php endif; ?>">
235-235: Update function call in breadcrumb generation.The
get_doc_ancestorsfunction call also needs to be updated to use the prefixed version.Apply this diff:
-$parent_docs = get_doc_ancestors($post); +$parent_docs = wedocs_get_doc_ancestors($post);
364-373: Verified — CSS custom properties are defined on breadcrumb links.Inline --hover-color and --hover-background are set on the anchor elements' style attributes in src/blocks/Breadcrumb/render.php (lines 439 and 453), so the hover rules using var(--hover-color / --hover-background) will resolve.
| $separator_icon = get_separator_icon($separator); | ||
| $breadcrumbs = get_breadcrumb_items(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update function calls to use prefixed names.
After prefixing the function names, update all function calls throughout the file to use the new prefixed versions.
Apply these diffs to update all function calls:
-$separator_icon = get_separator_icon($separator);
-$breadcrumbs = get_breadcrumb_items();
+$separator_icon = wedocs_get_separator_icon($separator);
+$breadcrumbs = wedocs_get_breadcrumb_items();
-$ol_classes = ['flex', get_alignment_class($alignment)];
+$ol_classes = ['flex', wedocs_get_alignment_class($alignment)];
-<?php if ($padding): ?>padding: <?php echo esc_attr(get_spacing_value($padding)); ?>;<?php endif; ?>
-<?php if ($margin): ?>margin: <?php echo esc_attr(get_spacing_value($margin)); ?>;<?php endif; ?>
-<?php if ($border_radius): ?>border-radius: <?php echo esc_attr(get_border_radius_value($border_radius)); ?>;<?php endif; ?>
-<?php if ($border_width): ?>border-width: <?php echo esc_attr(get_border_width_value($border_width)); ?>;<?php endif; ?>
-<?php if ($effective_border_color): ?>border-color: <?php echo esc_attr(get_color_value($effective_border_color)); ?>;<?php endif; ?>
-<?php if ($shadow): ?>box-shadow: <?php echo esc_attr(get_shadow_value($shadow)); ?>;<?php endif; ?>
-<?php if ($font_family): ?>font-family: <?php echo esc_attr(get_typography_value($font_family)); ?>;<?php endif; ?>
+<?php if ($padding): ?>padding: <?php echo esc_attr(wedocs_get_spacing_value($padding)); ?>;<?php endif; ?>
+<?php if ($margin): ?>margin: <?php echo esc_attr(wedocs_get_spacing_value($margin)); ?>;<?php endif; ?>
+<?php if ($border_radius): ?>border-radius: <?php echo esc_attr(wedocs_get_border_radius_value($border_radius)); ?>;<?php endif; ?>
+<?php if ($border_width): ?>border-width: <?php echo esc_attr(wedocs_get_border_width_value($border_width)); ?>;<?php endif; ?>
+<?php if ($effective_border_color): ?>border-color: <?php echo esc_attr(wedocs_get_color_value($effective_border_color)); ?>;<?php endif; ?>
+<?php if ($shadow): ?>box-shadow: <?php echo esc_attr(wedocs_get_shadow_value($shadow)); ?>;<?php endif; ?>
+<?php if ($font_family): ?>font-family: <?php echo esc_attr(wedocs_get_typography_value($font_family)); ?>;<?php endif; ?>Continue updating all remaining function calls with the prefixed versions throughout lines 439, 453, and 459.
Also applies to: 378-378, 404-417, 424-424, 439-439, 453-453, 459-459
🤖 Prompt for AI Agents
In assets/build/blocks/Breadcrumb/render.php around lines 360-361 (and also
update usages at 378, 404-417, 424, 439, 453, 459), replace all calls to the old
unprefixed functions with their new prefixed equivalents — e.g., update
get_separator_icon(...) and get_breadcrumb_items(...) to the corresponding
prefixed function names, and do the same for every other function call in the
listed line ranges so all calls match the recently renamed/prefixed definitions.
fixes #154
a weDocs Breadcrumb block with both functional and styling controls
Summary by CodeRabbit
New Features
Style
Refactor
Chores