Configured wedocs new package system #250
Conversation
|
Warning Rate limit exceeded@iftakharul-islam has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 27 minutes and 35 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (12)
📒 Files selected for processing (16)
WalkthroughAdds a new dynamic Table of Contents block (editor, frontend, styles, server render), refactors DocsGrid PHP rendering with guarded functions and inline styles/JS, updates block registration to conditionally register blocks from Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Extra attention recommended:
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
package.json(1 hunks)src/blocks/DocsGrid/render.php(1 hunks)webpack.config.js(1 hunks)wedocs.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/blocks/DocsGrid/render.php (2)
src/blocks/DocsGrid/edit.js (1)
attributes(20-48)src/blocks/DocsGrid/save.js (1)
attributes(4-16)
🪛 PHPMD (2.15.0)
src/blocks/DocsGrid/render.php
182-182: Avoid unused local variables such as '$total_pages'. (undefined)
(UnusedLocalVariable)
227-227: Avoid unused local variables such as '$total_articles'. (undefined)
(UnusedLocalVariable)
| return ob_get_clean(); | ||
| } | ||
| echo render_wedocs_docs_grid($attributes); | ||
| } |
There was a problem hiding this comment.
Return the rendered markup instead of echoing
When a block.json uses the render file, WordPress expects that file to return the HTML string. Echoing here causes the metadata loader to return 1, which results in broken front-end output. Replace the echo with a return so the block renders correctly.
- echo render_wedocs_docs_grid($attributes);
+ return render_wedocs_docs_grid($attributes);📝 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.
| return ob_get_clean(); | |
| } | |
| echo render_wedocs_docs_grid($attributes); | |
| } | |
| return ob_get_clean(); | |
| } | |
| return render_wedocs_docs_grid($attributes); | |
| } |
🤖 Prompt for AI Agents
In src/blocks/DocsGrid/render.php around lines 321 to 324, the file currently
echoes the rendered markup which causes WordPress to return 1 from the render
file; replace the final echo call with a return so the function returns the HTML
string (i.e., change echo render_wedocs_docs_grid($attributes); to return
render_wedocs_docs_grid($attributes);) and ensure no additional output is sent
after the return.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
wedocs.php (2)
176-188: Apply WordPress coding standards for spacing around control structures.The control structures don't follow WordPress coding standards for spacing. WordPress requires spaces after opening parentheses and before closing parentheses.
Apply this diff to align with WordPress coding standards:
public function register_blocks() { $block_lists = [ WEDOCS_PATH . '/assets/build/blocks/DocsGrid', ]; - foreach ($block_lists as $block) { - if (file_exists($block . '/block.json')) { + foreach ( $block_lists as $block ) { + if ( file_exists( $block . '/block.json' ) ) { // Register the DocsGrid block - register_block_type($block); + register_block_type( $block ); } } }
184-184: Update comment to reflect generic block registration.The comment specifically mentions "DocsGrid block" but this code is inside a loop that can register multiple block types. Consider making the comment more generic to match the actual functionality.
Apply this diff:
- // Register the DocsGrid block + // Register the block register_block_type($block);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
package.json(1 hunks)wedocs.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/blocks/TableOfContents/render.php (1)
74-95: Inline style building is fine; consider null-coalescing to avoid empty style attr.Current get_block_wrapper_attributes handles null, but you can skip style when no styles exist.
- 'style' => !empty($inline_styles) ? implode('; ', $inline_styles) : null + // Only set 'style' when we actually have styles. + ...( ! empty( $inline_styles ) ? [ 'style' => implode( '; ', $inline_styles ) ] : [] )src/blocks/TableOfContents/style.scss (1)
58-76: Add visible focus styles for keyboard navigation.Improve accessibility by styling focus-visible on links.
a { text-decoration: none; color: var(--toc-link-color, inherit); display: inline-block; transition: color 0.2s ease; + &:focus-visible { + outline: 2px solid currentColor; + outline-offset: 2px; + } &:hover { color: var(--toc-link-hover-color, currentColor); text-decoration: underline; }Optionally honor reduced motion:
html.smooth-scroll { - scroll-behavior: smooth; + scroll-behavior: smooth; + @media (prefers-reduced-motion: reduce) { + scroll-behavior: auto; + } }Also applies to: 138-141
src/blocks/TableOfContents/edit.js (1)
279-381: Avoid __experimental APIs where stable equivalents exist; mirror final ID strategy in preview.
- Prefer stable ToolsPanel components when your target WP version allows, to reduce churn.
- For consistency, preview numbering/indent is fine; optionally use the same slugify ID strategy used in view/SSR to keep anchors consistent in editor previews.
Please confirm your minimum WP version. If ≥ 6.3, you can replace __experimentalToolsPanel and related with their stable counterparts.
Also applies to: 128-144
wedocs.php (1)
176-189: Apply the suggested refactor to improve maintainability and handle incomplete builds.The verification reveals that
TableOfContentsis not yet built inassets/build/blocks/(onlyDocsGridexists), while the hard-coded array tries to register both. Thefile_exists()guard masks this incomplete build. The suggested refactor addresses this:
- Using
glob()auto-discovers only built blocks, preventing drift from hard-coded lists- Adding
function_exists('register_block_type')guards against older WordPress versions- Future blocks will be automatically registered without code changes
The review comment suggestions are sound and should be applied.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
includes/Frontend.php(1 hunks)src/blocks/TableOfContents/block.json(1 hunks)src/blocks/TableOfContents/edit.js(1 hunks)src/blocks/TableOfContents/editor.scss(1 hunks)src/blocks/TableOfContents/index.js(1 hunks)src/blocks/TableOfContents/render.php(1 hunks)src/blocks/TableOfContents/save.js(1 hunks)src/blocks/TableOfContents/style.scss(1 hunks)src/blocks/TableOfContents/view.js(1 hunks)src/blocks/index.js(1 hunks)wedocs.php(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- includes/Frontend.php
- src/blocks/TableOfContents/save.js
- src/blocks/index.js
🧰 Additional context used
🧬 Code graph analysis (3)
src/blocks/TableOfContents/render.php (2)
src/blocks/TableOfContents/edit.js (2)
attributes(55-70)headings(72-72)src/blocks/TableOfContents/view.js (13)
content(159-159)headings(55-55)headings(174-177)level(104-104)text(105-105)id(106-106)id(175-175)html(95-95)counters(100-100)i(113-113)i(129-129)indent(122-122)numbers(128-128)
src/blocks/TableOfContents/index.js (1)
src/blocks/TableOfContents/edit.js (1)
Edit(54-409)
src/blocks/TableOfContents/edit.js (1)
src/blocks/TableOfContents/view.js (14)
headings(55-55)headings(174-177)supportedHeadings(42-42)showHierarchy(43-43)showNumbering(44-44)counters(100-100)i(113-113)i(129-129)indent(122-122)numberText(125-125)level(104-104)tocTitle(12-12)collapsibleOnMobile(16-16)smoothScroll(15-15)
🔇 Additional comments (5)
src/blocks/TableOfContents/editor.scss (1)
6-27: LGTM for editor styles.Scoped, minimal, and safe for editor. No blocking issues.
src/blocks/TableOfContents/index.js (1)
29-34: Registration pattern is correct.Dynamic block wired via block.json + Edit; no save() needed. Good.
src/blocks/TableOfContents/block.json (3)
106-106: Verify textdomain matches project convention.Line 106 specifies textdomain as
"dynamic-table-of-contents-block-wp", but the AI summary indicates it should be"wedocs". Ensure this matches your project's localization and textdomain configuration to avoid translation/i18n issues.
84-92: Verify color attribute defaults are handled safely in frontend code.The color attributes (
titleColor,linkColor,linkHoverColor) lack default values (lines 85, 88, 91). Confirm that the frontend rendering code, view script, and render.php handler gracefully manageundefinedornullvalues for these attributes to prevent styling breakage.
107-111: Verify asset file paths and naming match build output.Asset paths reference
index.css,style-index.css,index.js,view.js, andrender.php. Confirm these file names align with your webpack/build configuration to ensure assets are correctly located and bundled.
| if ( ! function_exists( 'wedocs_extract_headings_from_content_safe' ) ) { | ||
| function wedocs_extract_headings_from_content_safe($content, $supported_headings) { | ||
| $headings = []; | ||
|
|
||
| if (empty($content) || !is_array($supported_headings)) { | ||
| return $headings; | ||
| } | ||
|
|
||
| // Extract only content within entry-content div for weDocs pages | ||
| // This prevents the TOC from picking up headings from headers, footers, sidebars, etc. | ||
| if (preg_match('/<div[^>]*class=["\'][^"\']*entry-content[^"\']*["\'][^>]*>(.*?)<\/div>/is', $content, $entry_match)) { | ||
| $content = $entry_match[1]; | ||
| } | ||
|
|
||
| // Convert h-tags to lowercase for pattern matching | ||
| $supported_tags = array_map('strtolower', $supported_headings); | ||
| $pattern = '/<(' . implode('|', $supported_tags) . ')(?:[^>]*(?:\s+id=["\']([^"\'][^"\']*)["\'\'])?[^>]*)>(.*?)<\/\1>/i'; | ||
|
|
||
| if (preg_match_all($pattern, $content, $matches, PREG_SET_ORDER)) { | ||
| $used_ids = []; | ||
|
|
||
| foreach ($matches as $match) { | ||
| if (count($match) < 4) continue; | ||
|
|
||
| $tag = strtolower($match[1]); | ||
| $level = (int) substr($tag, 1); | ||
| $existing_id = isset($match[2]) && !empty(trim($match[2])) ? trim($match[2]) : ''; | ||
| $text = wp_strip_all_tags($match[3]); | ||
| $text = trim($text); | ||
|
|
||
| if (empty($text)) continue; | ||
|
|
||
| $id = !empty($existing_id) ? $existing_id : sanitize_title($text); | ||
|
|
||
| // Ensure unique IDs | ||
| $original_id = $id; | ||
| $counter = 1; | ||
| while (in_array($id, $used_ids)) { | ||
| $id = $original_id . '-' . $counter; | ||
| $counter++; | ||
| } | ||
| $used_ids[] = $id; | ||
|
|
||
| $headings[] = [ | ||
| 'level' => $level, | ||
| 'text' => $text, | ||
| 'id' => $id | ||
| ]; | ||
| } | ||
| } | ||
|
|
||
| return $headings; | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix SSR/CSR ID generation mismatch; expand counters to cover H6.
- Server derives IDs from a slug of the text; frontend uses “toc-heading-{index}”. If JS fails, anchors won’t resolve. Align JS to use the same slug algorithm and uniqueness policy.
- Expand $counters to include level 6 and reset loop to 6 to avoid notices if H6 ever appears.
Apply on PHP side (robust counters + small regex cleanup):
- $pattern = '/<(' . implode('|', $supported_tags) . ')(?:[^>]*(?:\s+id=["\']([^"\'][^"\']*)["\'])?[^>]*)>(.*?)<\/\1>/i';
+ $pattern = '/<(' . implode('|', $supported_tags) . ')(?:[^>]*(?:\s+id=["\']([^"\']*)["\'])?[^>]*)>(.*?)<\/\1>/i';- $counters = [1 => 0, 2 => 0, 3 => 0, 4 => 0, 5 => 0];
+ $counters = [1 => 0, 2 => 0, 3 => 0, 4 => 0, 5 => 0, 6 => 0];
...
- for ($i = $level + 1; $i <= 5; $i++) {
+ for ($i = $level + 1; $i <= 6; $i++) {
$counters[$i] = 0;
}Then update view.js to compute the same slug and assign IDs before generating the list (see inline diff in that file’s comment). This makes SSR anchors work without JS and avoids duplicate/mismatched IDs. Based on learnings
Also applies to: 176-249
🤖 Prompt for AI Agents
In src/blocks/TableOfContents/render.php around lines 121-174, the PHP TOC
slug/ID generation and heading-level handling are incomplete: extend the
counters to cover H1–H6 and ensure the uniqueness loop resets/checks up to level
6 (avoid notices if an H6 appears); simplify/clean the regex to reliably capture
optional id attributes and inner text (keep case-insensitive matching and
entry-content extraction), and preserve the sanitize_title-based slug plus the
incrementing uniqueness suffix logic. Then update view.js to compute the exact
same slug algorithm and uniqueness policy client-side (replicate sanitize_title
behavior and the same incremental "-n" suffix strategy), assign those IDs to
headings before building the TOC list so SSR anchors match CSR IDs and anchors
work even if JS runs or fails.
| const smoothScroll = tocBlock.classList.contains('smooth-scroll') || tocBlock.dataset.smoothScroll === 'true'; | ||
| const collapsibleOnMobile = tocBlock.classList.contains('collapsible-mobile') || window.innerWidth <= 768; | ||
|
|
There was a problem hiding this comment.
Front-end behavior bugs: collapsible flag, smoothScroll toggle, ID strategy, and escaping.
- Collapsible is forced on small screens even when disabled.
- smoothScroll=false still intercepts and animates.
- IDs must be assigned before list generation and follow the same slug/uniqueness as PHP.
- Escape text before injecting HTML; expand counters to H6.
Apply:
- const smoothScroll = tocBlock.classList.contains('smooth-scroll') || tocBlock.dataset.smoothScroll === 'true';
- const collapsibleOnMobile = tocBlock.classList.contains('collapsible-mobile') || window.innerWidth <= 768;
+ const smoothScroll = tocBlock.classList.contains('smooth-scroll') || tocBlock.dataset.smoothScroll === 'true';
+ const collapsibleOnMobile = tocBlock.classList.contains('collapsible-mobile');
@@ function generateTOC(tocBlock) {
- // Generate TOC HTML
- const tocHTML = generateTOCHTML(headings, supportedHeadings, showHierarchy, showNumbering);
- const tocContent = tocBlock.querySelector('.toc-content');
- tocContent.innerHTML = tocHTML;
-
- // Add IDs to headings if they don't have them
- headings.forEach(function(heading, index) {
- if (!heading.id) {
- heading.id = 'toc-heading-' + index;
- }
- });
+ // Assign IDs first (slugify + ensure uniqueness) so anchors work even without JS re-rendering.
+ const used = new Set(Array.from(headings).map(h => h.id).filter(Boolean));
+ headings.forEach((heading) => {
+ if (!heading.id) {
+ const base = slugify(heading.textContent || '');
+ let id = base || 'section';
+ let n = 1;
+ while (used.has(id)) { id = `${base}-${n++}`; }
+ heading.id = id;
+ used.add(id);
+ }
+ });
+
+ // Generate TOC HTML
+ const tocHTML = generateTOCHTML(headings, showHierarchy, showNumbering);
+ const tocContent = tocBlock.querySelector('.toc-content');
+ tocContent.innerHTML = tocHTML;- function generateTOCHTML(headings, supportedHeadings, showHierarchy, showNumbering) {
+ function generateTOCHTML(headings, showHierarchy, showNumbering) {
let html = '<ul class="toc-list';
@@
- const counters = { 1: 0, 2: 0, 3: 0, 4: 0, 5: 0 };
+ const counters = { 1: 0, 2: 0, 3: 0, 4: 0, 5: 0, 6: 0 };
let globalCounter = 1;
@@
- const text = heading.textContent.trim();
- const id = heading.id || 'toc-heading-' + Array.from(headings).indexOf(heading);
+ const text = (heading.textContent || '').trim();
+ const id = heading.id;
@@
- for (let i = level + 1; i <= 5; i++) {
+ for (let i = level + 1; i <= 6; i++) {
counters[i] = 0;
}
} else {
counters[level] = globalCounter++;
}
}
@@
- html += '<li style="margin-left: ' + indent + 'px">';
- html += '<a href="#' + id + '" class="toc-link">';
+ html += '<li style="margin-left: ' + indent + 'px">';
+ html += '<a href="#' + encodeURIComponent(id) + '" class="toc-link">';
if (showNumbering) {
html += '<span class="toc-number">' + numberText + '</span>';
}
- html += text;
+ html += escapeHtml(text);
html += '</a></li>'; function setupSmoothScrolling(tocBlock, smoothScroll) {
const links = tocBlock.querySelectorAll('.toc-link');
links.forEach(function(link) {
link.addEventListener('click', function(e) {
- e.preventDefault();
+ if (!smoothScroll) {
+ // Let the browser jump to the anchor normally.
+ return;
+ }
+ e.preventDefault();
const targetId = link.getAttribute('href').substring(1);
const targetElement = document.getElementById(targetId);
if (targetElement) {
- const offsetTop = targetElement.offsetTop - 80; // Account for sticky headers
+ const rect = targetElement.getBoundingClientRect();
+ const offsetTop = window.pageYOffset + rect.top - 80; // Account for sticky headers
if (smoothScroll && 'scrollBehavior' in document.documentElement.style) {
window.scrollTo({
top: offsetTop,
behavior: 'smooth'
});
} else {
// Fallback smooth scroll
animateScrollTo(offsetTop, 600);
}
}
});
});
}
+
+// Helpers
+function slugify(text) {
+ return String(text)
+ .toLowerCase()
+ .trim()
+ .replace(/&/g, '-and-')
+ .replace(/['"]/g, '')
+ .replace(/[^a-z0-9]+/g, '-')
+ .replace(/^-+|-+$/g, '');
+}
+function escapeHtml(s) {
+ return String(s)
+ .replace(/&/g, '&')
+ .replace(/</g, '<')
+ .replace(/>/g, '>')
+ .replace(/"/g, '"')
+ .replace(/'/g, ''');
+}This aligns IDs with PHP, respects the collapsible toggle, fixes smoothScroll off behavior, and hardens HTML injection.
Also applies to: 63-74, 94-151, 220-244
🤖 Prompt for AI Agents
In src/blocks/TableOfContents/view.js around lines 15-17 (and also apply similar
fixes to ranges 63-74, 94-151, 220-244): the code forces collapsible behavior on
small screens, ignores smoothScroll when explicitly false, assigns IDs after
list generation, and injects unescaped HTML; fix by (1) respect the collapsible
flag by combining the class check with the user setting rather than forcing true
for window.innerWidth <= 768 (only enable collapsible when the class/data flag
is set, optionally add responsive behavior as a separate feature), (2) change
smoothScroll logic so smoothScroll is true only when the class/data attribute
explicitly indicates true (do not default to true), (3) compute and assign
stable, unique IDs for headings before building the TOC list using the same
slugging/uniqueness algorithm as the PHP backend so IDs match, (4) escape
heading text and attributes before injecting into innerHTML or, better, create
DOM nodes/textContent to avoid HTML injection, and (5) expand the heading
counter logic to include H1–H6; apply these same corrections to the other
indicated line ranges.
Summary by CodeRabbit
New Features
Refactor
Chores