Skip to content

Conversation

@sapayth
Copy link
Member

@sapayth sapayth commented Sep 19, 2025

fixes #167

Show Previous Doc and Next Doc links at the bottom (or top) of a doc, helping users navigate between articles within the same section.

Summary by CodeRabbit

  • New Features

    • Added Doc Navigation block: server-rendered previous/next links with rich editor controls for SEO rel, typography, colors, borders, spacing and arrow styling; responsive preview.
  • Style

    • Bundled carousel styles into main CSS and added fallback styles for Doc Navigation when Tailwind is unavailable.
  • Chores

    • Bumped build asset versions and updated bundles for cache-busting.
  • Refactor

    • Modularized data layer/storage for improved editor/data stability.

@coderabbitai
Copy link

coderabbitai bot commented Sep 19, 2025

Walkthrough

Adds a server-rendered DocNavigation Gutenberg block (editor UI, styles, PHP renderer), registers it, modularizes the JavaScript data stores, updates build artifacts (webpack scaffolding, inlined CSS), adds wp-data to block dependencies, and bumps several asset version hashes.

Changes

Cohort / File(s) Summary of Changes
DocNavigation block (source & build)
src/blocks/DocNavigation/block.json, src/blocks/DocNavigation/edit.js, src/blocks/DocNavigation/index.js, src/blocks/DocNavigation/save.js, src/blocks/DocNavigation/style.scss, src/blocks/DocNavigation/render.php, assets/build/blocks/DocNavigation/block.json, assets/build/blocks/DocNavigation/render.php, assets/build/blocks/DocNavigation/*
New dynamic block wedocs/wedocs-doc-navigation: manifest, editor component with InspectorControls, fallback SCSS, server-side render PHP, save() returns null; block registered and renderer loaded in plugin.
Plugin integration
wedocs.php
Loads and registers the new DocNavigation block and its server render callback.
Doc navigation helpers
includes/functions.php
Adds wedocs_get_doc_navigation_posts( $post ) and refactors wedocs_doc_nav() to use it for next/prev doc resolution.
Data store modularization (source & build)
src/data/*, src/data/docs/*, src/data/settings/*, src/data/store.js, assets/build/store.js, assets/build/store.asset.php
Replaces monolithic store bundle with modular docs and settings stores, exports named constants and default stores, and registers stores via a new entry; asset version bump.
Block asset dependency change
assets/build/block.asset.php
Adds wp-data to block dependencies and updates version hash.
Webpack/runtime & CSS bundling changes (build)
assets/build/frontend.js, assets/build/print.js, assets/build/frontend.css, assets/build/print.css, assets/build/index.css
Adds webpack runtime scaffolding for CSS entries, loader header comments, inlines carousel CSS into index.css, and appends sourceMap references; no runtime behavior changes beyond bundler wiring.
Asset manifest version bumps
assets/build/frontend.asset.php, assets/build/index.asset.php, assets/build/print.asset.php, assets/build/block.asset.php, assets/build/store.asset.php
Updated version hashes for multiple build manifests; dependencies mostly unchanged except block.asset.php.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

QA Approved, Ready To Merge

Suggested reviewers

  • iftakharul-islam

Poem

"I nibble code and hop with delight,
New arrows point left and right.
Blocks blossom where pages meet,
Stores split neat like fresh spring wheat.
A whiskered cheer — docs now glide light!" 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning The PR includes several changes that appear outside the stated objective of adding the DocNavigation block: most notably a large modularization of the built data store (assets/build/store.js with many new public exports) and numerous broad build-artifact changes (inlined carousel CSS, webpack runtime wrappers, and multiple asset version bumps). These architectural and generated-asset changes are not described in the linked issue and increase review surface and risk of unrelated regressions, so they appear out-of-scope for this feature PR. A focused feature PR should avoid bundling such large refactors or unrelated build noise without explicit justification. Either split the store refactor and other large changes into a separate PR or add a clear justification and tests in this PR explaining why they are required for the DocNavigation block; if the build outputs are incidental, restrict the commit to source changes and include only the minimal regenerated assets necessary for the new block. Also run regression tests and document any public API changes introduced by the new store layout.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "feat: block doc navigation" is concise and accurately reflects the primary change — adding a document navigation Gutenberg block. It uses a conventional "feat" prefix and contains no extraneous detail, making it easy to scan in history. The title is specific enough for teammates to understand the main intention of the PR.
Linked Issues Check ✅ Passed The PR implements a server-rendered block (src/blocks/DocNavigation and assets/build/blocks/DocNavigation), an Edit component with InspectorControls exposing extensive styling attributes, a save() that returns null, a PHP renderer (render_wedocs_doc_navigation) and block registration in wedocs.php, which together satisfy the linked issue's request to provide a Gutenberg block and relevant customization settings. The new wedocs_get_doc_navigation_posts() centralizes the prev/next logic and is used by the block renderer, aligning the code with the feature objective. Based on the provided diffs the block's attributes and editor controls cover the declared customization requirements from issue #167.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

🧹 Nitpick comments (24)
assets/build/index.asset.php (1)

1-1: Confirm React handles vs wp-element.

Deps list includes react and react-dom alongside wp-element. Verify supported WP versions register these handles; otherwise prefer externalizing to wp-element only to avoid duplication.

src/blocks/DocNavigation/edit.js (3)

82-90: Remove unused themeGradients; tighten selector payload.

themeGradients isn’t used; keep only what you need.

Apply:

-    const { themeColors, themeGradients } = useSelect((select) => {
+    const { themeColors } = useSelect((select) => {
         const editorSettings = select('core/block-editor').getSettings();
         return {
-            themeColors: editorSettings.colors,
-            themeGradients: editorSettings.gradients,
+            themeColors: editorSettings.colors,
         };
     });

53-59: Arrow margin should be directional (prev vs next).

Single margin string yields wrong spacing on the next arrow. Compute per‑direction margins.

Apply:

-        const arrowStyle = {
+        const arrowStyle = {
             fontSize: arrowSize || '16px',
             color: arrowColor || '#333333',
             backgroundColor: arrowBackgroundColor || 'transparent',
             padding: arrowPadding ? `${arrowPadding.top} ${arrowPadding.right} ${arrowPadding.bottom} ${arrowPadding.left}` : '8px',
-            margin: arrowMargin ? `${arrowMargin.top} ${arrowMargin.right} ${arrowMargin.bottom} ${arrowMargin.left}` : '0 8px 0 0'
         };
+        const prevArrowStyle = {
+            ...arrowStyle,
+            margin: arrowMargin ? `${arrowMargin.top} ${arrowMargin.right} ${arrowMargin.bottom} ${arrowMargin.left}` : '0 8px 0 0',
+        };
+        const nextArrowStyle = {
+            ...arrowStyle,
+            margin: arrowMargin ? `${arrowMargin.top} ${arrowMargin.right} ${arrowMargin.bottom} ${arrowMargin.left}` : '0 0 0 8px',
+        };
...
-                        <span className="wedocs-doc-nav-arrow" style={arrowStyle}>←</span>
+                        <span className="wedocs-doc-nav-arrow" style={prevArrowStyle}>←</span>
...
-                        <span className="wedocs-doc-nav-arrow" style={arrowStyle}>→</span>
+                        <span className="wedocs-doc-nav-arrow" style={nextArrowStyle}>→</span>

Also applies to: 65-76


139-142: Clarify option label.

“Add rel” is vague; prefer “rel='prev/next'”.

Apply:

-        { label: __('Add rel', 'wedocs'), value: 'prev_next' }
+        { label: __('rel="prev/next"', 'wedocs'), value: 'prev_next' }
src/blocks/DocNavigation/style.scss (2)

10-67: Avoid blanket !important; scope selectors instead.

Excessive !important can fight themes. Prefer specificity (e.g., wrap in .wp-block-wedocs-wedocs-doc-navigation) and drop !important where not needed.


69-85: Add RTL tweaks for arrow spacing.

Improve UX in RTL locales by swapping arrow margins.

Apply at file end:

+/* RTL adjustments */
+[dir="rtl"] .wedocs-doc-navigation .wedocs-doc-nav-prev .wedocs-doc-nav-arrow { margin-left: 0.5rem; margin-right: 0; }
+[dir="rtl"] .wedocs-doc-navigation .wedocs-doc-nav-next { margin-right: auto; margin-left: 0; }
+[dir="rtl"] .wedocs-doc-navigation .wedocs-doc-nav-next .wedocs-doc-nav-arrow { margin-right: 0.5rem; margin-left: 0; }
src/blocks/DocNavigation/render.php (2)

