improved search function and category searching#2733
improved search function and category searching#2733codervivek5 merged 1 commit intocodervivek5:mainfrom
Conversation
|
@Samridhi-2007 is attempting to deploy a commit to the Vivek Prajapati's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe changes enhance the search functionality by introducing category-based filtering and improved result ranking. UserNavbar now defines searchable categories and extends items with category metadata, while SearchBar is redesigned as a stateful component with a category selector, dropdown state management, and outside-click handling to close the dropdown. Changes
Sequence DiagramsequenceDiagram
actor User
participant UserNavbar
participant SearchBar
participant FilterLogic
User->>SearchBar: Enters search query
SearchBar->>UserNavbar: Triggers handleSearch (parent)
UserNavbar->>FilterLogic: Apply two-phase ranking<br/>(startsWith → contains)
FilterLogic-->>UserNavbar: Return ranked results
UserNavbar-->>SearchBar: Pass searchResults
SearchBar->>SearchBar: Filter by selectedCategory<br/>(useMemo)
SearchBar->>SearchBar: Render dropdown with<br/>filtered results
SearchBar-->>User: Display suggestions
User->>SearchBar: Clicks on result item
SearchBar->>SearchBar: handleResultClick
SearchBar-->>User: Navigate & close dropdown
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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
🤖 Fix all issues with AI agents
In `@src/User/components/SearchBar/SearchBar.jsx`:
- Line 65: The fixed width on the search container (the div with className="flex
items-center rounded-full border-green-800 border-2 bg-gray-200 px-2 py-2
w-[420px]" in SearchBar.jsx) can cause overflow at the md breakpoint; change the
width strategy to be responsive by replacing the rigid w-[420px] with a fluid
approach such as w-full max-w-[420px] (or make the container flex-grow inside
its parent) so the search bar shrinks to available space without exceeding
420px.
- Around line 95-122: Add proper keyboard handling and ARIA linkage for the
SearchBar dropdown: assign an id to the listbox container and set the input's
aria-controls to that id; handle Escape to set showDropdown false; implement
ArrowUp/ArrowDown to track a focusedIndex state (default -1) and move it within
processedResults length, updating DOM focus/highlight on change; handle Enter to
invoke handleResultClick(processedResults[focusedIndex].link) when focusedIndex
>= 0; ensure each <li> has tabIndex={-1} and aria-selected based on focusedIndex
and update focus programmatically when focusedIndex changes inside the SearchBar
component.
🧹 Nitpick comments (5)
src/User/components/SearchBar/SearchBar.jsx (3)
23-33: Redundant name-match filtering — parent already filterssearchResultsby query.The parent (
UserNavbar) already filterssearchableItemsbysearchTermand only passes matching items assearchResults. Re-filtering byitem.name.toLowerCase().includes(query)here is redundant. Only the category filter is actually needed in this component.This also means the "No results found" state could be misleading: if the parent returns matches but none belong to the selected category, users see "No results found" — consider displaying something like "No results in this category" instead.
♻️ Simplify to only apply the category filter
const processedResults = useMemo(() => { const query = searchTerm.trim().toLowerCase(); if (!query) return []; - return searchResults.filter((item) => { - const matchesSearch = item.name.toLowerCase().includes(query); - const matchesCategory = - selectedCategory === "All" || item.category === selectedCategory; - return matchesSearch && matchesCategory; - }); + return searchResults.filter((item) => { + return selectedCategory === "All" || item.category === selectedCategory; + }); }, [searchTerm, searchResults, selectedCategory]);
73-77: Prefer the category string as thekeyinstead of array index.Since each category is already a unique string, using it directly as the key is more semantically correct and avoids potential issues if the list is reordered.
- {categories.map((cat, index) => ( - <option key={index} value={cat}> + {categories.map((cat) => ( + <option key={cat} value={cat}>
4-10:onResultClickis called without a null guard.If
onResultClickis not provided by the parent,handleResultClickwill throw. Consider a guard or marking it as required via PropTypes/TypeScript.const handleResultClick = (link) => { setIsDropdownOpen(false); - onResultClick(link); + onResultClick?.(link); };Also applies to: 56-59
src/User/components/Navbar/UserNavbar.jsx (2)
29-60: Move static constants outside the component to avoid re-creation on every render.
SEARCH_CATEGORIESandsearchableItemsare static data that never depend on props or state. Defining them inside the component body means new array references are allocated on every render, which also causes downstream memoization (e.g., thecategoriesuseMemoinSearchBar) to needlessly recompute.♻️ Hoist to module scope
+const SEARCH_CATEGORIES = [ + "All", + "Fashion", + "Gifts", + "Body Care", + "Furniture", + "Stationery", + "Food & Beverages", + "Beauty & Wellness", + "Health Supplements", + "Latest in Market", +]; + +const searchableItems = [ + { name: "Fashion & Accessories", link: "/popularCategories/fashionAccessories", category: "Fashion" }, + // ... remaining items ... +]; + const UserNavbar = ({ isAdmin }) => { // ... - const SEARCH_CATEGORIES = [ - "All", - ... - ]; - - const searchableItems = [ - ... - ];
62-80: Ranking logic looks good; minor edge case with whitespace-only input.The two-tier ranking (startsWith → contains, each sorted alphabetically) is a solid improvement.
One minor note: if the user types only whitespace,
searchTermis truthy (e.g.," "), so the effect runs andquerybecomes"". Since every stringincludes("")andstartsWith(""), all items are pushed intosearchResults. This doesn't cause a visible bug becauseSearchBarguards against it withsearchTerm.trim().length > 0, but it's wasted work. A simple guard would avoid it:useEffect(() => { - if (searchTerm) { - const query = searchTerm.trim().toLowerCase(); + const query = searchTerm.trim().toLowerCase(); + if (query) {
|
|
||
| return ( | ||
| <div className="relative" ref={searchRef}> | ||
| <div className="flex items-center rounded-full border-green-800 border-2 bg-gray-200 px-2 py-2 w-[420px]"> |
There was a problem hiding this comment.
Fixed w-[420px] may overflow on medium-width viewports.
The search bar is shown at the md breakpoint (≥768px) per the parent's hidden md:block. At 768px with surrounding navbar elements (logo, nav links, icons, auth button), a rigid 420px-wide search bar could overflow or compress siblings. Consider using a responsive width like w-full max-w-[420px] or a flex-based approach.
🤖 Prompt for AI Agents
In `@src/User/components/SearchBar/SearchBar.jsx` at line 65, The fixed width on
the search container (the div with className="flex items-center rounded-full
border-green-800 border-2 bg-gray-200 px-2 py-2 w-[420px]" in SearchBar.jsx) can
cause overflow at the md breakpoint; change the width strategy to be responsive
by replacing the rigid w-[420px] with a fluid approach such as w-full
max-w-[420px] (or make the container flex-grow inside its parent) so the search
bar shrinks to available space without exceeding 420px.
| {showDropdown && ( | ||
| <div | ||
| className="absolute top-14 left-0 bg-white border border-green-800 w-full rounded-md shadow-lg z-20 max-h-72 overflow-y-auto" | ||
| role="listbox" | ||
| > | ||
| {processedResults.length > 0 ? ( | ||
| <> | ||
| <div className="px-3 py-2 bg-green-50 border-b border-green-100 text-xs font-semibold text-green-800 uppercase tracking-wide"> | ||
| Categories | ||
| </div> | ||
| <ul className="py-1"> | ||
| {processedResults.map((result, index) => ( | ||
| <li | ||
| key={`${result.link}-${index}`} | ||
| role="option" | ||
| className="px-4 py-2.5 text-green-700 hover:bg-green-100 cursor-pointer list-none transition-colors" | ||
| onClick={() => handleResultClick(result.link)} | ||
| > | ||
| {result.name} | ||
| </li> | ||
| ))} | ||
| </ul> | ||
| </> | ||
| ) : ( | ||
| <div className="px-4 py-3 text-gray-500 text-sm">No results found</div> | ||
| )} | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Dropdown lacks keyboard navigation — accessibility gap for role="listbox".
The dropdown declares role="listbox" and items use role="option", but there is no keyboard interaction support:
- Escape should close the dropdown.
- Arrow Up/Down should move focus between options.
- Enter should select the focused option.
- The input should have
aria-controlspointing to the listboxid.
Without these, keyboard-only and screen-reader users cannot navigate the suggestions. This is a significant accessibility blocker for a component advertising ARIA listbox semantics.
🤖 Prompt for AI Agents
In `@src/User/components/SearchBar/SearchBar.jsx` around lines 95 - 122, Add
proper keyboard handling and ARIA linkage for the SearchBar dropdown: assign an
id to the listbox container and set the input's aria-controls to that id; handle
Escape to set showDropdown false; implement ArrowUp/ArrowDown to track a
focusedIndex state (default -1) and move it within processedResults length,
updating DOM focus/highlight on change; handle Enter to invoke
handleResultClick(processedResults[focusedIndex].link) when focusedIndex >= 0;
ensure each <li> has tabIndex={-1} and aria-selected based on focusedIndex and
update focus programmatically when focusedIndex changes inside the SearchBar
component.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Kindly mention the issue number related to the PR and close it as well |
|
Congratulations, Your pull request has been successfully merged 🥳🎉 Thank you for your valuable contribution to the project 🚀 Keep contributing!! ✨ 📢 Don’t forget to share your VigyBag PR on LinkedIn and tag @VigyBag and me — we’d love to celebrate your achievement with you! 🔗💡 |
2 participants
🚀 Changes Implemented
1️⃣ Improved Suggestion Ranking
Prioritized results that start with the search query
Secondary results include items that contain the query
Applied alphabetical sorting for consistent ordering
2️⃣ Category-Based Search
Added a category dropdown selector next to the search bar
Users can filter search results by:
All
Fashion
Gifts
Furniture
Body-Care
etc.
Categories are dynamically generated from the full product list
3️⃣ Close on Outside Click
Search suggestions now close automatically when clicking outside the search component
4️⃣ No Results Handling
Displays "No results found" when no matching items exist
Prevents empty dropdown state
Screenshot of changes
if any adjustment needed let me know
Summary by CodeRabbit
Release Notes