Skip to content

feat: add tooltips to navbar elements #652

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat: add tooltips to navbar elements #652

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f61688c
feat: add tooltips to navbar elements
Nishal77 Nov 19, 2025
7963b8f
fix: move tooltip inside FaceSearchDialog for proper interaction
Nishal77 Nov 19, 2025
9632189
fix: remove redundant onClick handler from DialogTrigger
Nishal77 Nov 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 25 additions & 13 deletions frontend/src/components/Dialog/FaceSearchDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ import {
DialogTitle,
DialogTrigger,
} from '@/components/ui/dialog';
import {
Tooltip,
TooltipContent,
TooltipTrigger,
} from '@/components/ui/tooltip';
import { useDispatch } from 'react-redux';
import { useFile } from '@/hooks/selectFile';
import { startSearch, clearSearch } from '@/features/searchSlice';
Expand Down Expand Up @@ -43,7 +48,7 @@ export function FaceSearchDialog() {
showInfoDialog({
title: 'No Matches Found',
message:
'We couldn’t find any matching faces in your gallery for this photo.',
"We couldn't find any matching faces in your gallery for this photo.",
variant: 'info',
}),
);
Expand Down Expand Up @@ -91,21 +96,28 @@ export function FaceSearchDialog() {
return (
<>
<Dialog open={isDialogOpen} onOpenChange={setIsDialogOpen}>
<DialogTrigger asChild>
<Button
onClick={() => setIsDialogOpen(true)}
variant="ghost"
size="icon"
className="h-8 w-8 cursor-pointer p-1"
>
<ScanFace className="h-4 w-4" />
<span className="sr-only">Face Detection Search</span>
</Button>
</DialogTrigger>
<Tooltip>
<TooltipTrigger asChild>
<DialogTrigger asChild>
<Button
variant="ghost"
size="icon"
className="h-8 w-8 cursor-pointer p-1"
aria-label="Search by face"
>
<ScanFace className="h-4 w-4" />
<span className="sr-only">Search by face</span>
</Button>
</DialogTrigger>
</TooltipTrigger>
<TooltipContent>
<p>Search by face</p>
</TooltipContent>
</Tooltip>

<DialogContent className="sm:max-w-md">
<DialogHeader>
<DialogTitle>Face Detection Search</DialogTitle>
<DialogTitle>Search by Face</DialogTitle>
<DialogDescription>
Search for images containing specific faces by uploading a photo
or using your webcam.
Expand Down
72 changes: 48 additions & 24 deletions frontend/src/components/Navigation/Navbar/Navbar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ import { selectAvatar, selectName } from '@/features/onboardingSelectors';
import { clearSearch } from '@/features/searchSlice';
import { convertFileSrc } from '@tauri-apps/api/core';
import { FaceSearchDialog } from '@/components/Dialog/FaceSearchDialog';
import {
Tooltip,
TooltipContent,
TooltipTrigger,
} from '@/components/ui/tooltip';

export function Navbar() {
const userName = useSelector(selectName);
Expand Down Expand Up @@ -42,14 +47,20 @@ export function Navbar() {
className="h-7 w-7 rounded object-cover"
/>
{isSearchActive && (
<button
onClick={() => dispatch(clearSearch())}
className="absolute -top-1 -right-1 flex h-3 w-3 items-center justify-center rounded-full bg-red-600 text-[10px] leading-none text-white"
title="Close"
aria-label="Close"
>
✕
</button>
<Tooltip>
<TooltipTrigger asChild>
<button
onClick={() => dispatch(clearSearch())}
className="absolute -top-1 -right-1 flex h-3 w-3 items-center justify-center rounded-full bg-red-600 text-[10px] leading-none text-white"
aria-label="Close"
>
✕
</button>
</TooltipTrigger>
<TooltipContent>
<p>Clear search</p>
</TooltipContent>
</Tooltip>
Comment on lines +50 to +63
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Align aria-label with tooltip text for consistency.

The button's aria-label="Close" (line 55) doesn't match the tooltip text "Clear search" (line 61). Since the action clears the search query, "Clear search" is more descriptive. Update the aria-label to match.

Apply this diff:

-                      aria-label="Close"
+                      aria-label="Clear search"
📝 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.

Suggested change
<Tooltip>
<TooltipTrigger asChild>
<button
onClick={() => dispatch(clearSearch())}
className="absolute -top-1 -right-1 flex h-3 w-3 items-center justify-center rounded-full bg-red-600 text-[10px] leading-none text-white"
aria-label="Close"
>
✕
</button>
</TooltipTrigger>
<TooltipContent>
<p>Clear search</p>
</TooltipContent>
</Tooltip>
<Tooltip>
<TooltipTrigger asChild>
<button
onClick={() => dispatch(clearSearch())}
className="absolute -top-1 -right-1 flex h-3 w-3 items-center justify-center rounded-full bg-red-600 text-[10px] leading-none text-white"
aria-label="Clear search"
>
✕
</button>
</TooltipTrigger>
<TooltipContent>
<p>Clear search</p>
</TooltipContent>
</Tooltip>
🤖 Prompt for AI Agents 
In frontend/src/components/Navigation/Navbar/Navbar.tsx around lines 50 to 63,
the button's aria-label is "Close" while the tooltip reads "Clear search";
update the button's aria-label to "Clear search" so it matches the tooltip and
accurately describes the action (i.e., replace aria-label="Close" with
aria-label="Clear search").

)}
</div>
)}
Expand All @@ -62,16 +73,22 @@ export function Navbar() {
/>

{/* FaceSearch Dialog */}

<FaceSearchDialog />

<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"
>
<Search className="h-4 w-4" />
</button>
{/* Search Button */}
<Tooltip>
<TooltipTrigger asChild>
<button
className="text-muted-foreground hover:bg-accent dark:hover:bg-accent/50 hover:text-foreground mx-1 cursor-pointer rounded-sm p-2"
aria-label="Search"
>
<Search className="h-4 w-4" />
</button>
</TooltipTrigger>
<TooltipContent>
<p>Search images</p>
</TooltipContent>
</Tooltip>
Comment on lines +79 to +91
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Align aria-label with tooltip text for consistency.

The button's aria-label="Search" (line 92) is less specific than the tooltip text "Search images" (line 98). Update the aria-label to match.

Apply this diff:

-                aria-label="Search"
+                aria-label="Search images"
📝 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.

Suggested change
<Tooltip>
<TooltipTrigger asChild>
<button
className="text-muted-foreground hover:bg-accent dark:hover:bg-accent/50 hover:text-foreground mx-1 cursor-pointer rounded-sm p-2"
aria-label="Search"
>
<Search className="h-4 w-4" />
</button>
</TooltipTrigger>
<TooltipContent>
<p>Search images</p>
</TooltipContent>
</Tooltip>
<Tooltip>
<TooltipTrigger asChild>
<button
className="text-muted-foreground hover:bg-accent dark:hover:bg-accent/50 hover:text-foreground mx-1 cursor-pointer rounded-sm p-2"
aria-label="Search images"
>
<Search className="h-4 w-4" />
</button>
</TooltipTrigger>
<TooltipContent>
<p>Search images</p>
</TooltipContent>
</Tooltip>
🤖 Prompt for AI Agents 
frontend/src/components/Navigation/Navbar/Navbar.tsx around lines 88 to 100: the
button's aria-label currently reads "Search" while the TooltipContent says
"Search images"; update the button's aria-label to exactly "Search images" so
screen readers match the visible tooltip and improve accessibility/consistency.

</div>
</div>

Expand All @@ -82,15 +99,22 @@ export function Navbar() {
<span className="hidden text-sm sm:inline-block">
Welcome <span className="text-muted-foreground">{userName}</span>
</span>
<a href="/settings" className="p-2">
<img
src={userAvatar || '/photo1.png'}
className="hover:ring-primary/50 h-8 w-8 cursor-pointer rounded-full transition-all hover:ring-2"
alt="User avatar"
/>
</a>
<Tooltip>
<TooltipTrigger asChild>
<a href="/settings" className="p-2">
<img
src={userAvatar || '/photo1.png'}
className="hover:ring-primary/50 h-8 w-8 cursor-pointer rounded-full transition-all hover:ring-2"
alt="User avatar"
/>
</a>
</TooltipTrigger>
<TooltipContent>
<p>Profile settings</p>
</TooltipContent>
</Tooltip>
Comment on lines +102 to +115
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Add aria-label to the profile settings link.

The anchor on line 113 is missing an aria-label, which would improve accessibility for screen reader users. While the image has alt="User avatar", the link's purpose (navigating to settings) isn't explicitly conveyed.

Apply this diff:

-              <a href="/settings" className="p-2">
+              <a href="/settings" className="p-2" aria-label="Profile settings">
📝 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.

Suggested change
<Tooltip>
<TooltipTrigger asChild>
<a href="/settings" className="p-2">
<img
src={userAvatar || '/photo1.png'}
className="hover:ring-primary/50 h-8 w-8 cursor-pointer rounded-full transition-all hover:ring-2"
alt="User avatar"
/>
</a>
</TooltipTrigger>
<TooltipContent>
<p>Profile settings</p>
</TooltipContent>
</Tooltip>
<Tooltip>
<TooltipTrigger asChild>
<a href="/settings" className="p-2" aria-label="Profile settings">
<img
src={userAvatar || '/photo1.png'}
className="hover:ring-primary/50 h-8 w-8 cursor-pointer rounded-full transition-all hover:ring-2"
alt="User avatar"
/>
</a>
</TooltipTrigger>
<TooltipContent>
<p>Profile settings</p>
</TooltipContent>
</Tooltip>
🤖 Prompt for AI Agents 
In frontend/src/components/Navigation/Navbar/Navbar.tsx around lines 111 to 124,
the anchor linking to /settings is missing an aria-label which means screen
readers don't get the explicit purpose of the link; add a descriptive aria-label
attribute to the <a> (for example aria-label="Open profile settings" or similar
localized text) while keeping the existing image alt intact so the link's
function is conveyed to assistive technologies.

</div>
</div>
</div>
);
}
}
83 changes: 47 additions & 36 deletions frontend/src/components/ThemeToggle.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,46 +7,57 @@ import {
DropdownMenuItem,
DropdownMenuTrigger,
} from '@/components/ui/dropdown-menu';
import {
Tooltip,
TooltipContent,
TooltipTrigger,
} from '@/components/ui/tooltip';
import { useTheme } from '@/contexts/ThemeContext';

export function ThemeSelector() {
const { setTheme } = useTheme();

return (
<DropdownMenu>
<DropdownMenuTrigger asChild>
<Button
className="cursor-pointer"
variant="outline"
size="icon"
title="Themes"
aria-label="themes"
>
<Sun className="h-[1.2rem] w-[1.2rem] scale-100 rotate-0 transition-all dark:scale-0 dark:-rotate-90" />
<Moon className="absolute h-[1.2rem] w-[1.2rem] scale-0 rotate-90 transition-all dark:scale-100 dark:rotate-0" />
<span className="sr-only">Toggle theme</span>
</Button>
</DropdownMenuTrigger>
<DropdownMenuContent align="end">
<DropdownMenuItem
className="cursor-pointer"
onClick={() => setTheme('light')}
>
Light
</DropdownMenuItem>
<DropdownMenuItem
className="cursor-pointer"
onClick={() => setTheme('dark')}
>
Dark
</DropdownMenuItem>
<DropdownMenuItem
className="cursor-pointer"
onClick={() => setTheme('system')}
>
System
</DropdownMenuItem>
</DropdownMenuContent>
</DropdownMenu>
<Tooltip>
<TooltipTrigger asChild>
<DropdownMenu>
<DropdownMenuTrigger asChild>
<Button
className="cursor-pointer"
variant="outline"
size="icon"
aria-label="Toggle theme"
>
<Sun className="h-[1.2rem] w-[1.2rem] scale-100 rotate-0 transition-all dark:scale-0 dark:-rotate-90" />
<Moon className="absolute h-[1.2rem] w-[1.2rem] scale-0 rotate-90 transition-all dark:scale-100 dark:rotate-0" />
<span className="sr-only">Toggle theme</span>
</Button>
</DropdownMenuTrigger>
<DropdownMenuContent align="end">
<DropdownMenuItem
className="cursor-pointer"
onClick={() => setTheme('light')}
>
Light
</DropdownMenuItem>
<DropdownMenuItem
className="cursor-pointer"
onClick={() => setTheme('dark')}
>
Dark
</DropdownMenuItem>
<DropdownMenuItem
className="cursor-pointer"
onClick={() => setTheme('system')}
>
System
</DropdownMenuItem>
</DropdownMenuContent>
</DropdownMenu>
</TooltipTrigger>
<TooltipContent>
<p>Toggle theme</p>
</TooltipContent>
</Tooltip>
);
}
}