233-239: Add aria-labels for better a11y.

Expose “Previous/Next: Title” to screen readers.

Apply:

-                    <a href="<?php echo esc_url(get_permalink($prev_post->ID)); ?>" 
-                       <?php echo ($seo_links === 'prev_next') ? 'rel="prev"' : ''; ?>>
+                    <a href="<?php echo esc_url(get_permalink($prev_post->ID)); ?>" 
+                       <?php echo ($seo_links === 'prev_next') ? 'rel="prev"' : ''; ?>
+                       aria-label="<?php echo esc_attr( sprintf( __( 'Previous: %s', 'wedocs' ), $prev_post->post_title ) ); ?>">
...
-                    <a href="<?php echo esc_url(get_permalink($next_post->ID)); ?>" 
-                       <?php echo ($seo_links === 'prev_next') ? 'rel="next"' : ''; ?>>
+                    <a href="<?php echo esc_url(get_permalink($next_post->ID)); ?>" 
+                       <?php echo ($seo_links === 'prev_next') ? 'rel="next"' : ''; ?>
+                       aria-label="<?php echo esc_attr( sprintf( __( 'Next: %s', 'wedocs' ), $next_post->post_title ) ); ?>">

Also applies to: 245-251


9-9: Tidy: silence unused parameter and drop unused local.

$content is required by signature but unused; $wp_class_name is never used.

Apply:

 function render_wedocs_doc_navigation($attributes, $content = '') {
+    // Unused by this renderer.
+    unset( $content );
...
-    $wp_class_name = $attributes['className'] ?? '';

Also applies to: 67-67

src/blocks/DocNavigation/block.json (4)

6-9: Add textdomain for i18n of title/description.

Block metadata should declare the plugin text domain so "title" and "description" can be translated.

   "name": "wedocs/wedocs-doc-navigation",
   "version": "1.0.0",
   "title": "weDocs - Doc Navigation",
+  "textdomain": "wedocs",
   "icon": "arrow-left-alt2",

22-25: Enable text color support to align with WP style engine.

You already handle background via supports; enable text as well so users can use global styles without custom attrs.

   "color": {
-    "background": true
+    "background": true,
+    "text": true
   },

29-32: Constrain seoLinks with an enum.

Prevents invalid values and improves editor tooling.

   "seoLinks": {
     "type": "string",
-    "default": "none"
+    "default": "none",
+    "enum": ["none", "prev_next"]
   },

12-26: Consider relying more on core supports (spacing/border/color/shadow).

Many custom attributes (e.g., navPadding/navMargin) duplicate core supports; this increases surface area and SSR CSS assembly. Prefer core supports unless you need per-item overrides.

Also applies to: 33-121

assets/build/blocks/DocNavigation/render.php (3)

64-68: Remove unused $wp_class_name or actually use it.

It's extracted but not used. Either pass via wrapper attributes or drop it.

-    $wp_class_name = $attributes['className'] ?? '';

232-247: Add accessible labels to navigation links.

Improves a11y for screen readers.

-                    <a href="<?php echo esc_url(get_permalink($prev_post->ID)); ?>" 
-                       <?php echo ($seo_links === 'prev_next') ? 'rel="prev"' : ''; ?>>
+                    <a href="<?php echo esc_url(get_permalink($prev_post->ID)); ?>" 
+                       <?php echo ($seo_links === 'prev_next') ? 'rel="prev"' : ''; ?>
+                       aria-label="<?php echo esc_attr( sprintf( __( 'Previous: %s', 'wedocs' ), $prev_post->post_title ) ); ?>">
@@
-                    <a href="<?php echo esc_url(get_permalink($next_post->ID)); ?>" 
-                       <?php echo ($seo_links === 'prev_next') ? 'rel="next"' : ''; ?>>
+                    <a href="<?php echo esc_url(get_permalink($next_post->ID)); ?>" 
+                       <?php echo ($seo_links === 'prev_next') ? 'rel="next"' : ''; ?>
+                       aria-label="<?php echo esc_attr( sprintf( __( 'Next: %s', 'wedocs' ), $next_post->post_title ) ); ?>">

Also applies to: 244-251


69-134: Rely on core wrapper styles instead of reassembling WP style attributes.

Manual mapping of background/text/border/spacing/shadow duplicates core style engine and risks drift. Prefer passing extra classes/styles to get_block_wrapper_attributes and let supports handle the rest.

assets/build/store.js (3)

321-331: Wrap switch-case declarations in a block and prevent doc duplicates.

Fixes lint error (noSwitchDeclarations) and avoids multiple entries of the same doc ID.

-    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_DOC': {
+      const idx = state.docs.findIndex(d => d?.id === action?.doc?.id);
+      const nextDocs = idx === -1
+        ? [...state.docs, action.doc]
+        : Object.assign([...state.docs], { [idx]: action.doc });
+      const setDocState = { ...state, docs: nextDocs };
+      const isNotInParent = !state.parents.some(parent => parent?.id === action?.doc?.id);
+      if (!action.doc.parent && isNotInParent) {
+        setDocState.parents = [{ ...action.doc }, ...state.parents];
+      }
+      return setDocState;
+    }

