-
Notifications
You must be signed in to change notification settings - Fork 723
feat:[Site] Add search functionality to blog #2375
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
feat:[Site] Add search functionality to blog #2375
Conversation
|
Yay, your first pull request! 👍 A contributor will be by to give feedback soon. In the meantime, please review the Newcomers' Guide and sure to join the community Slack. |
✅ Deploy Preview for mesheryio-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary of ChangesHello @Sandijigs, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the blog section by integrating a robust search capability. Users can now efficiently locate relevant blog posts by typing keywords into a dedicated search bar, which filters results in real-time across post titles, content, and categories. The implementation includes performance optimizations like debouncing, along with user-friendly features such as a dynamic results counter and a clear message when no matches are found. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a valuable search feature to the blog. The implementation is well-structured, incorporating debouncing for performance and good user experience elements like a results counter and an 'Escape' key handler. My review focuses on a critical CSS syntax error that needs correction, a high-priority performance enhancement for the search logic to make it more scalable, and a couple of medium-priority suggestions to improve code maintainability and consistency in styling.
blog/index.html
Outdated
| @media (max-width: 768px) { | ||
| text-align: center; | ||
| margin: 0 auto 1.5rem auto; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This @media query is nested inside the .sorting-options rule. Standard CSS does not support nested rules; this is a feature of preprocessors like SASS/SCSS. As this is in a plain <style> tag, browsers will ignore this block, and the responsive styles for .sorting-options will not be applied. You need to move this media query to the top level of the stylesheet, outside of any other rule blocks.
For example:
.sorting-options {
/* ... existing styles ... */
}
@media (max-width: 768px) {
.sorting-options {
text-align: center;
margin: 0 auto 1.5rem auto;
}
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Sandijigs you might want to take this suggestion in consideration
blog/index.html
Outdated
| const blogPosts = Array.from(blogPostsList.querySelectorAll('.blog-post')); | ||
| const totalPosts = blogPosts.length; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The totalPosts variable is declared but never used and can be removed. More importantly, for better performance, you can pre-process the blog post data here instead of inside the search loop. This avoids repeatedly querying the DOM on every keystroke. This change prepares the data for a more efficient search function.
const blogPosts = Array.from(blogPostsList.querySelectorAll('.blog-post'));
const postsData = blogPosts.map(post => {
const title = post.querySelector('h2 a')?.textContent.toLowerCase() || '';
const excerpt = post.querySelector('.post-content')?.textContent.toLowerCase() || '';
const categories = Array.from(post.querySelectorAll('.post-categories a, .blog-filter a'))
.map(cat => cat.textContent.toLowerCase())
.join(' ');
return {
element: post,
searchableText: `${title} ${excerpt} ${categories}`
};
});
blog/index.html
Outdated
| blogPosts.forEach(post => { | ||
| const title = post.querySelector('h2 a')?.textContent.toLowerCase() || ''; | ||
| const excerpt = post.querySelector('.post-content')?.textContent.toLowerCase() || ''; | ||
| const categories = Array.from(post.querySelectorAll('.post-categories a, .blog-filter a')) | ||
| .map(cat => cat.textContent.toLowerCase()) | ||
| .join(' '); | ||
|
|
||
| // Check if search term matches title, excerpt, or categories | ||
| const matches = title.includes(searchTerm) || | ||
| excerpt.includes(searchTerm) || | ||
| categories.includes(searchTerm); | ||
|
|
||
| if (matches) { | ||
| post.classList.remove('hidden'); | ||
| visibleCount++; | ||
| } else { | ||
| post.classList.add('hidden'); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following the previous suggestion to create postsData, you can now simplify this loop to iterate over the pre-processed data. This makes the search logic much more efficient and readable, as it only needs to perform a simple string inclusion check against the pre-compiled searchableText.
postsData.forEach(({ element, searchableText }) => {
const matches = searchableText.includes(searchTerm);
if (matches) {
element.classList.remove('hidden');
visibleCount++;
} else {
element.classList.add('hidden');
}
});
blog/index.html
Outdated
| <ul class="blog-posts" id="blog-posts-list"> | ||
| {% include list-posts.html posts=paginator.posts %} | ||
| </ul> | ||
| <div id="no-results" class="no-results" style="display: none;"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using an inline style attribute to hide this element is not ideal as it mixes presentation with structure. A more consistent approach would be to use a utility hidden class, similar to how posts are hidden. This would require three small changes:
-
Update the HTML: Add the
hiddenclass to thisdivinstead of the inline style.<div id="no-results" class="no-results hidden">
-
Generalize the CSS: Make the
.hiddenclass reusable by removing the.blog-postscope in your stylesheet (at line 192)./* Change from: */ .blog-post.hidden { ... } /* To: */ .hidden { ... }
-
Update the JavaScript: Use
classList.add('hidden')andclassList.remove('hidden')on thenoResultselement instead of changing itsstyle.displayproperty.// e.g., in the 'if (visibleCount === 0)' block noResults.classList.remove('hidden'); // instead of .style.display = 'block' // and noResults.classList.add('hidden'); // instead of .style.display = 'none'
This approach keeps all styling concerns within the CSS and makes the code more consistent.
blog/index.html
Outdated
| .sorting-options::before { | ||
| content: ''; | ||
| position: absolute; | ||
| left: 15px; | ||
| top: 50%; | ||
| transform: translateY(-50%); | ||
| width: 20px; | ||
| height: 20px; | ||
| background-image: url('data:image/svg+xml;utf8,<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="none" stroke="%23666" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><circle cx="11" cy="11" r="8"></circle><path d="m21 21-4.35-4.35"></path></svg>'); | ||
| background-size: contain; | ||
| background-repeat: no-repeat; | ||
| pointer-events: none; | ||
| z-index: 1; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The search icon SVG has a hardcoded stroke color (%23666, which is #666). This color may not have enough contrast against the dark background (var(--color-primary-dark)) and won't adapt to theme changes. It's better to use currentColor for the stroke in the SVG, and then set the color property on the pseudo-element. This allows the icon to inherit color and adapt to themes easily. I've suggested using #888 to match the input's placeholder color.
.sorting-options::before {
content: '';
position: absolute;
left: 15px;
top: 50%;
transform: translateY(-50%);
width: 20px;
height: 20px;
background-image: url('data:image/svg+xml;utf8,<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><circle cx="11" cy="11" r="8"></circle><path d="m21 21-4.35-4.35"></path></svg>');
background-size: contain;
background-repeat: no-repeat;
pointer-events: none;
z-index: 1;
color: #888;
}
|
@Sandijigs make sure you sign off your previous commits as well , the DCO is failing |
|
🚨 Alert! Git Police! We couldn’t help but notice that one or more of your commits is missing a sign-off. A what? A commit sign-off (your email address). To amend the commits in this PR with your signoff using the instructions provided in the DCO check. To configure your dev environment to automatically signoff on your commits in the future, see these instructions.
|
|
@Sandijigs we have search bar styling in other pages like catalog and extensions , please try to reuse them for consistency |
|
okay @Namanv0509 , Thanks for the feed back |
|
@Namanv0509 , I have modified the blog search design similar to the search bar in catalog and extensions |
|
hey @Sandijigs -- For DCO check use these command git rebase -i HEAD~1
git commit --amend --signoff
git rebase --continue
git push --force |
a31e0e4 to
2421f62
Compare
Signed-off-by: Sandijigs <[email protected]>
Signed-off-by: Sandijigs <[email protected]>
Signed-off-by: Sandijigs <[email protected]>
2421f62 to
e1413df
Compare
thanks @Namanv0509 . the checks have all passed |
leecalcote
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use Meilisearch.
Description
This PR fixes #2354 [[Site] Add search functionality to blog #2354]
Notes for Reviewers
This PR adds search functionality to the blog section at
/blog, addressing the missing ability to search blog posts by keywords.Added Functionality:
Signed commits