Feature/search form modal integration #239
Conversation
- Convert doc-search-form.php from form submission to modal-based search - Remove Button Style and Button Size controls from PrintButton block - Fix JavaScript null reference errors in frontend.js - Enable search modal functionality on all pages (not just single docs) - Add defensive programming to prevent querySelector errors - Maintain original design while enabling quick search functionality - Fix CSS positioning to keep search icon on right side Changes: - templates/doc-search-form.php: Added modal integration with CSS fixes - assets/js/frontend.js: Fixed null reference errors and enabled modal on all pages - includes/Frontend.php: Set isSingleDoc to true for universal modal access - PrintButton block: Removed buttonStyle and buttonSize attributes and controls
WalkthroughEnables and guards the single-document search modal across the app, adds null-safe checks and error handling in frontend JS, adjusts PHP to always set isSingleDoc true, and updates the search form template with styling, a readonly input, and a button-based trigger. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
assets/js/frontend.js (2)
223-228: Potential runtime error: missing.search-cleanelement.
[0]on an empty jQuery set yields undefined;undefined.stylewill throw. Also treat whitespace as blank.Apply:
- $( e.target ).next( '.search-clean' )[0].style.display = `${ searchValue ? 'block' : 'none' }`; + const $clean = $( e.target ).next( '.search-clean' ); + const hasValue = !!( searchValue && searchValue.trim().length ); + if ( $clean.length ) { + $clean[0].style.display = hasValue ? 'block' : 'none'; + }
231-254: Escape dynamic content to avoid XSS; prefer permalink over guid.HTML is constructed with unescaped titles/URLs and
guid(not reliable). Escape text and usepermalinkconsistently.Apply:
- const ulNode = document.createElement( 'ul' ); + const ulNode = document.createElement( 'ul' ); + const esc = (s) => String(s ?? '').replace(/[&<>"']/g, (m) => ({'&':'&','<':'<','>':'>','"':'"',"'":'''}[m])); ... - let url = data?.article ? data?.article?.permalink : + let url = data?.article ? data?.article?.permalink : ( data?.section ? data?.section?.permalink : data?.parent?.permalink ), title = data?.article ? data?.article?.post_title : ( data?.section ? data?.section?.post_title : data?.parent?.post_title ), parentNavigation = data?.section ? `<div class='parent-doc-nav'> ${ weDocs_Vars.docNavLabel } - <span data-url="${ data?.parent?.guid }" class="doc-search-hit-path"> - ${ this.extractedTitle( data?.parent?.post_title, 35 ) } + <span data-url="${ esc( data?.parent?.permalink ) }" class="doc-search-hit-path"> + ${ esc( this.extractedTitle( data?.parent?.post_title, 35 ) ) } </span> </div>` : '', sectionNavigation = data?.article ? `<div class='section-doc-nav'> ${ weDocs_Vars.sectionNavLabel } - <span data-url="${ data?.section?.guid }" class="doc-search-hit-path"> - ${ this.extractedTitle( data?.section?.post_title, 35 ) } + <span data-url="${ esc( data?.section?.permalink ) }" class="doc-search-hit-path"> + ${ esc( this.extractedTitle( data?.section?.post_title, 35 ) ) } </span> </div>` : ''; ... - title = this.extractedTitle( title ); + title = this.extractedTitle( title ); ... - href="${ url }" + href="${ esc( url ) }" ... - <div class="doc-search-hit-title">${ title }</div> + <div class="doc-search-hit-title">${ esc( title ) }</div>Optional: build the DOM via
createElement/textContentto avoid string concatenation entirely.Also applies to: 265-291, 294-299
includes/Frontend.php (1)
83-90:isSingleDocis hard-coded true but scripts still enqueue only on single docs.JS now initializes the modal everywhere, but
wedocs-scriptsis only enqueued inenqueue_single_scripts()foris_singular('docs'). This likely prevents the new search from working on the docs home/archive.Apply:
- public function enqueue_single_scripts() { - if ( is_singular( 'docs' ) ) { + public function enqueue_single_scripts() { + if ( is_singular( 'docs' ) || is_post_type_archive( 'docs' ) ) { self::enqueue_assets(); } }If the search form can appear elsewhere, consider enqueuing when the form/template is rendered (action hook) or enqueue globally and lazy‑mount.
🧹 Nitpick comments (5)
templates/doc-search-form.php (1)
1-10: Move inline styles to the stylesheet (and avoid!important).Inline
<style>in a template complicates overrides and caching. Move these rules intoassets/build/frontend.css(or relevant stylesheet) and drop!importantby increasing selector specificity.assets/js/frontend.js (3)
169-179: Prefersuccessovercompletefor parsing JSON; set docs only on success.Avoid reading
responseJSONon failures. Use the success callback and checkresp.data.Apply:
- $.ajax({ - data, - url : weDocs_Vars.ajaxurl, - type : 'POST', - error : ( e ) => console.log( e ), - complete : ( response ) => { - if ( response?.responseJSON?.data ) { - this.docs = response?.responseJSON?.data; - } - }, - }); + $.ajax({ + data, + url : weDocs_Vars.ajaxurl, + type : 'POST', + success : ( resp ) => { + if ( resp?.data ) { + this.docs = resp.data; + } + }, + error : ( e ) => console.log( e ), + });
189-199: Prevent default browser shortcut on Ctrl/Cmd+K.Some browsers use Ctrl/Cmd+K for search; prevent default to reliably open the modal.
Apply:
- if ( ( event.ctrlKey || event.metaKey ) && event.key === 'k' ) { - this.showSinglePageSearchModal(); + if ( ( event.ctrlKey || event.metaKey ) && event.key === 'k' ) { + event.preventDefault(); + this.showSinglePageSearchModal();
206-211: Selector too strict; may miss non-text inputs.Use a generic input selector and also trigger input event after clearing.
Apply:
- $( this ).hide(); - $( this ).prev( 'input[type="text"]' ).val( null ).focus(); + $( this ).hide(); + $( this ).prev( 'input' ).val( '' ).trigger( 'input' ).focus();includes/Frontend.php (1)
78-90: Consider removingisSingleDocif unused, or make it accurate.With the JS gate removed,
isSingleDocmay be dead weight. Either drop it or set it to the actual condition to prevent confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
assets/js/frontend.js(7 hunks)includes/Frontend.php(1 hunks)templates/doc-search-form.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
assets/js/frontend.js (1)
src/utils/menuFix.js (1)
$(2-2)
🔇 Additional comments (3)
assets/js/frontend.js (2)
181-187: LGTM: Guarded modal mount.Mounting only when
weDocs_Vars.searchModalexists avoids DOM pollution and errors.
440-444: LGTM: Auto-expand current ancestor.Null-safe check and class toggling look correct.
includes/Frontend.php (1)
65-70: Versioning: use built asset’s mtime forwedocs-scripts(if applicable).If the script is bundled to
assets/build/frontend.js, prefer that path for cache-busting consistency; otherwise ignore.Can you confirm whether
assets/js/frontend.jsis the served file in production? If not, we should switch the handle URL and version to the built asset.
| <form role='search' method='get' class='search-form wedocs-search-form wedocs-single-search-input' action='<?php echo esc_url( home_url( '/' ) ) ?>'> | ||
| <div class='wedocs-search-input'> | ||
| <input | ||
| name='s' | ||
| readonly | ||
| type='search' | ||
| class='search-field' | ||
| value='<?php get_search_query(); ?>' | ||
| title='<?php echo esc_attr_x( 'Search for:', 'label', 'wedocs' ); ?>' | ||
| placeholder='<?php echo esc_attr_x( 'Search for a topic or question', 'placeholder', 'wedocs' ); ?>' | ||
| /> | ||
| <input type='hidden' name='post_type' value='docs' /> | ||
| <button type='submit' class='search-submit'> | ||
| <button type='button' class='search-submit'> | ||
| <svg width='15' height='16' fill='none'> | ||
| <path fill-rule='evenodd' d='M11.856 10.847l2.883 2.883a.89.89 0 0 1 0 1.257c-.173.174-.401.261-.629.261s-.455-.087-.629-.261l-2.883-2.883c-1.144.874-2.532 1.353-3.996 1.353a6.56 6.56 0 0 1-4.671-1.935c-2.576-2.575-2.576-6.765 0-9.341C3.179.934 4.839.247 6.603.247s3.424.687 4.671 1.935a6.56 6.56 0 0 1 1.935 4.67 6.55 6.55 0 0 1-1.353 3.995zM3.189 3.439c-1.882 1.882-1.882 4.945 0 6.827.912.912 2.124 1.414 3.414 1.414s2.502-.502 3.414-1.414 1.414-2.124 1.414-3.413-.502-2.502-1.414-3.413-2.124-1.414-3.414-1.414-2.502.502-3.414 1.414z' fill='#fff' /> | ||
| </svg> | ||
| </button> | ||
| </div> | ||
| </form> | ||
| </form> |
There was a problem hiding this comment.
Non‑JS fallback is broken: readonly input + type="button" prevents search.
With JS disabled, users can neither type nor submit. Remove readonly and make the button a submit, then have JS intercept to open the modal (progressive enhancement). Add basic a11y attributes.
Apply:
-<form role='search' method='get' class='search-form wedocs-search-form wedocs-single-search-input' action='<?php echo esc_url( home_url( '/' ) ) ?>'>
+<form role='search' method='get' class='search-form wedocs-search-form wedocs-single-search-input' action='<?php echo esc_url( home_url( '/' ) ) ?>'>
<input
name='s'
- readonly
type='search'
class='search-field'
- value='<?php get_search_query(); ?>'
+ value='<?php echo esc_attr( get_search_query() ); ?>'
+ aria-label='<?php echo esc_attr_x( 'Search the documentation', 'aria-label', 'wedocs' ); ?>'
title='<?php echo esc_attr_x( 'Search for:', 'label', 'wedocs' ); ?>'
placeholder='<?php echo esc_attr_x( 'Search for a topic or question', 'placeholder', 'wedocs' ); ?>'
/>
<input type='hidden' name='post_type' value='docs' />
- <button type='button' class='search-submit'>
+ <button type='submit' class='search-submit' aria-haspopup='dialog' aria-controls='wedocs-single-doc-search-modal' aria-expanded='false'>In JS, prevent default on this form/button to open the modal when JS is available.
📝 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.
| <form role='search' method='get' class='search-form wedocs-search-form wedocs-single-search-input' action='<?php echo esc_url( home_url( '/' ) ) ?>'> | |
| <div class='wedocs-search-input'> | |
| <input | |
| name='s' | |
| readonly | |
| type='search' | |
| class='search-field' | |
| value='<?php get_search_query(); ?>' | |
| title='<?php echo esc_attr_x( 'Search for:', 'label', 'wedocs' ); ?>' | |
| placeholder='<?php echo esc_attr_x( 'Search for a topic or question', 'placeholder', 'wedocs' ); ?>' | |
| /> | |
| <input type='hidden' name='post_type' value='docs' /> | |
| <button type='submit' class='search-submit'> | |
| <button type='button' class='search-submit'> | |
| <svg width='15' height='16' fill='none'> | |
| <path fill-rule='evenodd' d='M11.856 10.847l2.883 2.883a.89.89 0 0 1 0 1.257c-.173.174-.401.261-.629.261s-.455-.087-.629-.261l-2.883-2.883c-1.144.874-2.532 1.353-3.996 1.353a6.56 6.56 0 0 1-4.671-1.935c-2.576-2.575-2.576-6.765 0-9.341C3.179.934 4.839.247 6.603.247s3.424.687 4.671 1.935a6.56 6.56 0 0 1 1.935 4.67 6.55 6.55 0 0 1-1.353 3.995zM3.189 3.439c-1.882 1.882-1.882 4.945 0 6.827.912.912 2.124 1.414 3.414 1.414s2.502-.502 3.414-1.414 1.414-2.124 1.414-3.413-.502-2.502-1.414-3.413-2.124-1.414-3.414-1.414-2.502.502-3.414 1.414z' fill='#fff' /> | |
| </svg> | |
| </button> | |
| </div> | |
| </form> | |
| </form> | |
| <form role='search' method='get' class='search-form wedocs-search-form wedocs-single-search-input' action='<?php echo esc_url( home_url( '/' ) ) ?>'> | |
| <div class='wedocs-search-input'> | |
| <input | |
| name='s' | |
| type='search' | |
| class='search-field' | |
| value='<?php echo esc_attr( get_search_query() ); ?>' | |
| aria-label='<?php echo esc_attr_x( 'Search the documentation', 'aria-label', 'wedocs' ); ?>' | |
| title='<?php echo esc_attr_x( 'Search for:', 'label', 'wedocs' ); ?>' | |
| placeholder='<?php echo esc_attr_x( 'Search for a topic or question', 'placeholder', 'wedocs' ); ?>' | |
| /> | |
| <input type='hidden' name='post_type' value='docs' /> | |
| <button | |
| type='submit' | |
| class='search-submit' | |
| aria-haspopup='dialog' | |
| aria-controls='wedocs-single-doc-search-modal' | |
| aria-expanded='false' | |
| > | |
| <svg width='15' height='16' fill='none'> | |
| <path | |
| fill-rule='evenodd' | |
| d='M11.856 10.847l2.883 2.883a.89.89 0 0 1 0 1.257c-.173.174-.401.261-.629.261s-.455-.087-.629-.261l-2.883-2.883c-1.144.874-2.532 1.353-3.996 1.353a6.56 6.56 0 0 1-4.671-1.935c-2.576-2.575-2.576-6.765 0-9.341C3.179.934 4.839.247 6.603.247s3.424.687 4.671 1.935a6.56 6.56 0 0 1 1.935 4.67 6.55 6.55 0 0 1-1.353 3.995zM3.189 3.439c-1.882 1.882-1.882 4.945 0 6.827.912.912 2.124 1.414 3.414 1.414s2.502-.502 3.414-1.414 1.414-2.124 1.414-3.413-.502-2.502-1.414-3.413-2.124-1.414-3.414-1.414-2.502.502-3.414 1.414z' | |
| fill='#fff' | |
| /> | |
| </svg> | |
| </button> | |
| </div> | |
| </form> |
| name='s' | ||
| readonly | ||
| type='search' | ||
| class='search-field' | ||
| value='<?php get_search_query(); ?>' | ||
| title='<?php echo esc_attr_x( 'Search for:', 'label', 'wedocs' ); ?>' | ||
| placeholder='<?php echo esc_attr_x( 'Search for a topic or question', 'placeholder', 'wedocs' ); ?>' | ||
| /> |
There was a problem hiding this comment.
Fix missing echo and escape the search value.
get_search_query() returns a value; it isn't echoed. Also escape for attribute context.
Apply:
- value='<?php get_search_query(); ?>'
+ value='<?php echo esc_attr( get_search_query() ); ?>'📝 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.
| name='s' | |
| readonly | |
| type='search' | |
| class='search-field' | |
| value='<?php get_search_query(); ?>' | |
| title='<?php echo esc_attr_x( 'Search for:', 'label', 'wedocs' ); ?>' | |
| placeholder='<?php echo esc_attr_x( 'Search for a topic or question', 'placeholder', 'wedocs' ); ?>' | |
| /> | |
| name='s' | |
| readonly | |
| type='search' | |
| class='search-field' | |
| value='<?php echo esc_attr( get_search_query() ); ?>' | |
| title='<?php echo esc_attr_x( 'Search for:', 'label', 'wedocs' ); ?>' | |
| placeholder='<?php echo esc_attr_x( 'Search for a topic or question', 'placeholder', 'wedocs' ); ?>' | |
| /> |
🤖 Prompt for AI Agents
In templates/doc-search-form.php around lines 14 to 21, the input value uses
get_search_query() but does not echo it and is not escaped for an attribute;
update the value attribute to echo the result wrapped with esc_attr(
get_search_query() ) so the search query is output and safely escaped for HTML
attribute context.
Rubaiyat-E-Mohammad
added
QA Approved
bug
and removed
Need Testing
QA Approved
bug
Needs Review
labels
2 participants
📌 PR Description Close issue
Feature: Convert Query-based Search to AJAX Search – WeDocs Docs Home
Summary:
This PR replaces the traditional query-based search with an AJAX-powered live search in the WeDocs documentation home page. The new search implementation provides a faster and smoother user experience by fetching results dynamically without a full page reload.
Changes Implemented:
Testing Steps:
Summary by CodeRabbit