372-377: Fix unsafe optional chaining with spread in REMOVE_DOC.

Spreading possibly-undefined (docs?.filter(...)) can throw. Use safe arrays.

-    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':
+      return {
+        ...state,
+        docs: (Array.isArray(state.docs) ? state.docs : []).filter(doc => doc.id !== action.docId),
+        parents: (Array.isArray(state.parents) ? state.parents : []).filter(parent => parent.id !== action.docId)
+      };

410-419: Sort without mutating and guard inputs.

Use defensive copies and array checks to avoid latent issues.

-    const parentDocs = response.filter(doc => !doc.parent);
-    const sortableDocs = parentDocs?.sort((a, b) => a.menu_order - b.menu_order);
+    const parentDocs = Array.isArray(response) ? response.filter(doc => !doc.parent) : [];
+    const sortableDocs = [...parentDocs].sort((a, b) => a.menu_order - b.menu_order);

Apply similar pattern to selectors where sections?.sort / articles?.sort are used.

Also applies to: 525-546, 548-562

wedocs.php (1)

187-193: Rely on block.json render to avoid duplication.

Since block.json already declares "render": "file:./render.php", you can omit render_callback here and let metadata drive it.

-        register_block_type(
-            plugin_dir_path(__FILE__) . 'assets/build/blocks/DocNavigation',
-            array(
-                'render_callback' => 'render_wedocs_doc_navigation'
-            )
-        );
+        register_block_type( plugin_dir_path(__FILE__) . 'assets/build/blocks/DocNavigation' );
src/blocks/DocNavigation/index.js (1)

2-2: Remove unused import.

__ isn’t used in this module; drop the import to avoid an unnecessary dependency.

-import { __ } from '@wordpress/i18n';
assets/build/index.css (1)

1-3: Exclude built artifacts from linters.

Biome errors here are from vendor/minified CSS (e.g., old IE filters, duplicate properties for prefixes). Add a linter ignore for assets/build/** to reduce noise and CI churn.

Example Biome override (adjust path as needed):

{
  "overrides": [{ "include": ["assets/build/**"], "linter": { "enabled": false }, "formatter": { "enabled": false } }]
}
assets/build/blocks/DocNavigation/block.json (3)

1-9: Add textdomain and consider inserter category.

  • Add "textdomain": "wedocs" for translations extracted from block metadata.
  • Optional: consider "category": "design" for better inserter discoverability (many sites don’t use the legacy “widgets” category in post editor).
 {
   "$schema": "https://schemas.wp.org/trunk/block.json",
   "apiVersion": 3,
   "name": "wedocs/wedocs-doc-navigation",
   "version": "1.0.0",
+  "textdomain": "wedocs",
   "title": "weDocs - Doc Navigation",
-  "icon": "arrow-left-alt2",
-  "category": "widgets",
+  "icon": "arrow-left-alt2",
+  "category": "design",

27-121: Sanitize and escape server-rendered styles/URLs.

Ensure render.php sanitizes color/dimension values and escapes URLs/attributes to avoid XSS via block attributes.

Checklist:

  • esc_url() for hrefs
  • esc_attr() for style fragments and aria labels
  • sanitize_hex_color() for hex colors
  • wp_kses_post() where HTML fragments are unavoidable

29-32: Clarify SEO behavior: seoLinks adds rel attributes to the anchor elements (no head output).

  • Findings: assets/build/blocks/DocNavigation/block.json default is "none". assets/build/blocks/DocNavigation/render.php sets rel="prev" (line 234) and rel="next" (line 246) on the nav anchors when $seo_links === 'prev_next'. No wp_head/add_action or <link rel="prev"/"next"> in the head was found.
  • Action: If you want head-level for crawlers, add a wp_head-hooked function that echoes only when the prev/next posts exist; otherwise document that seoLinks controls anchor rel attributes only.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c80fc84 and 5cce480.

⛔ Files ignored due to path filters (9)
  • assets/build/block.js.map is excluded by !**/*.map
  • assets/build/frontend.css.map is excluded by !**/*.map
  • assets/build/frontend.js.map is excluded by !**/*.map
  • assets/build/index.css.map is excluded by !**/*.map
  • assets/build/index.js.map is excluded by !**/*.map
  • assets/build/print.css.map is excluded by !**/*.map
  • assets/build/print.js.map is excluded by !**/*.map
  • assets/build/store.js.map is excluded by !**/*.map
  • assets/build/style-block.css.map is excluded by !**/*.map
