Feat: Implement Smart Search Bar with Voice Search for Quick Navigation#646
Feat: Implement Smart Search Bar with Voice Search for Quick Navigation#646ParikhShreya wants to merge 0 commit intoAOSSIE-Org:mainfrom
Conversation
WalkthroughA blank line preceding the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
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 |
|
Hello @rahulharpal1603 Sir , I’ve implemented the Smart Search Bar with a Voice Search button for PictoPy, as discussed in issue #627 . The PR is ready for review: #646 I’d appreciate any feedback or suggestions you might have. Thanks, |
![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
🧹 Nitpick comments (4)
frontend/src/components/Dialog/VoiceCommand.tsx (2)
9-17: Align unsupported-browser handling with app-wide dialogsUsing
alert("Speech recognition not supported")is jarring compared to the ErrorDialog pattern introduced in the Navbar. Consider surfacing this via the same dialog or a shared error mechanism so the UX stays consistent across entry points.
46-51: Improve mic button accessibility and consider reusing shared voice UIThe mic button currently has only an icon and no accessible label. Adding something like
aria-label="Start voice search"(and optionallytitle) would help screen readers and clarify its purpose. Also, this modal’s mic/“Speak now…” UI closely mirrors the Navbar’s voice modal; consider extracting a shared component or reusing this one from Navbar to avoid duplication and drift.Also applies to: 53-82
frontend/src/components/Navigation/Navbar/Navbar.tsx (2)
63-70: Prefer SPA navigation helper overwindow.location.hrefUsing
window.location.href = routeMap[key];forces a full page reload and bypasses your router’s navigation (e.g.,useNavigatefrom React Router), which can drop client state and feel slower. If this project already uses a routing library (it appears to, givenuseNavigateinFaceSearchDialog), consider refactoringgoToPageto use that instead for a smoother in-app navigation experience.
50-61: Search suggestions logic looks sound; consider minor UX refinementsThe core suggestion behavior and keyboard navigation (ArrowUp/Down, Enter) look correct and should satisfy the quick navigation requirement. You might optionally consider:
- Closing the dropdown on blur or Esc to avoid lingering suggestions when the field loses focus.
- Highlighting and auto-selecting the first suggestion on Enter when
activeIndex === -1as a convenience.These are UX tweaks, not correctness issues.
Also applies to: 73-89, 91-94, 195-212
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/components/Dialog/VoiceCommand.tsx(1 hunks)frontend/src/components/Navigation/Navbar/Navbar.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/components/Navigation/Navbar/Navbar.tsx (3)
frontend/src/features/onboardingSelectors.ts (2)
selectName(15-15)selectAvatar(13-13)frontend/src/features/searchSlice.ts (1)
clearSearch(21-24)frontend/src/components/Dialog/FaceSearchDialog.tsx (1)
FaceSearchDialog(25-153)
| {/* Voice Command */} | ||
| <button | ||
| className="text-muted-foreground hover:bg-accent dark:hover:bg-accent/50 hover:text-foreground mx-1 cursor-pointer rounded-sm p-2" | ||
| title="Search" | ||
| aria-label="Search" | ||
| onClick={startListening} | ||
| className="p-3 rounded-full bg-purple-600 hover:bg-purple-700 transition shadow-md flex items-center justify-center z-50" | ||
| > | ||
| <Search className="h-4 w-4" /> | ||
| <Mic className="h-6 w-6 text-white" /> | ||
| </button> |
There was a problem hiding this comment.
Align mic button accessibility and reuse shared SpeechRecognition UI
A couple of small but impactful improvements here:
- The navbar mic button has only an icon; adding
aria-label="Start voice search"(and optionallytitle) would improve accessibility and clarify its purpose. - The voice modal markup (glowing mic, “Speak now…”, transcript text) is very similar to the new
VoiceCommandcomponent’s UI. Consolidating this into a shared component (or directly reusingVoiceCommand) would reduce duplication in SpeechRecognition setup and help keep behavior/visuals consistent whenever you tweak the voice UI.
Also note: the voice modal’s inner container lacks relative, but the close button uses absolute positioning, so the ✕ is positioned relative to the viewport instead of the card. You can fix this by making the card container relative, analogous to ErrorDialog:
- <div className="bg-white dark:bg-black/95 backdrop-blur-md rounded-3xl shadow-2xl p-6 w-full max-w-sm text-center border border-white/20 dark:border-neutral-700/50">
+ <div className="relative bg-white dark:bg-black/95 backdrop-blur-md rounded-3xl shadow-2xl p-6 w-full max-w-sm text-center border border-white/20 dark:border-neutral-700/50">Also applies to: 234-258
🤖 Prompt for AI Agents
In frontend/src/components/Navigation/Navbar/Navbar.tsx around lines 187-193
(and also apply same changes to 234-258), the mic button is icon-only and the
modal markup duplicates the VoiceCommand UI and mispositions the close button;
update the mic <button> to include an accessible aria-label (e.g.,
aria-label="Start voice search") and optionally a title, replace the duplicated
modal UI by reusing the existing VoiceCommand component (or extract shared UI
into one component and render it here) so SpeechRecognition setup and visuals
are unified, and make the modal/card inner container position:relative so the
close (absolute) button is positioned correctly inside the card. Ensure
props/context needed by VoiceCommand (callbacks/state) are passed through and
remove the duplicated markup.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
frontend/src/components/Navigation/Navbar/Navbar.tsx (2)
113-121: Consider more precise voice matching for ambiguous input.The current first-match approach might not find the most relevant page when spoken input is ambiguous. For example, "home videos" would match "home" first, even though "videos" might be the intended target.
Consider sorting matches by specificity (length or exact match):
const found = suggestionKeys.find((k) => spoken.toLowerCase().replace(/\s+/g, "-").includes(k) ); + +// Alternative: prioritize longer matches +const matches = suggestionKeys.filter((k) => + spoken.toLowerCase().replace(/\s+/g, "-").includes(k) +); +const found = matches.sort((a, b) => b.length - a.length)[0];
232-258: Consider consolidating duplicate voice UI with VoiceCommand component.As noted in a previous review, this voice modal markup closely duplicates the
VoiceCommandcomponent mentioned in the PR summary. The inline SpeechRecognition setup and UI creates maintenance overhead—any updates to the voice experience would need to be applied in multiple places.Based on past review feedback, consider either:
- Importing and reusing the existing
VoiceCommandcomponent, or- Extracting the shared voice UI/logic into a single reusable component
This would centralize the SpeechRecognition setup, visual styling (glowing mic, transcript display), and behavior, ensuring consistency across the application.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/Navigation/Navbar/Navbar.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/components/Navigation/Navbar/Navbar.tsx (4)
frontend/src/features/onboardingSelectors.ts (2)
selectName(15-15)selectAvatar(13-13)frontend/src/features/searchSlice.ts (1)
clearSearch(21-24)frontend/src/components/Dialog/FaceSearchDialog.tsx (1)
FaceSearchDialog(25-153)frontend/src/components/ThemeToggle.tsx (1)
ThemeSelector(12-52)
| const goToPage = (label: string) => { | ||
| const key = label.trim().toLowerCase().replace(/\s+/g, "-"); | ||
| if (routeMap[key]) { | ||
| window.location.href = routeMap[key]; | ||
| } else { | ||
| setError(`The page "${label}" does not exist.`); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Avoid full-page reload; use React Router navigation.
window.location.href triggers a full page reload, which loses all React state and is slower than client-side navigation. Since this appears to be a React SPA, use React Router's useNavigate() hook instead.
Apply this diff:
+import { useNavigate } from 'react-router-dom';
+
export function Navbar() {
+ const navigate = useNavigate();
// ...
const goToPage = (label: string) => {
const key = label.trim().toLowerCase().replace(/\s+/g, "-");
if (routeMap[key]) {
- window.location.href = routeMap[key];
+ navigate(routeMap[key]);
} else {
setError(`The page "${label}" does not exist.`);
}
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/src/components/Navigation/Navbar/Navbar.tsx around lines 61 to 68,
replace the full-page reload approach (window.location.href) with React Router
client-side navigation: import and call the useNavigate() hook at the top of the
component, obtain a navigate function, and use navigate(routeMap[key]) instead
of window.location.href; keep the existing setError branch intact and ensure the
useNavigate import is added from 'react-router-dom' and invoked inside the
component body before goToPage is used.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/src/components/Navigation/Navbar/Navbar.tsx (1)
59-68: Still using full-page reload; must use React Router navigation.Despite the comment claiming "No full page reload,"
window.location.assign()on line 64 does trigger a full page reload, which loses all React state and is slower than client-side navigation. The previous review requested switching to React Router'suseNavigate()hook, but this was not implemented.Apply this diff:
+import { useNavigate } from 'react-router-dom'; + export function Navbar() { + const navigate = useNavigate(); const userName = useSelector(selectName); // ... const goToPage = (label: string) => { const key = label.trim().toLowerCase().replace(/\s+/g, "-"); if (routeMap[key]) { - window.location.assign(routeMap[key]); // ⭐ FIXED + navigate(routeMap[key]); } else { setError(`The page "${label}" does not exist.`); } };
🧹 Nitpick comments (1)
frontend/src/components/Navigation/Navbar/Navbar.tsx (1)
240-266: Consider reusing the VoiceCommand component to reduce duplication.The voice modal UI (animated mic, "Speak now…" text, transcript display) duplicates functionality that could be provided by the
VoiceCommandcomponent mentioned in the AI summary. Consolidating this into a shared component would:
- Reduce code duplication
- Keep voice UI behavior consistent across the app
- Simplify future maintenance
Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/Navigation/Navbar/Navbar.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/components/Navigation/Navbar/Navbar.tsx (3)
frontend/src/features/onboardingSelectors.ts (2)
selectName(15-15)selectAvatar(13-13)frontend/src/features/searchSlice.ts (1)
clearSearch(21-24)frontend/src/components/Dialog/FaceSearchDialog.tsx (1)
FaceSearchDialog(25-153)
🔇 Additional comments (6)
frontend/src/components/Navigation/Navbar/Navbar.tsx (6)
1-9: Imports look good.All dependencies are appropriate for the smart search and voice functionality being implemented.
11-25: ErrorDialog implementation is correct.The component properly handles error display with correct positioning for the close button.
36-57: Route mapping and state management are well-structured.The routeMap and suggestionKeys are consistent (all lowercase), and the filtered suggestions logic correctly handles query matching.
70-88: Keyboard navigation implementation is solid.The ArrowDown, ArrowUp, and Enter key handlers correctly manage suggestion selection, and the useEffect properly resets the active index when the query changes.
144-219: Search UI and accessibility improvements are well-implemented.The search bar layout is clean, keyboard navigation is integrated, and the voice button now includes proper
aria-labelandtitleattributes for accessibility. The suggestions dropdown provides a good user experience.
270-271: Default export addition is a helpful enhancement.Providing both named and default exports gives consumers flexibility in how they import the component.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
frontend/src/components/Navigation/Navbar/Navbar.tsx (2)
251-277: Consider reusing VoiceCommand component to reduce duplication.The voice modal markup duplicates functionality that exists in the separate
VoiceCommandcomponent (mentioned in the AI summary). This creates maintenance overhead when updating voice UI.Consider replacing the inline voice modal with the
VoiceCommandcomponent:- {voiceOpen && ( - <div className="fixed top-40 left-[256px] right-4 z-[1000] flex justify-center"> - <div className="relative bg-white dark:bg-black/95 backdrop-blur-md rounded-3xl shadow-2xl p-6 w-full max-w-sm text-center border border-white/20 dark:border-neutral-700/50"> - {/* ... mic icon, text, etc ... */} - </div> - </div> - )} + {voiceOpen && ( + <VoiceCommand + isOpen={voiceOpen} + onClose={() => setVoiceOpen(false)} + transcript={voiceText} + /> + )}This would consolidate speech recognition UI and behavior in one place. You may need to adjust
VoiceCommand's props to support this use case.
63-71: Use React Router navigation instead of full page reload.The
window.location.hrefassignment (line 67) triggers a full page reload, which loses all React state and is slower than client-side navigation. This issue was flagged in a previous review but remains unaddressed.Apply this diff to use React Router:
+import { useNavigate } from 'react-router-dom'; + export function Navbar() { + const navigate = useNavigate(); // ... existing code ... const goToPage = (label: string) => { const key = label.trim().toLowerCase().replace(/\s+/g, "-"); if (routeMap[key]) { - window.location.href = routeMap[key]; + navigate(routeMap[key]); } else { setError(`The page "${label}" does not exist.`); } };
🧹 Nitpick comments (2)
frontend/src/components/Navigation/Navbar/Navbar.tsx (2)
126-138: Consider more precise voice matching.The current matching logic (line 130-132) uses
.includes(), which allows fuzzy matching but may produce false positives. For example, "I want to go home" would match "home", and "video games" would match "videos".While this might be intentional for a flexible UX, consider using a more precise approach:
const found = suggestionKeys.find((k) => - spoken.toLowerCase().replace(/\s+/g, "-").includes(k) + { + const normalized = spoken.toLowerCase().replace(/\s+/g, "-"); + const words = normalized.split(/[\s-]+/); + return words.includes(k) || normalized === k; + } );This would match exact words or the full normalized phrase, reducing accidental matches.
167-229: Enhance search accessibility with ARIA attributes.The search input and dropdown could benefit from proper ARIA combobox patterns for better screen reader support:
<Input type="search" placeholder="Search or say something..." + aria-label="Search images and pages" + role="combobox" + aria-expanded={filtered.length > 0} + aria-controls="search-suggestions" + aria-activedescendant={activeIndex >= 0 ? `suggestion-${activeIndex}` : undefined} value={query} onChange={(e) => setQuery(e.target.value)} onKeyDown={handleKeyDown} className="flex-1 border-0 bg-transparent" /> {/* Dropdown */} {filtered.length > 0 && ( - <div className="absolute left-0 right-0 top-full mt-1 bg-white dark:bg-neutral-900 shadow-lg rounded-xl z-50 max-h-64 overflow-y-auto border border-gray-200 dark:border-neutral-700"> + <div id="search-suggestions" role="listbox" className="absolute left-0 right-0 top-full mt-1 bg-white dark:bg-neutral-900 shadow-lg rounded-xl z-50 max-h-64 overflow-y-auto border border-gray-200 dark:border-neutral-700"> {filtered.map((key, idx) => ( <div key={key} + id={`suggestion-${idx}`} + role="option" + aria-selected={idx === activeIndex} className={`px-4 py-2 cursor-pointer capitalize ${ idx === activeIndex ? "bg-purple-100 dark:bg-purple-700 text-purple-700 dark:text-white" : "hover:bg-neutral-100 dark:hover:bg-neutral-700" }`} onMouseDown={() => goToPage(key)} > {key} </div> ))} </div> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/Navigation/Navbar/Navbar.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/components/Navigation/Navbar/Navbar.tsx (4)
frontend/src/features/onboardingSelectors.ts (2)
selectName(15-15)selectAvatar(13-13)frontend/src/features/searchSlice.ts (1)
clearSearch(21-24)frontend/src/components/Dialog/FaceSearchDialog.tsx (1)
FaceSearchDialog(25-153)frontend/src/components/ThemeToggle.tsx (1)
ThemeSelector(12-52)
🔇 Additional comments (5)
frontend/src/components/Navigation/Navbar/Navbar.tsx (5)
1-10: LGTM! Imports are clean and complete.All necessary dependencies are imported, and the React 19 hooks are properly included.
11-25: LGTM! ErrorDialog properly positioned.The relative positioning on the inner container correctly anchors the absolute close button. Clean implementation.
73-88: LGTM! Keyboard navigation properly implemented.Arrow key cycling and Enter key selection work correctly with proper bounds checking and default behavior prevention.
95-102: LGTM! Cleanup properly prevents memory leaks.The cleanup effect correctly stops the speech recognition instance on unmount, addressing the critical issue from the previous review.
282-282: LGTM! Default export added.The addition of a default export alongside the named export provides flexibility for consumers while maintaining backward compatibility.
|
Hello @rahulharpal1603 Sir, I’ve completed the updates and submitted the PR. When you’re free, could you please take a look and share your feedback? I’m ready to make any improvements. Thanks! |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/app/database/connection.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-31T17:00:50.132Z
Learnt from: Hemil36
Repo: AOSSIE-Org/PictoPy PR: 570
File: backend/app/database/connection.py:16-24
Timestamp: 2025-10-31T17:00:50.132Z
Learning: In PictoPy backend, the user prefers not to use database connection retry logic or extended busy timeouts in the centralized get_db_connection() context manager, even though the app has concurrent access patterns via ProcessPoolExecutor and FastAPI.
Applied to files:
backend/app/database/connection.py
backend/app/database/connection.py
Outdated
| from app.config.settings import DATABASE_PATH | ||
|
|
||
|
|
||
| print("🔥 CURRENT DB PATH:", DATABASE_PATH) |
There was a problem hiding this comment.
Remove debug print statement or replace with proper logging.
This print statement appears to be a debug artifact. It will execute on every module import and clutter logs with unformatted output.
Additionally, this backend change seems unrelated to the PR's stated objective of adding a frontend Smart Search Bar with Voice Search. Please clarify if this was added intentionally or remove it before merging.
If database path logging is needed for operational purposes, use Python's logging module instead:
-print("🔥 CURRENT DB PATH:", DATABASE_PATH)
+import logging
+logger = logging.getLogger(__name__)
+logger.debug("Database path: %s", DATABASE_PATH)Or remove the line entirely:
-print("🔥 CURRENT DB PATH:", DATABASE_PATH)📝 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.
| print("🔥 CURRENT DB PATH:", DATABASE_PATH) |
🤖 Prompt for AI Agents
In backend/app/database/connection.py around line 6, remove the debug print
statement printing DATABASE_PATH or replace it with a proper logger call;
specifically either delete the line print("🔥 CURRENT DB PATH:", DATABASE_PATH)
or import and use Python's logging (getLogger(__name__).debug("Current DB path:
%s", DATABASE_PATH)) so it doesn't execute unformatted on module import; also
confirm whether this change is intentional for this PR and if not remove the
line before merging.
1 participant
Overview
This PR implements a Smart Search Bar with a Voice Search button for PictoPy.
Previously, users had to manually browse the sidebar to access features like AI Tagging, Videos, or Albums.
With this enhancement, users can quickly search for pages, images, or tags using text or voice input.
Features Added
Smart Search Bar
AI Tagging,Videos,Albums, etc.Benefits
Implementation Notes
SearchBarcomponent.Micicon for voice search.Screenshots
LIGHT MODE
DARK MODE
###Video
pictopy-2025-11-17-17-49-44_eGOANg8m.mp4
Issue
Closes #627
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.