enhance: image in quick search result #237
Conversation
WalkthroughAdds a new Quick Search Gutenberg block (editor, save, server render, templates, AJAX handler, frontend init), introduces a modular docs/settings data store, updates multiple built asset hashes and bundle runtimes, and adds a Node version file (.nvmrc). Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 20
🧹 Nitpick comments (20)
wedocs.php (2)
177-195: Harden glob result and replace placeholder @SInCE
- glob can return false; foreach on false emits warnings. Cast to array.
- Replace "@SInCE WEDOCS_SINCE" with the actual version before release.
Apply this diff:
- * @since WEDOCS_SINCE + * @since 2.1.13 @@ - if ( is_dir( $blocks_dir ) ) { - $block_dirs = glob( $blocks_dir . '*', GLOB_ONLYDIR ); - foreach ( $block_dirs as $block_dir ) { + if ( is_dir( $blocks_dir ) ) { + $block_dirs = glob( $blocks_dir . '*', GLOB_ONLYDIR ) ?: []; + foreach ( $block_dirs as $block_dir ) { $render_file = $block_dir . '/render.php'; if ( file_exists( $render_file ) ) { require_once $render_file; } } }
206-212: Avoid duplicating render wiring (block.json "render" vs PHP render_callback)You register the block from metadata and also override render via render_callback while block.json already has "render": "file:./render.php". Pick one to avoid confusion:
- Prefer register_block_type with just the path and let block.json "render" load the callback, or
- Remove the "render" entry from block.json and keep render_callback here.
src/blocks/QuickSearch/block.json (1)
13-13: Single source of truth for server renderThis manifest declares "render": "file:./render.php" while PHP also sets render_callback. Use only one render wiring path to prevent drift. If you keep the PHP render_callback, remove this line here (and in the built manifest). If you keep this, drop the PHP override.
src/blocks/QuickSearch/templates/search-item.php (2)
27-30: Use anchor instead of onclick for accessibility and securityClickable div with inline JS is not keyboard-accessible and misses rel noopener. Use an with href, target, and rel.
Apply this diff:
-<div class="flex items-start p-3 rounded-lg cursor-pointer transition-colors" - style="background-color: <?php echo esc_attr( $modal_styles['listItemBackgroundColor'] ?? '#F9FAFB' ); ?>;" - onclick="window.open('<?php echo esc_url( $permalink ); ?>', '_blank')"> +<a href="<?php echo esc_url( $permalink ); ?>" target="_blank" rel="noopener" + class="flex items-start p-3 rounded-lg cursor-pointer transition-colors" + style="background-color: <?php echo esc_attr( $modal_styles['listItemBackgroundColor'] ?? '#F9FAFB' ); ?>;"> @@ -</div> +</a>Also applies to: 50-50
44-46: Be cautious appending alpha suffix to hex colorUsing 20 assumes a 7-char hex. If a non-hex color is passed, CSS will be invalid. Consider computing rgba() server-side.
includes/Ajax.php (4)
270-279: Gracefully handle invalid JSON for modal_stylesjson_decode can fail; ensure array fallback covers null and errors. Current code works; optionally check json_last_error for diagnostics.
286-292: Tighten WP_Query for performanceAdd no_found_rows and ignore_sticky_posts for a faster, non-paginated search; optionally set orderby to relevance.
Apply this diff:
$args = [ 'post_type' => 'docs', 'posts_per_page' => $per_page, 's' => $query, 'post_status' => 'publish', + 'no_found_rows' => true, + 'ignore_sticky_posts' => true, + 'orderby' => 'relevance', ];
326-331: Consider returning meta alongside HTML consistentlyYou already return results and query; include result_image_type to mirror inputs for consumers.
Apply this diff:
- wp_send_json_success( [ - 'html' => $html, - 'results' => $results, - 'query' => $query - ] ); + wp_send_json_success( [ + 'html' => $html, + 'results' => $results, + 'query' => $query, + 'result_image_type' => $result_image_type, + ] );
281-283: Use mb_strlen when available for multi-byte queriesTo handle non-ASCII queries properly, prefer mb_strlen if available.
Apply this diff:
- if ( empty( $query ) || strlen( $query ) < 2 ) { + $qLen = function_exists( 'mb_strlen' ) ? mb_strlen( $query ) : strlen( $query ); + if ( empty( $query ) || $qLen < 2 ) { wp_send_json_error( __( 'Query must be at least 2 characters long.', 'wedocs' ) ); }src/blocks/QuickSearch/save.js (1)
19-21: Dynamic block: consider returning null from Save.This block has a PHP render callback. Saving markup here is unnecessary and risks divergence from server output.
src/blocks/QuickSearch/templates/search-results.php (1)
75-111: Prefer anchors for navigation for a11y and security.Use instead of onclick on a div for better semantics and to avoid window.open() quirks.
src/blocks/QuickSearch/edit.js (3)
387-392: Use boolean disabled prop.JSX prefers boolean props for disabled.
- disabled="disabled" + disabled
45-63: Remove unused variable.searchBoxStyle is computed but not used.
- const searchBoxStyle = { - backgroundColor: searchBoxStyles.backgroundColor, - borderColor: searchBoxStyles.borderColor, - borderWidth: searchBoxStyles.borderWidth, - borderRadius: searchBoxStyles.borderRadius, - paddingTop: searchBoxStyles.padding.top, - paddingRight: searchBoxStyles.padding.right, - paddingBottom: searchBoxStyles.padding.bottom, - paddingLeft: searchBoxStyles.padding.left, - marginTop: searchBoxStyles.margin.top, - marginRight: searchBoxStyles.margin.right, - marginBottom: searchBoxStyles.margin.bottom, - marginLeft: searchBoxStyles.margin.left, - fontSize: searchBoxStyles.fontSize, - fontWeight: searchBoxStyles.fontWeight, - letterSpacing: searchBoxStyles.letterSpacing, - lineHeight: searchBoxStyles.lineHeight, - color: searchBoxStyles.placeholderColor, - };
266-334: Expose “Article Label Color” to match template.Template uses modal_styles['articleLabelColor']; add a control mirroring Doc/Section colors.
assets/build/blocks/QuickSearch/render.php (1)
610-624: Respect configured list item background in selection reset.Hard-coded '#F9FAFB' should use the configured modal style or a CSS var to avoid visual mismatch.
assets/build/index.css (2)
1908-1932: Duplicate transition-property declarations in .badge (benign, build artifact).These three transition-property lines are duplicates from PostCSS/autoprefixing. Prefer excluding build artifacts from linters or de‑dup at source if possible.
Consider ignoring assets/build/**/*.css in your CSS linter config to avoid noise from generated code.
6333-6335: Minified vendor CSS triggers parser errors in linters.The appended react-responsive-carousel CSS is minified and includes patterns (duplicate properties, shorthand overrides) that static analyzers flag. Keep as-is, but exclude vendor/minified CSS from linting.
Proposed linter ignore: assets/build/index.css or a rule to skip “react-responsive-carousel/lib/styles/carousel.min.css” content.
src/blocks/QuickSearch/style.scss (1)
1-3: Avoid re‑including Tailwind layers per block.Including @tailwind base/components/utilities here risks duplicate CSS in the final bundle. Prefer a single global entry for Tailwind layers and keep this file block‑scoped rules only.
assets/build/store.js (2)
321-331: Declaration inside switch case without block (lint: noSwitchDeclarations).Wrap the case body in braces to avoid leakage across cases.
Apply in source (src/data/docs/reducer.js):
- 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 setDocState = { + ...state, + docs: [ ...(state.docs || []), action.doc ], + }; + const parents = Array.isArray(state.parents) ? state.parents : []; + const isNotInParent = !parents.some(parent => parent?.id === action?.doc?.id); + if (!action.doc.parent && isNotInParent) { + setDocState.parents = [ { ...action.doc }, ...parents ]; + } + return setDocState; + }
372-377: Unsafe optional chaining in array spreads.Spreading the result of state.docs?.filter(...) can throw if it’s undefined. Default to [] or drop optional chaining given DEFAULT_STATE initializes arrays.
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': + return { + ...state, + docs: (state.docs || []).filter(doc => doc.id !== action.docId), + parents: (state.parents || []).filter(parent => parent.id !== action.docId), + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (10)
assets/build/block.js.mapis excluded by!**/*.mapassets/build/frontend.css.mapis excluded by!**/*.mapassets/build/frontend.js.mapis excluded by!**/*.mapassets/build/index.css.mapis excluded by!**/*.mapassets/build/index.js.mapis excluded by!**/*.mapassets/build/print.css.mapis excluded by!**/*.mapassets/build/print.js.mapis excluded by!**/*.mapassets/build/store.js.mapis excluded by!**/*.mapassets/build/style-block.css.mapis excluded by!**/*.mappackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (28)
.nvmrc(1 hunks)assets/build/block.asset.php(1 hunks)assets/build/blocks/QuickSearch/block.json(1 hunks)assets/build/blocks/QuickSearch/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(42 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)assets/js/frontend.js(1 hunks)includes/Ajax.php(2 hunks)src/blocks/QuickSearch/attributes.js(1 hunks)src/blocks/QuickSearch/block.json(1 hunks)src/blocks/QuickSearch/edit.js(1 hunks)src/blocks/QuickSearch/index.js(1 hunks)src/blocks/QuickSearch/render.php(1 hunks)src/blocks/QuickSearch/save.js(1 hunks)src/blocks/QuickSearch/style.scss(1 hunks)src/blocks/QuickSearch/templates/empty-state.php(1 hunks)src/blocks/QuickSearch/templates/search-item.php(1 hunks)src/blocks/QuickSearch/templates/search-results.php(1 hunks)src/blocks/index.js(1 hunks)wedocs.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
includes/Ajax.php (1)
wedocs.php (1)
template_path(317-319)
src/blocks/QuickSearch/attributes.js (2)
src/blocks/QuickSearch/edit.js (1)
attributes(17-27)src/blocks/QuickSearch/save.js (1)
attributes(4-17)
src/blocks/QuickSearch/render.php (3)
src/blocks/QuickSearch/attributes.js (1)
attributes(3-89)wedocs.php (1)
template_path(317-319)includes/Frontend.php (2)
Frontend(10-179)enqueue_assets(113-118)
src/blocks/QuickSearch/save.js (2)
src/blocks/QuickSearch/attributes.js (1)
attributes(3-89)src/blocks/QuickSearch/edit.js (3)
attributes(17-27)blockProps(15-15)searchBoxStyle(45-63)
src/blocks/QuickSearch/index.js (2)
src/blocks/QuickSearch/save.js (1)
Save(3-99)src/blocks/QuickSearch/edit.js (1)
Edit(14-436)
assets/build/store.js (11)
src/components/Documentations/index.js (3)
docs(19-22)loading(29-32)parentDocs(24-27)src/components/DocListing/index.js (3)
docs(27-29)loading(37-40)sections(103-103)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)assets/js/frontend.js (2)
data(166-169)article(74-74)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/QuickSearch/edit.js (2)
src/blocks/QuickSearch/save.js (3)
blockProps(23-25)attributes(4-17)searchBoxStyle(27-47)src/blocks/QuickSearch/attributes.js (1)
attributes(3-89)
🪛 Biome (2.1.2)
assets/build/index.css
[error] 1913-1913: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
transition-property is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 2259-2259: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
grid-template-columns is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 6335-6335: Unexpected value or character.
Expected one of:
(parse)
[error] 6335-6335: Unexpected value or character.
Expected one of:
(parse)
[error] 6335-6335: Unexpected value or character.
Expected one of:
(parse)
[error] 6335-6335: Unexpected value or character.
Expected one of:
(parse)
[error] 6335-6335: Unexpected value or character.
Expected one of:
(parse)
[error] 6335-6335: Unexpected shorthand property flex-flow after flex-direction
(lint/suspicious/noShorthandPropertyOverrides)
[error] 6335-6335: 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] 6335-6335: 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] 6335-6335: 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)
🔇 Additional comments (14)
wedocs.php (1)
170-175: Init order for loader vs. registration is correctHooking load_block_render_files before register_blocks ensures callbacks are defined. Looks good.
includes/Ajax.php (1)
43-46: QuickSearch endpoints registered correctlyPublic and authenticated AJAX hooks are set up with consistent action names. Good.
assets/build/index.asset.php (1)
1-1: Version hash update is fine.Rebuilt bundle now references the new checksum; no issues spotted.
assets/build/print.asset.php (1)
1-1: Print asset version bump checked.Hash refresh aligns with regenerated print artifacts.
assets/build/frontend.asset.php (1)
1-1: Frontend asset metadata looks good.Only the version changed, matching the rebuilt bundle.
assets/build/frontend.css (1)
1-3: Generated header and sourcemap comment are expected.No functional CSS changes; metadata additions match the new build pipeline.
Also applies to: 1223-1224
assets/build/print.css (1)
1-3: Print CSS metadata updates verified.Added loader banner and source map comment without touching rules—looks correct.
Also applies to: 185-186
assets/build/block.asset.php (1)
1-1: Newwp-datadependency acknowledged.Makes sense with the Quick Search block relying on the data store.
assets/build/print.js (1)
1-28: Webpack bootstrap addition is expected.Runtime wrapper and ES module flagging are consistent with the regenerated bundle.
assets/build/frontend.js (1)
1-28: Frontend bundle bootstrap looks good.Matches the updated build tooling and keeps the CSS extraction intact.
assets/build/store.asset.php (1)
1-1: LGTM: asset version bump.Dependency list unchanged; version hash update aligns with new build.
src/blocks/index.js (1)
3-3: LGTM: QuickSearch block registered via side‑effect import.Importing './QuickSearch' ensures block registration during module init.
src/blocks/QuickSearch/style.scss (1)
5-8: LGTM: editor panel spacing tweak.Scoped, minimal, and safe for the inspector UI.
src/blocks/QuickSearch/index.js (1)
9-27: LGTM: block registration details.Title, icon, attributes, and i18n usage look correct. Category “widgets” is fine; verify it fits your block categorization.
| @@ -0,0 +1 @@ | |||
| 18 | |||
There was a problem hiding this comment.
Update Node baseline to a still-supported LTS
.nvmrc now pins Node 18, which reached end-of-life on April 30 2025 and no longer receives security fixes. Please move to an actively supported LTS (e.g., Node 20) before merging so local builds don’t rely on an unpatched runtime.
Apply this diff to bump the pinned version:
-18
+20📝 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.
| 18 | |
| 20 |
🤖 Prompt for AI Agents
In .nvmrc around lines 1 to 1 the file pins Node to 18 (which is EOL); update
the pinned Node version to a currently supported LTS such as 20 by replacing the
existing version string with "20" (or "v20" depending on your repo convention)
so local developers and CI use an actively supported runtime; commit the change
and run a quick local build/test to verify compatibility.
| "helpfulDocsCount": { | ||
| "type": "number", | ||
| "default": 10 | ||
| }, |
There was a problem hiding this comment.
Schema missing attributes used elsewhere (breaks featured/icon/none and styling).
Add:
- resultImageType (string)
- modalStyles.listItemBackgroundColor
- modalStyles.listItemIconBackgroundColor
- modalStyles.articleLabelColor
"helpfulDocsCount": {
"type": "number",
"default": 10
},
+ "resultImageType": {
+ "type": "string",
+ "default": "icon"
+ },
@@
"modalStyles": {
"type": "object",
"default": {
"placeholderColor": "#9CA3AF",
"searchIconColor": "#6B7280",
"backgroundColor": "#FFFFFF",
"fieldTextColor": "#111827",
"listItemIconColor": "#3B82F6",
"listItemTextColor": "#111827",
"docLabelColor": "#3B82F6",
"sectionLabelColor": "#3B82F6",
+ "articleLabelColor": "#8B5CF6",
"listItemPadding": {
"top": "12px",
"right": "16px",
"bottom": "12px",
"left": "16px"
},
"listItemMargin": {
"top": "0px",
"right": "0px",
"bottom": "0px",
"left": "0px"
},
"listItemBorderColor": "#E5E7EB",
"listItemBorderWidth": "1px",
- "listItemBorderRadius": "4px"
+ "listItemBorderRadius": "4px",
+ "listItemBackgroundColor": "#F9FAFB",
+ "listItemIconBackgroundColor": "transparent"
}
}Also applies to: 67-95
🤖 Prompt for AI Agents
In assets/build/blocks/QuickSearch/block.json around lines 35-38 (and similarly
for the schema block at lines 67-95), the JSON schema is missing attributes that
other code expects — add a string property "resultImageType" and under
"modalStyles" add "listItemBackgroundColor", "listItemIconBackgroundColor", and
"articleLabelColor" (all strings, with sensible defaults matching existing theme
tokens) so featured/icon/none rendering and styling work; update both schema
sections to include these properties and default values consistent with the rest
of the block schema.
| $helpful_docs_count = $attributes['helpfulDocsCount'] ?? 10; | ||
|
|
There was a problem hiding this comment.
Pass result image type through to AJAX/templates.
Currently not sent; templates default to icon. Wire attribute -> data attr -> AJAX.
- $helpful_docs_count = $attributes['helpfulDocsCount'] ?? 10;
+ $helpful_docs_count = $attributes['helpfulDocsCount'] ?? 10;
+ $result_image_type = $attributes['resultImageType'] ?? 'icon';
@@
$data_attributes = [
'data-placeholder="' . esc_attr( $search_box_placeholder ) . '"',
'data-modal-placeholder="' . esc_attr( $modal_placeholder ) . '"',
'data-modal-docs-source="' . esc_attr( $modal_docs_source ) . '"',
'data-section-ids="' . esc_attr( $section_ids ) . '"',
'data-article-ids="' . esc_attr( $article_ids ) . '"',
'data-helpful-docs-count="' . esc_attr( $helpful_docs_count ) . '"',
+ 'data-result-image-type="' . esc_attr( $result_image_type ) . '"',
];
@@
const requestData = {
action: 'wedocs_quick_search',
query: query,
per_page: '10',
format: 'html',
modal_styles: JSON.stringify(modalStyles),
+ result_image_type: trigger.getAttribute('data-result-image-type') || 'icon',
_wpnonce: weDocs_Vars.nonce
};Also applies to: 210-218, 552-559
🤖 Prompt for AI Agents
In assets/build/blocks/QuickSearch/render.php around lines 146-147 (also apply
same change to lines 210-218 and 552-559): the attribute for result image type
is not being passed through to the template/AJAX, so templates fall back to the
default 'icon'; add reading of the result image type from $attributes (e.g.
$result_image_type = $attributes['resultImageType'] ?? 'icon') and ensure it is
injected into the block markup as a data attribute (data-result-image-type) so
that it is available to the front-end and forwarded in the AJAX payload/template
rendering; repeat this pattern for the other referenced line ranges to
consistently wire the attribute through.
| <script> | ||
| (function() { | ||
| const trigger = document.querySelector('.wedocs-quick-search-trigger'); | ||
|
|
||
| if (!trigger) return; | ||
|
|
||
| // Create modal dynamically | ||
| const modal = document.createElement('div'); | ||
| modal.className = 'wedocs-quick-search-modal wedocs-document'; | ||
| modal.setAttribute('role', 'dialog'); |
There was a problem hiding this comment.
Only the first block instance is initialized.
document.querySelector selects a single trigger; pages with multiple Quick Search blocks won’t work as expected. Iterate over all triggers and create per-instance modals.
🤖 Prompt for AI Agents
In assets/build/blocks/QuickSearch/render.php around lines 343 to 352, the
script uses document.querySelector which only initializes the first
.wedocs-quick-search-trigger; update it to select all triggers
(document.querySelectorAll) and iterate over them, creating a separate modal
element and wiring up event listeners for each instance so every block on the
page gets its own modal; ensure per-instance attributes (ARIA ids/classes) or
avoid duplicate IDs to prevent collisions and clean up any references scoped to
the loop.
| const Save = ({ attributes }) => { | ||
| const { | ||
| hideBlock, | ||
| searchBoxPlaceholder, | ||
| modalPlaceholder, | ||
| modalDocsSource, | ||
| sectionIds, | ||
| articleIds, | ||
| helpfulDocsCount, | ||
| showIconInResults, | ||
| customCssClass, | ||
| searchBoxStyles, | ||
| modalStyles, | ||
| additionalCssClass, | ||
| } = attributes; |
There was a problem hiding this comment.
Wire result image type (fix mismatch) and avoid undefined class names.
- The UI exposes resultImageType (see Edit) but Save uses showIconInResults and emits data-show-icon-in-results. This breaks the “featured/icon/none” feature.
- Also avoid "undefined" in class list when customCssClass/additionalCssClass are not set.
Apply:
-const Save = ({ attributes }) => {
+const Save = ({ attributes }) => {
const {
hideBlock,
searchBoxPlaceholder,
modalPlaceholder,
modalDocsSource,
sectionIds,
articleIds,
helpfulDocsCount,
- showIconInResults,
+ resultImageType,
customCssClass,
searchBoxStyles,
modalStyles,
additionalCssClass,
} = attributes;
@@
- const blockProps = useBlockProps.save({
- className: `wedocs-quick-search-block ${customCssClass} ${additionalCssClass}`,
- });
+ const blockProps = useBlockProps.save({
+ className: ['wedocs-quick-search-block', customCssClass, additionalCssClass].filter(Boolean).join(' '),
+ });
@@
<div
className="wedocs-quick-search-trigger"
data-placeholder={searchBoxPlaceholder}
data-modal-placeholder={modalPlaceholder}
data-modal-docs-source={modalDocsSource}
data-section-ids={sectionIds}
data-article-ids={articleIds}
data-helpful-docs-count={helpfulDocsCount}
- data-show-icon-in-results={showIconInResults}
+ data-result-image-type={resultImageType}
style={modalStyle}
>Also applies to: 23-26, 72-84
🤖 Prompt for AI Agents
In src/blocks/QuickSearch/save.js around lines 3-17 (also apply same changes to
lines 23-26 and 72-84), the component reads and emits the wrong image setting
and may inject "undefined" into class lists; replace use of
showIconInResults/data-show-icon-in-results with resultImageType and emit
data-result-image-type with the attribute value (expecting
"featured"|"icon"|"none"), and build the CSS class string defensively by
defaulting customCssClass and additionalCssClass to empty strings or filtering
falsy values before joining so "undefined" never appears in the rendered class
attribute.
🧩 Analysis chain
Verify attributes exist (hideBlock/customCssClass/additionalCssClass).
These are referenced but not declared in block.json/attributes in this PR. If unsupported, remove them to avoid dead code.
🏁 Script executed:
#!/bin/bash
rg -nP -C2 '\b(hideBlock|customCssClass|additionalCssClass|showIconInResults)\b' --type=js --type=phpLength of output: 2626
🏁 Script executed:
#!/bin/bash
echo "Looking for block.json in QuickSearch block directory:"
find src/blocks/QuickSearch -maxdepth 1 -type f -name 'block.json'
echo
echo "Displaying block.json contents (if found):"
if [ -f src/blocks/QuickSearch/block.json ]; then
sed -n '1,200p' src/blocks/QuickSearch/block.json
else
echo " src/blocks/QuickSearch/block.json not found"
fi
echo
echo "Searching all JSON files for attribute keys:"
rg -nP '\b(hideBlock|customCssClass|additionalCssClass|showIconInResults)\b' -g '*.json'Length of output: 2383
Add missing attributes or remove their usage
hideBlock, showIconInResults, customCssClass, and additionalCssClass are referenced in src/blocks/QuickSearch/save.js but aren’t declared in src/blocks/QuickSearch/block.json. Either add these attributes to block.json or remove their related code.
🤖 Prompt for AI Agents
In src/blocks/QuickSearch/save.js around lines 3 to 17, the code destructures
hideBlock, showIconInResults, customCssClass, and additionalCssClass which are
not declared in src/blocks/QuickSearch/block.json; either (A) add those four
attributes to block.json with appropriate types and defaults (e.g., boolean for
hideBlock and showIconInResults, string for customCssClass and
additionalCssClass) so the save component can use them safely, or (B) remove all
references to these attributes from save.js (and any related markup/logic) to
keep the file consistent with block.json. Ensure whichever approach you choose
is applied consistently across the edit and update any unit/tests that expect
these props.
| <div class="mb-4"> | ||
| <?php echo $icon; ?> | ||
| </div> |
There was a problem hiding this comment.
Sanitize $icon output to prevent XSS.
$icon is echoed raw. If it ever contains user‑controlled HTML/SVG, this is an injection risk. Sanitize with wp_kses and an allowlist.
Apply:
- <?php if ( $icon ) : ?>
- <div class="mb-4">
- <?php echo $icon; ?>
- </div>
- <?php endif; ?>
+ <?php if ( $icon ) : ?>
+ <?php
+ // Allow only safe SVG/icon markup.
+ $allowed_svg = [
+ 'svg' => [
+ 'class' => true, 'xmlns' => true, 'width' => true, 'height' => true,
+ 'viewBox' => true, 'fill' => true, 'stroke' => true, 'stroke-width' => true,
+ 'aria-hidden' => true, 'focusable' => true, 'role' => true,
+ ],
+ 'path' => [
+ 'd' => true, 'fill' => true, 'stroke' => true, 'stroke-width' => true,
+ 'fill-rule' => true, 'clip-rule' => true,
+ ],
+ 'g' => [ 'fill' => true, 'stroke' => true, 'stroke-width' => true ],
+ 'title' => true,
+ 'circle'=> [ 'cx' => true, 'cy' => true, 'r' => true ],
+ 'rect' => [ 'width' => true, 'height' => true, 'x' => true, 'y' => true, 'rx' => true, 'ry' => true ],
+ 'polygon' => [ 'points' => true ],
+ ];
+ ?>
+ <div class="mb-4">
+ <?php echo wp_kses( $icon, $allowed_svg ); ?>
+ </div>
+ <?php endif; ?>🤖 Prompt for AI Agents
In src/blocks/QuickSearch/templates/empty-state.php around lines 21 to 23, $icon
is echoed raw which can lead to XSS if it contains user-controlled HTML/SVG;
wrap the output with wp_kses and a small allowlist for the SVG/HTML you expect
(e.g. allow svg, path, g, title, etc. and the common attributes like xmlns,
viewBox, fill, stroke, width, height, class) and then echo the sanitized string
instead of the raw $icon.
| // Get document title and permalink | ||
| $title = is_array( $doc['title'] ) ? $doc['title']['rendered'] : $doc['title']; | ||
| $permalink = $doc['permalink'] ?? '#'; |
There was a problem hiding this comment.
Guard against missing title to avoid PHP notices
Accessing $doc['title'] without isset can trigger notices. Add a guard and safe fallback.
Apply this diff:
-// Get document title and permalink
-$title = is_array( $doc['title'] ) ? $doc['title']['rendered'] : $doc['title'];
+// Get document title and permalink
+$title = '';
+if ( isset( $doc['title'] ) ) {
+ $title = is_array( $doc['title'] ) ? ( $doc['title']['rendered'] ?? '' ) : $doc['title'];
+}🤖 Prompt for AI Agents
In src/blocks/QuickSearch/templates/search-item.php around lines 22 to 24, the
code accesses $doc['title'] directly which can trigger PHP notices when missing;
update the logic to first check isset($doc['title']) (and that it's not
null/empty), then if it's an array use the ['rendered'] value if set, otherwise
use the scalar value; if no title exists fall back to an empty string or a safe
default like '(No title)'. Ensure you perform isset checks before the is_array
and ['rendered'] access so no undefined index/offset notices occur.
| $results = $results ?? []; | ||
| $modal_styles = $modal_styles ?? []; | ||
| $empty_message = $empty_message ?? __( 'No results found. Try different keywords.', 'wedocs' ); | ||
| $result_image_type = $result_image_type ?? 'icon'; | ||
|
|
There was a problem hiding this comment.
Prevent notices and XSS in title highlighting.
- $query may be undefined.
- Highlighting should operate on escaped title to avoid injecting unsanitized HTML.
$results = $results ?? [];
$modal_styles = $modal_styles ?? [];
$empty_message = $empty_message ?? __( 'No results found. Try different keywords.', 'wedocs' );
$result_image_type = $result_image_type ?? 'icon';
+// Query string may be injected by caller; sanitize and default.
+$query = isset( $query ) ? sanitize_text_field( $query ) : '';
@@
- // Highlight search terms in title
- $highlighted_title = $title;
- if ( ! empty( $query ) && strlen( $query ) >= 2 ) {
+ // Highlight search terms in title (operate on escaped text)
+ $safe_title = esc_html( $title );
+ $highlighted_title = $safe_title;
+ if ( $query !== '' && strlen( $query ) >= 2 ) {
$highlighted_title = preg_replace(
'/(' . preg_quote( $query, '/' ) . ')/i',
'<mark class="bg-yellow-200 px-1 rounded">$1</mark>',
- $title
+ $safe_title
);
}Also applies to: 48-56
🤖 Prompt for AI Agents
In src/blocks/QuickSearch/templates/search-results.php around lines 15-19 (and
similarly at lines 48-56), ensure $query is defined with a safe default (e.g.
empty string) to prevent undefined variable notices, and move title escaping
before applying any highlight/replace logic so that highlighting operates on an
already-escaped string; perform highlighting with functions that only inject
safe markup (or wrap the matched text in a safe span via esc_html on the title
and then replace the HTML-escaped matched substring with a sanitized highlight
wrapper), and avoid echoing raw/unescaped title content — use WordPress escaping
functions (esc_html/esc_attr/esc_html__ as appropriate) for output.
| // Determine document type and styling | ||
| $is_parent = $doc['parent'] === 0; | ||
| $is_section = $doc['parent'] > 0 && ! $is_parent; | ||
| $is_article = $doc['parent'] > 0 && $is_section; | ||
|
|
||
| $doc_type = 'Doc'; | ||
| $doc_type_color = $modal_styles['docLabelColor'] ?? '#3B82F6'; | ||
|
|
||
| if ( $is_section ) { | ||
| $doc_type = 'Section'; | ||
| $doc_type_color = $modal_styles['sectionLabelColor'] ?? '#10B981'; | ||
| } elseif ( $is_article ) { | ||
| $doc_type = 'Article'; | ||
| $doc_type_color = $modal_styles['articleLabelColor'] ?? '#8B5CF6'; | ||
| } |
There was a problem hiding this comment.
Fix document-type detection; current logic makes 'Article' unreachable.
$is_section and $is_article evaluate the same. Prefer explicit type if present; else fall back by parent.
- // Determine document type and styling
- $is_parent = $doc['parent'] === 0;
- $is_section = $doc['parent'] > 0 && ! $is_parent;
- $is_article = $doc['parent'] > 0 && $is_section;
-
- $doc_type = 'Doc';
- $doc_type_color = $modal_styles['docLabelColor'] ?? '#3B82F6';
-
- if ( $is_section ) {
- $doc_type = 'Section';
- $doc_type_color = $modal_styles['sectionLabelColor'] ?? '#10B981';
- } elseif ( $is_article ) {
- $doc_type = 'Article';
- $doc_type_color = $modal_styles['articleLabelColor'] ?? '#8B5CF6';
- }
+ // Determine document type and styling
+ $parent = (int) ( $doc['parent'] ?? 0 );
+ $doc_type = 'Doc';
+ $doc_type_color = $modal_styles['docLabelColor'] ?? '#3B82F6';
+ if ( ! empty( $doc['type'] ) ) {
+ if ( $doc['type'] === 'section' ) {
+ $doc_type = 'Section';
+ $doc_type_color = $modal_styles['sectionLabelColor'] ?? '#10B981';
+ } elseif ( $doc['type'] === 'article' ) {
+ $doc_type = 'Article';
+ $doc_type_color = $modal_styles['articleLabelColor'] ?? '#8B5CF6';
+ }
+ } elseif ( $parent > 0 ) {
+ $doc_type = 'Article';
+ $doc_type_color = $modal_styles['articleLabelColor'] ?? '#8B5CF6';
+ }📝 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.
| // Determine document type and styling | |
| $is_parent = $doc['parent'] === 0; | |
| $is_section = $doc['parent'] > 0 && ! $is_parent; | |
| $is_article = $doc['parent'] > 0 && $is_section; | |
| $doc_type = 'Doc'; | |
| $doc_type_color = $modal_styles['docLabelColor'] ?? '#3B82F6'; | |
| if ( $is_section ) { | |
| $doc_type = 'Section'; | |
| $doc_type_color = $modal_styles['sectionLabelColor'] ?? '#10B981'; | |
| } elseif ( $is_article ) { | |
| $doc_type = 'Article'; | |
| $doc_type_color = $modal_styles['articleLabelColor'] ?? '#8B5CF6'; | |
| } | |
| // Determine document type and styling | |
| $parent = (int) ( $doc['parent'] ?? 0 ); | |
| $doc_type = 'Doc'; | |
| $doc_type_color = $modal_styles['docLabelColor'] ?? '#3B82F6'; | |
| if ( ! empty( $doc['type'] ) ) { | |
| if ( $doc['type'] === 'section' ) { | |
| $doc_type = 'Section'; | |
| $doc_type_color = $modal_styles['sectionLabelColor'] ?? '#10B981'; | |
| } elseif ( $doc['type'] === 'article' ) { | |
| $doc_type = 'Article'; | |
| $doc_type_color = $modal_styles['articleLabelColor'] ?? '#8B5CF6'; | |
| } | |
| } elseif ( $parent > 0 ) { | |
| $doc_type = 'Article'; | |
| $doc_type_color = $modal_styles['articleLabelColor'] ?? '#8B5CF6'; | |
| } |
🤖 Prompt for AI Agents
In src/blocks/QuickSearch/templates/search-results.php around lines 28-42, the
current booleans make 'Article' unreachable because $is_section and $is_article
share the same condition; update the logic to prefer an explicit $doc['type']
when present and otherwise derive type from parent: compute $type = $doc['type']
?? null; set $is_parent = $doc['parent'] === 0; set $is_section = $type ===
'section' || ($type === null && $doc['parent'] > 0 && /* fallback rule for
section */ true); set $is_article = $type === 'article' || ($type === null &&
$doc['parent'] > 0 && /* fallback rule for article */ false) — in short, use
$doc['type'] first to set doc_type/doc_type_color (section/article/doc), and
only when type is missing fall back to parent-based logic (parent===0 => Doc,
else Section by default), ensuring Article becomes reachable when type ===
'article'.
|
Quick search block not working @sapayth vai Screen.Recording.2025-09-30.at.6.22.34.PM.mov |
|
please resolve merge conflict here @sapayth vai |
…_quick_search_result
@Rubaiyat-E-Mohammad bhai, please have a look if issue exist only in this PR |
Yes, the issue exists only in this pr @sapayth vai |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
src/blocks/QuickSearch/render.php (6)
69-75: Still missing posts_per_page limit.
get_posts()defaults to returning only 5 posts. When editors configure more than 5 section IDs, only the first five will be retrieved, silently dropping the rest from the modal.Apply this diff to retrieve all configured sections:
$sections = get_posts([ 'post_type' => 'docs', 'post__in' => $section_ids_array, 'post_status' => 'publish', 'orderby' => 'menu_order', - 'order' => 'ASC' + 'order' => 'ASC', + 'posts_per_page' => -1, ]);Based on past review comments.
95-101: Still missing posts_per_page limit.Same issue as sections: only the first 5 configured articles will be returned when editors select more than 5, leaving the overflow unreachable.
Apply this diff to retrieve all configured articles:
$articles = get_posts([ 'post_type' => 'docs', 'post__in' => $article_ids_array, 'post_status' => 'publish', 'orderby' => 'menu_order', - 'order' => 'ASC' + 'order' => 'ASC', + 'posts_per_page' => -1, ]);Based on past review comments.
217-228: Still vulnerable to "Undefined array key" notices.Direct access to nested keys like
$search_box_styles['padding']['top']will trigger PHP notices if the parentpaddingormarginarrays are missing from saved attributes. This can easily occur with older saved data, filtered attributes, or trimmed block JSON.Apply this diff to guard the nested lookups:
+ $padding = $search_box_styles['padding'] ?? []; + $margin = $search_box_styles['margin'] ?? []; + - $search_box_css_vars[] = '--wedocs-padding-top: ' . ( $search_box_styles['padding']['top'] ?? '12px' ); - $search_box_css_vars[] = '--wedocs-padding-right: ' . ( $search_box_styles['padding']['right'] ?? '16px' ); - $search_box_css_vars[] = '--wedocs-padding-bottom: ' . ( $search_box_styles['padding']['bottom'] ?? '12px' ); - $search_box_css_vars[] = '--wedocs-padding-left: ' . ( $search_box_styles['padding']['left'] ?? '16px' ); - $search_box_css_vars[] = '--wedocs-margin-top: ' . ( $search_box_styles['margin']['top'] ?? '0px' ); - $search_box_css_vars[] = '--wedocs-margin-right: ' . ( $search_box_styles['margin']['right'] ?? '0px' ); - $search_box_css_vars[] = '--wedocs-margin-bottom: ' . ( $search_box_styles['margin']['bottom'] ?? '0px' ); - $search_box_css_vars[] = '--wedocs-margin-left: ' . ( $search_box_styles['margin']['left'] ?? '0px' ); + $search_box_css_vars[] = '--wedocs-padding-top: ' . ( $padding['top'] ?? '12px' ); + $search_box_css_vars[] = '--wedocs-padding-right: ' . ( $padding['right'] ?? '16px' ); + $search_box_css_vars[] = '--wedocs-padding-bottom: ' . ( $padding['bottom'] ?? '12px' ); + $search_box_css_vars[] = '--wedocs-padding-left: ' . ( $padding['left'] ?? '16px' ); + $search_box_css_vars[] = '--wedocs-margin-top: ' . ( $margin['top'] ?? '0px' ); + $search_box_css_vars[] = '--wedocs-margin-right: ' . ( $margin['right'] ?? '0px' ); + $search_box_css_vars[] = '--wedocs-margin-bottom: ' . ( $margin['bottom'] ?? '0px' ); + $search_box_css_vars[] = '--wedocs-margin-left: ' . ( $margin['left'] ?? '0px' );Based on past review comments.
242-254: Still vulnerable to "Undefined array key" notices in modal styles.Same nested-key issue affects
$modal_styles['listItemPadding']['top'],listItemMargin, andlistItemBorderlookups. Guard these the same way as the search box styles.Apply this diff to guard the modal style lookups:
+ $listItemPadding = $modal_styles['listItemPadding'] ?? []; + $listItemMargin = $modal_styles['listItemMargin'] ?? []; + - $modal_css_vars[] = '--wedocs-modal-list-item-padding-top: ' . ( $modal_styles['listItemPadding']['top'] ?? '12px' ); - $modal_css_vars[] = '--wedocs-modal-list-item-padding-right: ' . ( $modal_styles['listItemPadding']['right'] ?? '16px' ); - $modal_css_vars[] = '--wedocs-modal-list-item-padding-bottom: ' . ( $modal_styles['listItemPadding']['bottom'] ?? '12px' ); - $modal_css_vars[] = '--wedocs-modal-list-item-padding-left: ' . ( $modal_styles['listItemPadding']['left'] ?? '16px' ); - $modal_css_vars[] = '--wedocs-modal-list-item-margin-top: ' . ( $modal_styles['listItemMargin']['top'] ?? '0px' ); - $modal_css_vars[] = '--wedocs-modal-list-item-margin-right: ' . ( $modal_styles['listItemMargin']['right'] ?? '0px' ); - $modal_css_vars[] = '--wedocs-modal-list-item-margin-bottom: ' . ( $modal_styles['listItemMargin']['bottom'] ?? '0px' ); - $modal_css_vars[] = '--wedocs-modal-list-item-margin-left: ' . ( $modal_styles['listItemMargin']['left'] ?? '0px' ); + $modal_css_vars[] = '--wedocs-modal-list-item-padding-top: ' . ( $listItemPadding['top'] ?? '12px' ); + $modal_css_vars[] = '--wedocs-modal-list-item-padding-right: ' . ( $listItemPadding['right'] ?? '16px' ); + $modal_css_vars[] = '--wedocs-modal-list-item-padding-bottom: ' . ( $listItemPadding['bottom'] ?? '12px' ); + $modal_css_vars[] = '--wedocs-modal-list-item-padding-left: ' . ( $listItemPadding['left'] ?? '16px' ); + $modal_css_vars[] = '--wedocs-modal-list-item-margin-top: ' . ( $listItemMargin['top'] ?? '0px' ); + $modal_css_vars[] = '--wedocs-modal-list-item-margin-right: ' . ( $listItemMargin['right'] ?? '0px' ); + $modal_css_vars[] = '--wedocs-modal-list-item-margin-bottom: ' . ( $listItemMargin['bottom'] ?? '0px' ); + $modal_css_vars[] = '--wedocs-modal-list-item-margin-left: ' . ( $listItemMargin['left'] ?? '0px' );Based on past review comments.
397-402: Multiple block instances will still conflict.
document.querySelector('.wedocs-quick-search-block .wedocs-quick-search-trigger')only ever returns the first matching trigger. On pages with multiple Quick Search blocks, every inline script will bind to that first trigger, leaving other blocks non-functional.Generate a unique wrapper ID and scope the query:
+ <?php $block_id = wp_unique_id( 'wedocs-quick-search-' ); ?> - <div class="<?php echo esc_attr( $css_classes_string ); ?>" style="<?php echo esc_attr( $css_vars_string ); ?>"> + <div id="<?php echo esc_attr( $block_id ); ?>" class="<?php echo esc_attr( $css_classes_string ); ?>" style="<?php echo esc_attr( $css_vars_string ); ?>">Then in the script:
<script> (function() { - // Use a more specific selector to avoid conflicts with other search elements - const trigger = document.querySelector('.wedocs-quick-search-block .wedocs-quick-search-trigger'); + const block = document.getElementById('<?php echo esc_js( $block_id ); ?>'); + if (!block) return; + const trigger = block.querySelector('.wedocs-quick-search-trigger');Based on past review comments.
314-317: Nested array access needs guarding.These inline style attributes access
$search_box_styles['margin']['top']etc. directly, which will trigger notices if the parent arrays are missing.After applying the guard from the earlier comment (lines 217-228), update these inline styles to use the guarded variables:
- margin-top: <?php echo esc_attr( $search_box_styles['margin']['top'] ?? '0px' ); ?>; - margin-right: <?php echo esc_attr( $search_box_styles['margin']['right'] ?? '0px' ); ?>; - margin-bottom: <?php echo esc_attr( $search_box_styles['margin']['bottom'] ?? '0px' ); ?>; - margin-left: <?php echo esc_attr( $search_box_styles['margin']['left'] ?? '0px' ); ?>; + margin-top: <?php echo esc_attr( $margin['top'] ?? '0px' ); ?>; + margin-right: <?php echo esc_attr( $margin['right'] ?? '0px' ); ?>; + margin-bottom: <?php echo esc_attr( $margin['bottom'] ?? '0px' ); ?>; + margin-left: <?php echo esc_attr( $margin['left'] ?? '0px' ); ?>;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
assets/js/frontend.js(2 hunks)src/blocks/QuickSearch/render.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/blocks/QuickSearch/render.php (3)
assets/js/frontend.js (1)
article(74-74)src/blocks/QuickSearch/attributes.js (1)
attributes(3-89)includes/Frontend.php (2)
Frontend(10-179)enqueue_assets(113-118)
🔇 Additional comments (6)
assets/js/frontend.js (2)
17-18: Previously flagged TypeError is now resolved.The
initializeQuickSearch()method now exists (lines 414-428), so this call will no longer throw a TypeError. The implementation correctly acknowledges that Quick Search blocks are self-contained.Based on past review comments.
414-428: LGTM! Clean compatibility implementation.The method provides a compatibility layer that checks for Quick Search blocks without interfering with their self-contained functionality. The comments clearly document the architectural decision.
src/blocks/QuickSearch/render.php (4)
20-44: LGTM! Helper functions are correctly implemented.The three helper functions correctly identify parent docs, sections, and articles using the hierarchical parent-child relationship. The logic is sound and consistent with the documented architecture.
118-178: LGTM! Helpful docs logic correctly implements vote-based ranking.The implementation properly:
- Retrieves all docs with
posts_per_page => -1- Calculates vote counts (positive - negative)
- Sorts by vote count descending
- Limits to the configured count
- Determines accurate doc types for the results
586-666: LGTM! Search implementation is robust.The async search function properly:
- Validates minimum query length
- Shows loading states with visual feedback
- Makes AJAX requests with proper error handling
- Renders results or appropriate empty states
- Announces results to screen readers
- Handles cleanup in the finally block
687-721: LGTM! Keyboard navigation is well-implemented.The navigation functions properly handle:
- Arrow key movement with boundary checks
- Smooth scrolling to selected items
- Visual selection highlighting
- Enter key to open results
- Fallback click behavior
| padding-top: <?php echo esc_attr( $search_box_styles['padding']['top'] ?? '12px' ); ?>; | ||
| padding-right: <?php echo esc_attr( $search_box_styles['padding']['right'] ?? '16px' ); ?>; | ||
| padding-bottom: <?php echo esc_attr( $search_box_styles['padding']['bottom'] ?? '12px' ); ?>; | ||
| padding-left: <?php echo esc_attr( $search_box_styles['padding']['left'] ?? '16px' ); ?>; |
There was a problem hiding this comment.
Nested array access needs guarding.
These inline style attributes access $search_box_styles['padding']['top'] etc. directly. Apply the same guard pattern.
After applying the guard from the earlier comment (lines 217-228), update these inline styles:
- padding-top: <?php echo esc_attr( $search_box_styles['padding']['top'] ?? '12px' ); ?>;
- padding-right: <?php echo esc_attr( $search_box_styles['padding']['right'] ?? '16px' ); ?>;
- padding-bottom: <?php echo esc_attr( $search_box_styles['padding']['bottom'] ?? '12px' ); ?>;
- padding-left: <?php echo esc_attr( $search_box_styles['padding']['left'] ?? '16px' ); ?>;
+ padding-top: <?php echo esc_attr( $padding['top'] ?? '12px' ); ?>;
+ padding-right: <?php echo esc_attr( $padding['right'] ?? '16px' ); ?>;
+ padding-bottom: <?php echo esc_attr( $padding['bottom'] ?? '12px' ); ?>;
+ padding-left: <?php echo esc_attr( $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.
| padding-top: <?php echo esc_attr( $search_box_styles['padding']['top'] ?? '12px' ); ?>; | |
| padding-right: <?php echo esc_attr( $search_box_styles['padding']['right'] ?? '16px' ); ?>; | |
| padding-bottom: <?php echo esc_attr( $search_box_styles['padding']['bottom'] ?? '12px' ); ?>; | |
| padding-left: <?php echo esc_attr( $search_box_styles['padding']['left'] ?? '16px' ); ?>; | |
| padding-top: <?php echo esc_attr( $padding['top'] ?? '12px' ); ?>; | |
| padding-right: <?php echo esc_attr( $padding['right'] ?? '16px' ); ?>; | |
| padding-bottom: <?php echo esc_attr( $padding['bottom'] ?? '12px' ); ?>; | |
| padding-left: <?php echo esc_attr( $padding['left'] ?? '16px' ); ?>; |
| const handleKeydown = (e) => { | ||
| // Only handle keyboard events when modal is active or when clicking on Quick Search trigger | ||
| if (!modal.classList.contains('active')) { | ||
| // Cmd/Ctrl + K to open modal - but only if the trigger is focused | ||
| if ((e.metaKey || e.ctrlKey) && e.key === 'k') { | ||
| // Only open if the trigger is focused to avoid conflicts with sidebar search | ||
| if (trigger.contains(document.activeElement)) { | ||
| e.preventDefault(); | ||
| openModal(); | ||
| setTimeout(focusSearchInput, 100); | ||
| } | ||
| } | ||
| return; |
There was a problem hiding this comment.
Keyboard shortcut will only work for the first block instance.
The trigger.contains(document.activeElement) check references the first block's trigger due to the selector issue flagged above. After applying the unique ID fix, this will work correctly for all instances.
This is a consequence of the previous issue. Once you apply the unique ID scoping, this code will function correctly.
🤖 Prompt for AI Agents
In src/blocks/QuickSearch/render.php around lines 772 to 784, the keyboard
shortcut check currently uses a trigger reference that points to the first block
instance, so Cmd/Ctrl+K only works for that instance; update the code to use the
instance-scoped trigger (the variable created when scoping/adding unique IDs)
instead of a global selector and ensure the keydown handler is registered
per-instance so the condition trigger.contains(document.activeElement) correctly
checks the active element within this block's trigger; no other behavior changes
are needed.
|
Image not showing in quick search block @sapayth vai |
…_quick_search_result
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
assets/build/index.css (1)
6555-6561: Third-party minified CSS: inspect source for potential issues.Line 6558 contains minified CSS from
react-responsive-carousel. Static analysis reports parse errors and duplicate property issues within this minified code. While these errors are expected in minified output, they suggest the source library may have duplication or syntax quirks. Consider:
- Verifying the carousel library version is current and stable
- Checking whether a non-minified version is available for debugging
- Confirming these warnings do not indicate functional issues in the library
wedocs.php (1)
184-195: Consider adding error handling for file operations.The method uses
glob()andrequire_oncewithout error handling. While the directory path is controlled and should be safe, adding basic checks would improve robustness and make debugging easier if block render files contain errors.Consider wrapping the require_once in a try-catch or at minimum checking
is_readable():public function load_block_render_files() { $blocks_dir = plugin_dir_path( __FILE__ ) . 'assets/build/blocks/'; if ( is_dir( $blocks_dir ) ) { $block_dirs = glob( $blocks_dir . '*', GLOB_ONLYDIR ); + if ( false === $block_dirs ) { + return; + } foreach ( $block_dirs as $block_dir ) { $render_file = $block_dir . '/render.php'; - if ( file_exists( $render_file ) ) { + if ( file_exists( $render_file ) && is_readable( $render_file ) ) { require_once $render_file; } } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
assets/build/index.css(51 hunks)wedocs.php(2 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
assets/build/index.css
[error] 1913-1913: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
transition-property is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 2316-2316: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
grid-template-columns is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 6558-6558: Unexpected value or character.
Expected one of:
(parse)
[error] 6558-6558: Unexpected value or character.
Expected one of:
(parse)
[error] 6558-6558: Unexpected value or character.
Expected one of:
(parse)
[error] 6558-6558: Unexpected value or character.
Expected one of:
(parse)
[error] 6558-6558: Unexpected value or character.
Expected one of:
(parse)
[error] 6558-6558: Unexpected shorthand property flex-flow after flex-direction
(lint/suspicious/noShorthandPropertyOverrides)
[error] 6558-6558: 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] 6558-6558: 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] 6558-6558: 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 (1)
wedocs.php (1)
206-212: LGTM!The QuickSearch block registration follows the same pattern as the existing DocsGrid block and correctly references the render callback that should be loaded via the new
load_block_render_files()method.
| .badge { | ||
| display: inline-flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| transition-property: color, background-color, border-color, text-decoration-color, fill, stroke, opacity, box-shadow, transform, filter, -webkit-backdrop-filter; | ||
| transition-property: color, background-color, border-color, text-decoration-color, fill, stroke, opacity, box-shadow, transform, filter, backdrop-filter; | ||
| transition-property: color, background-color, border-color, text-decoration-color, fill, stroke, opacity, box-shadow, transform, filter, backdrop-filter, -webkit-backdrop-filter; | ||
| transition-duration: 200ms; | ||
| transition-timing-function: cubic-bezier(0.4, 0, 0.2, 1); | ||
| height: 1.25rem; | ||
| font-size: 0.875rem; | ||
| line-height: 1.25rem; | ||
| width: -moz-fit-content; |
There was a problem hiding this comment.
Consolidate duplicate transition-property declarations.
Lines 1913 and 1914 define transition-property twice with slightly different values. The second declaration (which includes -webkit-backdrop-filter) overrides the first. Consolidate into a single property with all required vendors.
- transition-property: color, background-color, border-color, text-decoration-color, fill, stroke, opacity, box-shadow, transform, filter, backdrop-filter;
- transition-property: color, background-color, border-color, text-decoration-color, fill, stroke, opacity, box-shadow, transform, filter, backdrop-filter, -webkit-backdrop-filter;
+ transition-property: color, background-color, border-color, text-decoration-color, fill, stroke, opacity, box-shadow, transform, filter, backdrop-filter, -webkit-backdrop-filter;📝 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.
| .badge { | |
| display: inline-flex; | |
| align-items: center; | |
| justify-content: center; | |
| transition-property: color, background-color, border-color, text-decoration-color, fill, stroke, opacity, box-shadow, transform, filter, -webkit-backdrop-filter; | |
| transition-property: color, background-color, border-color, text-decoration-color, fill, stroke, opacity, box-shadow, transform, filter, backdrop-filter; | |
| transition-property: color, background-color, border-color, text-decoration-color, fill, stroke, opacity, box-shadow, transform, filter, backdrop-filter, -webkit-backdrop-filter; | |
| transition-duration: 200ms; | |
| transition-timing-function: cubic-bezier(0.4, 0, 0.2, 1); | |
| height: 1.25rem; | |
| font-size: 0.875rem; | |
| line-height: 1.25rem; | |
| width: -moz-fit-content; | |
| .badge { | |
| display: inline-flex; | |
| align-items: center; | |
| justify-content: center; | |
| transition-property: color, background-color, border-color, text-decoration-color, fill, stroke, opacity, box-shadow, transform, filter, backdrop-filter, -webkit-backdrop-filter; | |
| transition-duration: 200ms; | |
| transition-timing-function: cubic-bezier(0.4, 0, 0.2, 1); | |
| height: 1.25rem; | |
| font-size: 0.875rem; | |
| line-height: 1.25rem; | |
| width: -moz-fit-content; |
🧰 Tools
🪛 Biome (2.1.2)
[error] 1913-1913: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
transition-property 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 1908 to 1920, there are duplicate
transition-property declarations; consolidate them into a single
transition-property that includes all required tokens (color, background-color,
border-color, text-decoration-color, fill, stroke, opacity, box-shadow,
transform, filter, backdrop-filter, -webkit-backdrop-filter) and remove the
redundant lines so only one unified declaration remains.
| .steps .step { | ||
| display: grid; | ||
| grid-template-columns: repeat(1, minmax(0, 1fr)); | ||
| grid-template-columns: auto; | ||
| grid-template-rows: repeat(2, minmax(0, 1fr)); | ||
| grid-template-rows: 40px 1fr; | ||
| place-items: center; | ||
| text-align: center; | ||
| min-width: 4rem; | ||
| } |
There was a problem hiding this comment.
Remove duplicate grid-template-columns in .steps .step.
The .steps .step rule defines grid-template-columns twice (line 2315 and 2316). The second declaration (auto) overrides the first repeat(1, minmax(0, 1fr)), which likely unintended. Retain only the intended value.
.steps .step {
display: grid;
grid-template-columns: repeat(1, minmax(0, 1fr));
- grid-template-columns: auto;
grid-template-rows: repeat(2, minmax(0, 1fr));
grid-template-rows: 40px 1fr;
place-items: center;📝 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.
| .steps .step { | |
| display: grid; | |
| grid-template-columns: repeat(1, minmax(0, 1fr)); | |
| grid-template-columns: auto; | |
| grid-template-rows: repeat(2, minmax(0, 1fr)); | |
| grid-template-rows: 40px 1fr; | |
| place-items: center; | |
| text-align: center; | |
| min-width: 4rem; | |
| } | |
| .steps .step { | |
| display: grid; | |
| grid-template-columns: repeat(1, minmax(0, 1fr)); | |
| grid-template-rows: repeat(2, minmax(0, 1fr)); | |
| grid-template-rows: 40px 1fr; | |
| place-items: center; | |
| text-align: center; | |
| min-width: 4rem; | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 2316-2316: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
grid-template-columns 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 2313 to 2322, the .steps .step rule
defines grid-template-columns twice; remove the duplicate second declaration
(grid-template-columns: auto;) so the intended grid-template-columns: repeat(1,
minmax(0, 1fr)) remains and no longer gets overridden.
| add_action( 'init', [ $this, 'load_block_render_files' ] ); | ||
| add_action('init', [$this, 'register_blocks']); | ||
|
|
||
| // registeer our widget |
There was a problem hiding this comment.
Fix typo in comment.
"registeer" should be "register".
- // registeer our widget
+ // register our widget📝 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.
| // registeer our widget | |
| // register our widget |
🤖 Prompt for AI Agents
In wedocs.php around line 173, there's a typo in the inline comment: change
"registeer our widget" to "register our widget" by editing the comment text to
the correct spelling.
| /** | ||
| * Auto-load all block render files | ||
| * | ||
| * @since WEDOCS_SINCE | ||
| * | ||
| * @return void | ||
| */ | ||
| public function load_block_render_files() { | ||
| $blocks_dir = plugin_dir_path( __FILE__ ) . 'assets/build/blocks/'; | ||
| if ( is_dir( $blocks_dir ) ) { | ||
| $block_dirs = glob( $blocks_dir . '*', GLOB_ONLYDIR ); | ||
| foreach ( $block_dirs as $block_dir ) { | ||
| $render_file = $block_dir . '/render.php'; | ||
| if ( file_exists( $render_file ) ) { | ||
| require_once $render_file; | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Replace placeholder in @SInCE tag.
The @since tag contains a placeholder WEDOCS_SINCE that should be replaced with the actual version number (likely 2.1.14 or the next release version).
- * @since WEDOCS_SINCE
+ * @since 2.1.14📝 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.
| /** | |
| * Auto-load all block render files | |
| * | |
| * @since WEDOCS_SINCE | |
| * | |
| * @return void | |
| */ | |
| public function load_block_render_files() { | |
| $blocks_dir = plugin_dir_path( __FILE__ ) . 'assets/build/blocks/'; | |
| if ( is_dir( $blocks_dir ) ) { | |
| $block_dirs = glob( $blocks_dir . '*', GLOB_ONLYDIR ); | |
| foreach ( $block_dirs as $block_dir ) { | |
| $render_file = $block_dir . '/render.php'; | |
| if ( file_exists( $render_file ) ) { | |
| require_once $render_file; | |
| } | |
| } | |
| } | |
| } | |
| /** | |
| * Auto-load all block render files | |
| * | |
| * @since 2.1.14 | |
| * | |
| * @return void | |
| */ | |
| public function load_block_render_files() { | |
| $blocks_dir = plugin_dir_path( __FILE__ ) . 'assets/build/blocks/'; | |
| if ( is_dir( $blocks_dir ) ) { | |
| $block_dirs = glob( $blocks_dir . '*', GLOB_ONLYDIR ); | |
| foreach ( $block_dirs as $block_dir ) { | |
| $render_file = $block_dir . '/render.php'; | |
| if ( file_exists( $render_file ) ) { | |
| require_once $render_file; | |
| } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In wedocs.php around lines 177 to 195, the DocBlock @since tag uses the
placeholder WEDOCS_SINCE; replace that placeholder with the actual release
version (e.g., 2.1.14 or the correct upcoming release number) so the DocBlock
reads @since 2.1.14 (or the correct version) for the load_block_render_files()
method.
2 participants
fixes #190
show featured image/icon or none on quick search result
Summary by CodeRabbit
New Features
Style
Chores