📒 Files selected for processing (21)
  • assets/build/block.asset.php (1 hunks)
  • assets/build/blocks/DocNavigation/block.json (1 hunks)
  • assets/build/blocks/DocNavigation/render.php (1 hunks)
  • assets/build/frontend.asset.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 (2 hunks)
  • assets/build/print.asset.php (1 hunks)
  • assets/build/print.css (2 hunks)
  • assets/build/print.js (1 hunks)
  • assets/build/store.asset.php (1 hunks)
  • assets/build/store.js (1 hunks)
  • src/blocks/DocNavigation/block.json (1 hunks)
  • src/blocks/DocNavigation/edit.js (1 hunks)
  • src/blocks/DocNavigation/index.js (1 hunks)
  • src/blocks/DocNavigation/render.php (1 hunks)
  • src/blocks/DocNavigation/save.js (1 hunks)
  • src/blocks/DocNavigation/style.scss (1 hunks)
  • src/blocks/index.js (1 hunks)
  • wedocs.php (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/blocks/DocNavigation/index.js (1)
src/blocks/DocNavigation/edit.js (1)
  • Edit (12-310)
assets/build/blocks/DocNavigation/render.php (1)
src/blocks/DocNavigation/edit.js (1)
  • attributes (14-33)
assets/build/store.js (10)
src/components/DocListing/index.js (3)
  • docs (27-29)
  • loading (37-40)
  • sections (103-103)
src/components/Documentations/index.js (3)
  • docs (19-22)
  • loading (29-32)
  • parentDocs (24-27)
src/components/SelectBox.js (1)
  • pages (16-16)
src/blocks/DocsGrid/edit.js (2)
  • loading (18-18)
  • wp (16-16)
src/components/DocListing/ListingHeader.js (1)
  • loading (13-16)
src/components/Documentations/ParentDocs.js (2)
  • sections (36-40)
  • articles (42-46)
src/components/AddArticleModal.js (1)
  • articles (35-39)
src/components/DocListing/QuickEditModal.js (1)
  • articles (34-38)
src/components/DocListing/DocSections.js (2)
  • articles (70-70)
  • settings (86-89)
src/components/ProPreviews/index.js (1)
  • isProLoaded (13-16)
src/blocks/DocNavigation/render.php (2)
assets/build/blocks/DocNavigation/render.php (1)
  • render_wedocs_doc_navigation (9-259)
src/blocks/DocNavigation/edit.js (1)
  • attributes (14-33)
🪛 Biome (2.1.2)
assets/build/index.css

[error] 5851-5851: Unexpected value or character.

Expected one of:

(parse)


[error] 5851-5851: Unexpected value or character.

Expected one of:

(parse)


[error] 5851-5851: Unexpected value or character.

Expected one of:

(parse)


[error] 5851-5851: Unexpected value or character.

Expected one of:

(parse)


[error] 5851-5851: Unexpected value or character.

Expected one of:

(parse)


[error] 5851-5851: Unexpected shorthand property flex-flow after flex-direction

(lint/suspicious/noShorthandPropertyOverrides)


[error] 5851-5851: 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] 5851-5851: 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] 5851-5851: 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)

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)

🪛 PHPMD (2.15.0)
assets/build/blocks/DocNavigation/render.php

9-9: Avoid unused parameters such as '$content'. (undefined)

(UnusedFormalParameter)


67-67: Avoid unused local variables such as '$wp_class_name'. (undefined)

(UnusedLocalVariable)

src/blocks/DocNavigation/render.php

9-9: Avoid unused parameters such as '$content'. (undefined)

(UnusedFormalParameter)


67-67: Avoid unused local variables such as '$wp_class_name'. (undefined)

(UnusedLocalVariable)

🔇 Additional comments (16)
src/blocks/DocNavigation/save.js (1)

1-4: SSR dynamic block: save returns null — OK.

Looks correct for a server-rendered block.

Please confirm the render callback is registered (block.json “render” → PHP) and that editor preview reflects server output.

assets/build/frontend.asset.php (1)

1-1: Asset version bump only — OK.

assets/build/print.asset.php (1)

1-1: Asset version bump only — OK.

assets/build/print.css (2)

1-3: Adding loader header is fine.

No behavior change.


185-186: No action needed — print.css.map is present.
fd found one print.css.map under assets; keep the sourceMappingURL.

assets/build/store.asset.php (1)

1-1: Asset version bump only — OK.

assets/build/frontend.css (3)

1-3: Adding loader header is fine.


1223-1224: Frontend source map present — no action required.
frontend.css references frontend.css.map and the map file is present (1 match).


4-11: Incorrect — SVG path is correct (no change needed).
assets/build/5f4138884ca3828fa2ff.svg exists next to assets/build/frontend.css, so url(5f4138884ca3828fa2ff.svg) resolves correctly; do not add a fonts/ prefix.

Likely an incorrect or invalid review comment.

assets/build/block.asset.php (1)

1-1: Added wp-data dependency — OK.

Matches a block using @wordpress/data in editor.

Confirm the enqueue uses this asset file so wp-data loads before the block script.

assets/build/frontend.js (1)

1-28: Bundled runtime looks fine.

Standard webpack bootstrap with sourcemap; no actionable issues here.

assets/build/print.js (1)

1-28: OK to ship.

Matches the frontend bundle pattern; nothing risky in the generated runtime.

src/blocks/index.js (1)

3-3: LGTM: DocNavigation is properly registered in the bundle.

wedocs.php (1)

50-50: LGTM: Loads DocNavigation render file.

Consistent with DocsGrid include.

assets/build/store.js (1)

1218-1224: Confirm all call sites import the correct store symbol (docsStore vs docStore/settingsStore).

Some components (QuickEditModal/AddArticleModal in snippets) reference docStore; ensure that matches the exported name.

src/blocks/DocNavigation/index.js (1)

7-17: Dynamic block registration looks correct.

SSR via save: () => null with metadata-based registration is aligned with WP best practices.

Comment on lines +60 to +69
// Get WordPress style system classes and styles
$wrapper_attributes = get_block_wrapper_attributes();

// Extract WordPress style system attributes
$wp_style = $attributes['style'] ?? [];
$wp_background_color = $attributes['backgroundColor'] ?? null;
$wp_text_color = $attributes['textColor'] ?? null;
$wp_class_name = $attributes['className'] ?? '';

// Build WordPress style system styles
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix duplicate class attribute on wrapper; merge classes into get_block_wrapper_attributes.

Current markup yields two class= attributes, which is invalid HTML.

-    // Get WordPress style system classes and styles
-    $wrapper_attributes = get_block_wrapper_attributes();
+    // Get WordPress style system classes and styles
+    $wrapper_attributes = get_block_wrapper_attributes( array(
+        'class' => 'wedocs-document wedocs-doc-navigation-preview'
+    ) );
@@
-    <?php echo $hover_css; ?>
-    <div class="wedocs-document wedocs-doc-navigation-preview" <?php echo $wrapper_attributes; ?>>
+    <?php echo $hover_css; ?>
+    <div <?php echo $wrapper_attributes; ?>>

Also applies to: 229-231

🧰 Tools
🪛 PHPMD (2.15.0)

67-67: Avoid unused local variables such as '$wp_class_name'. (undefined)

(UnusedLocalVariable)

🤖 Prompt for AI Agents
In assets/build/blocks/DocNavigation/render.php around lines 60-69 (also applies
to lines 229-231): the code currently sets $wp_class_name separately which ends
up producing a duplicate class="..." on the wrapper; instead merge any
additional classes into the array/argument passed to
get_block_wrapper_attributes so the function returns a single class attribute.
Concretely, remove separate class output and build a single attributes array (or
append to $attributes['className']) before calling get_block_wrapper_attributes
so all classes are combined into that call and only one class attribute is
rendered.

Comment on lines +5848 to 5852
/*!************************************************************************************************************************************************************************************************************************!*\
!*** 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}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Scope the imported carousel CSS to prevent global collisions.

The inlined react-responsive-carousel rules target .carousel globally and can clash with themes/other plugins. Prefix to the plugin namespace (e.g., .wedocs-document .carousel …) during build (PostCSS prefixer) or load this stylesheet only when the DocNavigation UI that needs it is present.

I can share a minimal PostCSS config to prefix just this import if helpful.

🧰 Tools
🪛 Biome (2.1.2)

[error] 5851-5851: Unexpected value or character.

Expected one of:

(parse)


[error] 5851-5851: Unexpected value or character.

Expected one of:

(parse)


[error] 5851-5851: Unexpected value or character.

Expected one of:

(parse)


[error] 5851-5851: Unexpected value or character.

Expected one of:

(parse)


[error] 5851-5851: Unexpected value or character.

Expected one of:

(parse)


[error] 5851-5851: Unexpected shorthand property flex-flow after flex-direction

(lint/suspicious/noShorthandPropertyOverrides)


[error] 5851-5851: 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] 5851-5851: 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] 5851-5851: 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 around lines 5848 to 5852, the imported
react-responsive-carousel rules are global (targeting .carousel) and may collide
with other UI; scope them by prefixing all selectors with the DocNavigation
container class (for example `.wedocs-document .carousel ...`) or configure
PostCSS to apply a namespace only for this imported file; alternatively load the
carousel stylesheet conditionally only when the DocNavigation UI is mounted.
Ensure every selector in that block is updated to include the chosen namespace
so the rules only apply inside the docs component.

⚠️ Potential issue

🧩 Analysis chain

Preserve third‑party license text.

You’re redistributing minified CSS from react-responsive-carousel (MIT). Ensure the MIT notice is included in distribution (e.g., THIRD_PARTY_LICENSES.md and/or retained header in a vendored CSS file).


🏁 Script executed:

#!/bin/bash
# Verify react-responsive-carousel license presence and create a quick report.

echo "1) Locate package license file:"
fd -HI --glob "react-responsive-carousel" node_modules | head -n 3
fd -HI --glob "LICENSE*" node_modules/react-responsive-carousel -a

echo "2) Check repo for third-party notices:"
fd -HI --glob "{THIRD_PARTY*,LICENSE-THIRD-PARTY*,NOTICE*}" | sed 's/^/ - /'

echo "3) Grep for 'react-responsive-carousel' mentions:"
rg -n "react-responsive-carousel|carousel.min.css" -C2

Length of output: 285318


Preserve third‑party MIT license for react‑responsive‑carousel

You bundle react‑responsive‑carousel’s minified CSS into assets/build/index.css but the package is MIT and the published carousel.min.css does not include the MIT header — include the package’s MIT notice in your distribution (e.g. THIRD_PARTY_LICENSES.md or retain the header in a vendored CSS file). (github.com)

Locations: assets/build/index.css (~lines 5848–5852); src/components/ProPreviews/common/UpgradePopup.js (imports carousel.min.css); package.json (react-responsive-carousel ^3.2.23).

🧰 Tools
🪛 Biome (2.1.2)

[error] 5851-5851: Unexpected value or character.

Expected one of:

(parse)


[error] 5851-5851: Unexpected value or character.

Expected one of:

(parse)


[error] 5851-5851: Unexpected value or character.

Expected one of:

(parse)


[error] 5851-5851: Unexpected value or character.

Expected one of:

(parse)


[error] 5851-5851: Unexpected value or character.

Expected one of:

(parse)


[error] 5851-5851: Unexpected shorthand property flex-flow after flex-direction

(lint/suspicious/noShorthandPropertyOverrides)


[error] 5851-5851: 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] 5851-5851: 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] 5851-5851: 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 around lines 5848–5852 (the bundled
react-responsive-carousel minified CSS), the original MIT license header from
the react-responsive-carousel package is missing; add the package's MIT notice
to your distributed files by either restoring the original header at the top of
the vendored CSS or adding the exact license text and package attribution to a
project-wide THIRD_PARTY_LICENSES.md and ensure the build includes that file;
also verify src/components/ProPreviews/common/UpgradePopup.js (which imports
carousel.min.css) and package.json (react-responsive-carousel ^3.2.23) reference
are documented in the license file so the third‑party attribution is shipped
with the app.

} from '@wordpress/components';

const Edit = ({ attributes, setAttributes }) => {
const blockProps = useBlockProps();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Don’t override block wrapper classes; merge via useBlockProps.

You’re overwriting the wp‑generated class (e.g., wp-block-*) by passing className after {...blockProps}. Use useBlockProps({ className }) or merge strings.

Apply:

-    const blockProps = useBlockProps();
+    const blockProps = useBlockProps({ className: 'wedocs-document wedocs-doc-navigation-preview' });
...
-            <div {...blockProps} className="wedocs-document wedocs-doc-navigation-preview">
+            <div {...blockProps}>

Also applies to: 62-63

🤖 Prompt for AI Agents
In src/blocks/DocNavigation/edit.js around lines 13 and 62-63, the code
currently spreads {...blockProps} and then passes className afterwards which
overwrites the wp-generated wrapper classes; update to call useBlockProps with
the className prop (e.g. useBlockProps({ className })) or merge the class
strings before passing so the generated wp-block-* classes are preserved and
your custom classes are appended.

Comment on lines +157 to +171
<PanelColorSettings
colors={[
{
colors: themeColors,
name: __('Theme', 'wedocs'),
}
]}
colorSettings={[
{
value: navBorderColor,
label: __('Border Color', 'wedocs'),
onChange: (newBorderColor) => updateAttribute('navBorderColor')(newBorderColor)
}
]}
/>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

PanelColorSettings: wrong prop shape for colors.

PanelColorSettings expects a flat palette array, not an array of objects with a nested colors field.

Apply:

-                    <PanelColorSettings
-                        colors={[
-                            {
-                                colors: themeColors,
-                                name: __('Theme', 'wedocs'),
-                            }
-                        ]}
+                    <PanelColorSettings
+                        colors={ themeColors }
                         colorSettings={[
                             {
                                 value: navBorderColor,
                                 label: __('Border Color', 'wedocs'),
-                                onChange: (newBorderColor) => updateAttribute('navBorderColor')(newBorderColor)
+                                onChange: updateAttribute('navBorderColor')
                             }
                         ]}
                     />
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<PanelColorSettings
colors={[
{
colors: themeColors,
name: __('Theme', 'wedocs'),
}
]}
colorSettings={[
{
value: navBorderColor,
label: __('Border Color', 'wedocs'),
onChange: (newBorderColor) => updateAttribute('navBorderColor')(newBorderColor)
}
]}
/>
<PanelColorSettings
colors={ themeColors }
colorSettings={[
{
value: navBorderColor,
label: __('Border Color', 'wedocs'),
onChange: updateAttribute('navBorderColor')
}
]}
/>
🤖 Prompt for AI Agents
In src/blocks/DocNavigation/edit.js around lines 157 to 171, PanelColorSettings
is being passed an array with an object ({ colors: themeColors, name: 'Theme' })
but it expects a flat palette array; replace the colors prop with the flat
themeColors array (colors={ themeColors }) and remove the wrapping object, keep
the colorSettings prop as-is; if you need to show a label for the palette, use
the panel or block UI label elsewhere or pass a separate prop supported by
PanelColorSettings.

Comment on lines 37 to 48
$next_query = "SELECT ID, post_title FROM {$wpdb->posts}
WHERE post_parent = {$post->post_parent} and post_type = 'docs' and post_status = 'publish' and menu_order > {$post->menu_order}
ORDER BY menu_order ASC
LIMIT 0, 1";

$prev_query = "SELECT ID, post_title FROM {$wpdb->posts}
WHERE post_parent = {$post->post_parent} and post_type = 'docs' and post_status = 'publish' and menu_order < {$post->menu_order}
ORDER BY menu_order DESC
LIMIT 0, 1";

$next_post = $wpdb->get_row($next_query);
$prev_post = $wpdb->get_row($prev_query);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Use $wpdb->prepare and cast ints for safety and stability.

Avoid string interpolation; cast and prepare. Add secondary sort for determinism.

Apply:

-    $next_query = "SELECT ID, post_title FROM {$wpdb->posts}
-        WHERE post_parent = {$post->post_parent} and post_type = 'docs' and post_status = 'publish' and menu_order > {$post->menu_order}
-        ORDER BY menu_order ASC
-        LIMIT 0, 1";
-
-    $prev_query = "SELECT ID, post_title FROM {$wpdb->posts}
-        WHERE post_parent = {$post->post_parent} and post_type = 'docs' and post_status = 'publish' and menu_order < {$post->menu_order}
-        ORDER BY menu_order DESC
-        LIMIT 0, 1";
+    $parent     = (int) $post->post_parent;
+    $menu_order = (int) $post->menu_order;
+
+    $next_query = $wpdb->prepare(
+        "SELECT ID, post_title FROM {$wpdb->posts}
+         WHERE post_parent = %d AND post_type = %s AND post_status = 'publish' AND menu_order > %d
+         ORDER BY menu_order ASC, ID ASC
+         LIMIT 1",
+         $parent, 'docs', $menu_order
+    );
+
+    $prev_query = $wpdb->prepare(
+        "SELECT ID, post_title FROM {$wpdb->posts}
+         WHERE post_parent = %d AND post_type = %s AND post_status = 'publish' AND menu_order < %d
+         ORDER BY menu_order DESC, ID DESC
+         LIMIT 1",
+         $parent, 'docs', $menu_order
+    );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
$next_query = "SELECT ID, post_title FROM {$wpdb->posts}
WHERE post_parent = {$post->post_parent} and post_type = 'docs' and post_status = 'publish' and menu_order > {$post->menu_order}
ORDER BY menu_order ASC
LIMIT 0, 1";
$prev_query = "SELECT ID, post_title FROM {$wpdb->posts}
WHERE post_parent = {$post->post_parent} and post_type = 'docs' and post_status = 'publish' and menu_order < {$post->menu_order}
ORDER BY menu_order DESC
LIMIT 0, 1";
$next_post = $wpdb->get_row($next_query);
$prev_post = $wpdb->get_row($prev_query);
$parent = (int) $post->post_parent;
$menu_order = (int) $post->menu_order;
$next_query = $wpdb->prepare(
"SELECT ID, post_title FROM {$wpdb->posts}
WHERE post_parent = %d AND post_type = %s AND post_status = 'publish' AND menu_order > %d
ORDER BY menu_order ASC, ID ASC
LIMIT 1",
$parent, 'docs', $menu_order
);
$prev_query = $wpdb->prepare(
"SELECT ID, post_title FROM {$wpdb->posts}
WHERE post_parent = %d AND post_type = %s AND post_status = 'publish' AND menu_order < %d
ORDER BY menu_order DESC, ID DESC
LIMIT 1",
$parent, 'docs', $menu_order
);
$next_post = $wpdb->get_row($next_query);
$prev_post = $wpdb->get_row($prev_query);

}

// Get WordPress style system classes and styles
$wrapper_attributes = get_block_wrapper_attributes();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Invalid markup: duplicate class attribute + “preview” class leaking to frontend.

You echo a hardcoded class and wrapper attributes (which include class), producing two class="" attrs. Also, “-preview” should be editor‑only.

Apply:

-    $wrapper_attributes = get_block_wrapper_attributes();
+    $wrapper_attributes = get_block_wrapper_attributes( array( 'class' => 'wedocs-document' ) );
...
-    <div class="wedocs-document wedocs-doc-navigation-preview" <?php echo $wrapper_attributes; ?>>
+    <div <?php echo $wrapper_attributes; ?>>

Also applies to: 229-229

🤖 Prompt for AI Agents
In src/blocks/DocNavigation/render.php around lines 61 and 229, you are echoing
a hardcoded class attribute alongside the output of
get_block_wrapper_attributes(), which already includes a class attribute,
causing duplicate class="" markup and leaking the "-preview" class to the
frontend; remove the hardcoded class attribute and instead pass your classes
into get_block_wrapper_attributes(['class' => 'your-class names']) so it merges
them properly, and only append the "-preview" class when in the editor (wrap
that class addition with an is_editor() check before passing it into
get_block_wrapper_attributes).

Comment on lines +138 to +147
// Handle custom padding (always apply since WordPress style system doesn't have spacing in this case)
if ($nav_padding) {
$custom_nav_styles[] = sprintf(
'padding: %s %s %s %s;',
esc_attr($nav_padding['top'] ?? '12px'),
esc_attr($nav_padding['right'] ?? '16px'),
esc_attr($nav_padding['bottom'] ?? '12px'),
esc_attr($nav_padding['left'] ?? '16px')
);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Frontend default padding missing (editor shows 12px 16px).

PHP only applies padding when navPadding is set, causing editor/frontend mismatch.

Apply:

-    if ($nav_padding) {
-        $custom_nav_styles[] = sprintf(
-            'padding: %s %s %s %s;',
-            esc_attr($nav_padding['top'] ?? '12px'),
-            esc_attr($nav_padding['right'] ?? '16px'),
-            esc_attr($nav_padding['bottom'] ?? '12px'),
-            esc_attr($nav_padding['left'] ?? '16px')
-        );
-    }
+    $custom_nav_styles[] = sprintf(
+        'padding: %s %s %s %s;',
+        esc_attr($nav_padding['top'] ?? '12px'),
+        esc_attr($nav_padding['right'] ?? '16px'),
+        esc_attr($nav_padding['bottom'] ?? '12px'),
+        esc_attr($nav_padding['left'] ?? '16px')
+    );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Handle custom padding (always apply since WordPress style system doesn't have spacing in this case)
if ($nav_padding) {
$custom_nav_styles[] = sprintf(
'padding: %s %s %s %s;',
esc_attr($nav_padding['top'] ?? '12px'),
esc_attr($nav_padding['right'] ?? '16px'),
esc_attr($nav_padding['bottom'] ?? '12px'),
esc_attr($nav_padding['left'] ?? '16px')
);
}
// Handle custom padding (always apply since WordPress style system doesn't have spacing in this case)
$custom_nav_styles[] = sprintf(
'padding: %s %s %s %s;',
esc_attr($nav_padding['top'] ?? '12px'),
esc_attr($nav_padding['right'] ?? '16px'),
esc_attr($nav_padding['bottom'] ?? '12px'),
esc_attr($nav_padding['left'] ?? '16px')
);
🤖 Prompt for AI Agents
In src/blocks/DocNavigation/render.php around lines 138 to 147, the code only
appends padding styles when $nav_padding is truthy, causing the frontend to miss
the editor's default 12px 16px padding; always compute padding values using the
existing defaults (top: 12px, right: 16px, bottom: 12px, left: 16px) by reading
from $nav_padding with those fallbacks and unconditionally add the resulting
padding declaration to $custom_nav_styles so the frontend matches the editor.

@Rubaiyat-E-Mohammad
Copy link

Navigation block not showing at frontend. @sapayth vai

Screen.Recording.2025-09-22.at.10.40.09.AM.mov

@sapayth sapayth added Need Testing and removed bug labels Sep 22, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (4)
src/blocks/DocNavigation/render.php (3)

9-9: Remove unused parameter to clean up code.

The $content parameter is not used in this function and should be removed.

Apply this diff:

-function render_wedocs_doc_navigation($attributes, $content = '') {
+function render_wedocs_doc_navigation($attributes) {

31-31: Remove unused variable to clean up code.

The $wpdb global variable is declared but never used in this function.

Apply this diff:

-    global $post, $wpdb;
+    global $post;

178-182: Consider using CSS classes instead of inline hover styles.

Inline styles with !important can make styling harder to maintain. Consider adding CSS classes to the block's stylesheet instead.

assets/build/blocks/DocNavigation/render.php (1)

216-216: Remove editor-only preview class from frontend output.

The "wedocs-doc-navigation-preview" class should only be present in the editor, not on the frontend.

Apply this diff to remove the preview class from the frontend:

-        'class' => 'wedocs-document wedocs-doc-navigation-preview' 
+        'class' => 'wedocs-document' 
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5cce480 and c7782bf.

📒 Files selected for processing (3)
  • assets/build/blocks/DocNavigation/render.php (1 hunks)
  • includes/functions.php (1 hunks)
  • src/blocks/DocNavigation/render.php (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
assets/build/blocks/DocNavigation/render.php (2)
src/blocks/DocNavigation/edit.js (1)
  • attributes (14-33)
includes/functions.php (1)
  • wedocs_get_doc_navigation_posts (191-238)
src/blocks/DocNavigation/render.php (2)
src/blocks/DocNavigation/edit.js (1)
  • attributes (14-33)
includes/functions.php (1)
  • wedocs_get_doc_navigation_posts (191-238)
🪛 PHPMD (2.15.0)
assets/build/blocks/DocNavigation/render.php

9-9: Avoid unused parameters such as '$content'. (undefined)

(UnusedFormalParameter)


31-31: Avoid unused local variables such as '$wpdb'. (undefined)

(UnusedLocalVariable)


54-54: Avoid unused local variables such as '$wp_class_name'. (undefined)

(UnusedLocalVariable)

src/blocks/DocNavigation/render.php

9-9: Avoid unused parameters such as '$content'. (undefined)

(UnusedFormalParameter)


31-31: Avoid unused local variables such as '$wpdb'. (undefined)

(UnusedLocalVariable)


54-54: Avoid unused local variables such as '$wp_class_name'. (undefined)

(UnusedLocalVariable)

🔇 Additional comments (9)
includes/functions.php (2)

191-238: Excellent navigation logic implementation!

The new wedocs_get_doc_navigation_posts function handles edge cases well (menu_order = 0, non-sequential ordering) with a proper fallback mechanism. The function correctly separates concerns from the rendering logic.


246-267: LGTM! Good refactor using the new navigation function.

The updated wedocs_doc_nav() function properly delegates to the new navigation logic while maintaining the same output format. The code is cleaner and more maintainable.

assets/build/blocks/DocNavigation/render.php (5)

9-9: Remove unused parameter to clean up code.

The $content parameter is not used in this function and should be removed.

Apply this diff:

-function render_wedocs_doc_navigation($attributes, $content = '') {
+function render_wedocs_doc_navigation($attributes) {

31-31: Remove unused variable to clean up code.

The $wpdb global variable is declared but never used in this function.

Apply this diff:

-    global $post, $wpdb;
+    global $post;

178-182: Consider using CSS classes instead of inline hover styles.

Inline styles with !important can make styling harder to maintain. Consider adding CSS classes to the block's stylesheet instead.


48-48: Fix duplicate class attribute on wrapper.

The code produces duplicate class attributes because get_block_wrapper_attributes() already returns a class attribute, and you're adding another hardcoded class attribute.

Apply this diff:

-    $wrapper_attributes = get_block_wrapper_attributes();
+    $wrapper_attributes = get_block_wrapper_attributes( array( 
+        'class' => 'wedocs-document wedocs-doc-navigation-preview' 
+    ) );
    
-    $wp_class_name = $attributes['className'] ?? '';

And update line 216:

-    <div class="wedocs-document wedocs-doc-navigation-preview" <?php echo $wrapper_attributes; ?>>
+    <div <?php echo $wrapper_attributes; ?>>

Also applies to: 54-54


125-134: Frontend default padding missing (editor shows 12px 16px).

The PHP only applies padding when navPadding is set, causing editor/frontend mismatch. The editor has default padding values that should be applied on the frontend too.

Apply this diff:

-    if ($nav_padding) {
-        $custom_nav_styles[] = sprintf(
-            'padding: %s %s %s %s;',
-            esc_attr($nav_padding['top'] ?? '12px'),
-            esc_attr($nav_padding['right'] ?? '16px'),
-            esc_attr($nav_padding['bottom'] ?? '12px'),
-            esc_attr($nav_padding['left'] ?? '16px')
-        );
-    }
+    $custom_nav_styles[] = sprintf(
+        'padding: %s %s %s %s;',
+        esc_attr($nav_padding['top'] ?? '12px'),
+        esc_attr($nav_padding['right'] ?? '16px'),
+        esc_attr($nav_padding['bottom'] ?? '12px'),
+        esc_attr($nav_padding['left'] ?? '16px')
+    );
src/blocks/DocNavigation/render.php (2)

48-48: Fix duplicate class attribute on wrapper.

The code produces duplicate class attributes because get_block_wrapper_attributes() already returns a class attribute, and you're adding another hardcoded class attribute.

Apply this diff:

-    $wrapper_attributes = get_block_wrapper_attributes();
+    $wrapper_attributes = get_block_wrapper_attributes( array( 
+        'class' => 'wedocs-document wedocs-doc-navigation-preview' 
+    ) );
    
-    $wp_class_name = $attributes['className'] ?? '';

And update line 216:

-    <div class="wedocs-document wedocs-doc-navigation-preview" <?php echo $wrapper_attributes; ?>>
+    <div <?php echo $wrapper_attributes; ?>>

Also applies to: 54-54


125-134: Frontend default padding missing (editor shows 12px 16px).

The PHP only applies padding when navPadding is set, causing editor/frontend mismatch. The editor has default padding values that should be applied on the frontend too.

Apply this diff:

-    if ($nav_padding) {
-        $custom_nav_styles[] = sprintf(
-            'padding: %s %s %s %s;',
-            esc_attr($nav_padding['top'] ?? '12px'),
-            esc_attr($nav_padding['right'] ?? '16px'),
-            esc_attr($nav_padding['bottom'] ?? '12px'),
-            esc_attr($nav_padding['left'] ?? '16px')
-        );
-    }
+    $custom_nav_styles[] = sprintf(
+        'padding: %s %s %s %s;',
+        esc_attr($nav_padding['top'] ?? '12px'),
+        esc_attr($nav_padding['right'] ?? '16px'),
+        esc_attr($nav_padding['bottom'] ?? '12px'),
+        esc_attr($nav_padding['left'] ?? '16px')
+    );

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants