fix(navbar): complete tooltips, SPA nav, and error handling (#785)#789
fix(navbar): complete tooltips, SPA nav, and error handling (#785)#789VasuS609 wants to merge 1 commit intoAOSSIE-Org:mainfrom
Conversation
|
|
WalkthroughThis PR enhances the frontend UI with tooltip additions to Dialog and Settings components, implements client-side routing in the Navbar, and introduces a custom avatar upload feature with Tauri file picker integration and error handling for file selection operations. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as React Component
participant TauriDialog as Tauri Dialog API
participant FileSystem as File System
participant AssetConverter as Asset URL Converter
participant State as Component State
User->>UI: Click "Upload Custom Avatar"
UI->>TauriDialog: open() - File Picker (images only)
TauriDialog->>FileSystem: Request file selection
FileSystem-->>TauriDialog: User selects image file
TauriDialog-->>UI: Return file path
UI->>AssetConverter: convertFileSrc(filePath)
AssetConverter-->>UI: Return asset URL
UI->>State: Store preview URL & file path
State-->>UI: Update customAvatarPreview state
UI-->>User: Display preview image in button
alt Predefined Avatar Selected
User->>UI: Click predefined avatar
UI->>State: Clear customAvatarPreview
State-->>UI: Reset to null
UI-->>User: Remove custom preview
end
alt File Selection Error
FileSystem-->>TauriDialog: Selection failed
TauriDialog-->>UI: Error response
UI->>UI: catch block - log error
UI-->>User: Show error dialog
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (2)
frontend/src/components/Dialog/FaceSearchDialog.tsx (1)
89-90: Consider moving navigation after successful file selection.The navigation to home happens before the user picks a file. If the user cancels the file picker dialog, they're already navigated away from their current page. Consider moving
navigate()to line 92, afterfilePathis confirmed to be truthy.Apply this diff to improve the user flow:
try { - navigate(`/${ROUTES.HOME}`); const filePath = await pickSingleFile(); if (filePath) { + navigate(`/${ROUTES.HOME}`); setIsDialogOpen(false); dispatch(startSearch(filePath));frontend/src/components/OnboardingSteps/AvatarSelectionStep.tsx (1)
59-82: Good error handling structure, but consider user feedback.The try/catch prevents crashes, which is good. However, the error is only logged to console (line 79). While the comment on line 80 acknowledges this is optional, users won't know why their upload failed.
Consider dispatching an error dialog similar to other components in this PR:
} catch (error) { console.error('Error selecting custom avatar:', error); - // Optionally show user feedback here + dispatch( + showInfoDialog({ + title: 'Avatar Upload Error', + message: 'Failed to select custom avatar. Please try again.', + variant: 'error', + }), + ); }You'll need to import
showInfoDialogfrom@/features/infoDialogSliceat the top of the file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/src/components/Dialog/FaceSearchDialog.tsx(2 hunks)frontend/src/components/Navigation/Navbar/Navbar.tsx(3 hunks)frontend/src/components/OnboardingSteps/AvatarSelectionStep.tsx(4 hunks)frontend/src/pages/SettingsPage/components/ApplicationControlsCard.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/components/Dialog/FaceSearchDialog.tsx (4)
frontend/src/constants/routes.ts (1)
ROUTES(1-12)frontend/src/features/searchSlice.ts (1)
startSearch(17-20)frontend/src/features/loaderSlice.ts (1)
showLoader(17-20)frontend/src/features/infoDialogSlice.ts (1)
showInfoDialog(16-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Tauri Build Check (ubuntu-22.04)
- GitHub Check: Tauri Build Check (macos-latest, --target aarch64-apple-darwin)
- GitHub Check: Tauri Build Check (windows-latest)
- GitHub Check: Backend Tests
🔇 Additional comments (5)
frontend/src/components/Navigation/Navbar/Navbar.tsx (1)
9-9: LGTM! Clean migration to SPA navigation.The conversion from anchor tags to React Router's
<Link>components is correct and eliminates full page reloads. The import from'react-router'(not'react-router-dom') is appropriate for React Router v7.Also applies to: 24-27, 86-92
frontend/src/pages/SettingsPage/components/ApplicationControlsCard.tsx (1)
10-15: LGTM! Well-structured tooltip implementation.The tooltip additions follow best practices:
- Single
TooltipProviderwraps all tooltips to avoid provider nesting issues- Each button correctly uses
asChildonTooltipTrigger- Tooltip content is clear and descriptive
Also applies to: 137-195
frontend/src/components/Dialog/FaceSearchDialog.tsx (2)
12-17: LGTM! Tooltip implementation is correct.The tooltip structure properly wraps the dialog trigger and provides clear user guidance about the face search feature.
Also applies to: 111-130
88-106: Excellent error handling for file picker failures.The try/catch block with user-facing error dialog prevents crashes from permission errors or dialog failures.
frontend/src/components/OnboardingSteps/AvatarSelectionStep.tsx (1)
21-23: LGTM! Clean UI implementation with proper preview.The custom avatar upload button integrates well with the predefined avatar grid, and the preview functionality provides good user feedback. The state management correctly clears the custom preview when switching to predefined avatars.
Also applies to: 38-40, 50-51, 161-181
| const assetUrl = convertFileSrc(selected); | ||
| setCustomAvatarPreview(assetUrl); | ||
| setLocalAvatar(selected); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check avatar handling in the codebase
echo "=== Checking AvatarSelectionStep.tsx around lines 74-76 ==="
fd -e tsx -e ts | grep -i avatar | head -20Repository: AOSSIE-Org/PictoPy
Length of output: 257
🏁 Script executed:
#!/bin/bash
# Find avatar-related files and check their content
find frontend -type f \( -name "*avatar*" -o -name "*navbar*" -o -name "*Navbar*" \) 2>/dev/null | head -20Repository: AOSSIE-Org/PictoPy
Length of output: 474
🏁 Script executed:
#!/bin/bash
# Look for convertFileSrc usage
rg -n "convertFileSrc" frontend/src --type tsx --type ts 2>/dev/null | head -30Repository: AOSSIE-Org/PictoPy
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Look for localStorage usage with avatars
rg -n -B 3 -A 3 "localStorage.*avatar\|avatar.*localStorage\|setLocalAvatar\|selectedAvatar" frontend/src --type tsx --type ts 2>/dev/null | head -50Repository: AOSSIE-Org/PictoPy
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check Navbar or Navigation component for avatar display
find frontend/src -type f \( -name "*Navbar*" -o -name "*navbar*" -o -name "*Navigation*" \) 2>/dev/nullRepository: AOSSIE-Org/PictoPy
Length of output: 210
🏁 Script executed:
#!/bin/bash
# Read the specific file and check lines 74-76 and 88
echo "=== AvatarSelectionStep.tsx content ==="
wc -l frontend/src/components/OnboardingSteps/AvatarSelectionStep.tsxRepository: AOSSIE-Org/PictoPy
Length of output: 169
🏁 Script executed:
#!/bin/bash
# Read specific lines from AvatarSelectionStep.tsx
sed -n '70,95p' frontend/src/components/OnboardingSteps/AvatarSelectionStep.tsxRepository: AOSSIE-Org/PictoPy
Length of output: 797
🏁 Script executed:
#!/bin/bash
# Read Navbar.tsx to check avatar display
cat frontend/src/components/Navigation/Navbar/Navbar.tsxRepository: AOSSIE-Org/PictoPy
Length of output: 3600
🏁 Script executed:
#!/bin/bash
# Search more broadly for convertFileSrc
rg "convertFileSrc" frontend/ 2>/dev/nullRepository: AOSSIE-Org/PictoPy
Length of output: 1309
🏁 Script executed:
#!/bin/bash
# Search for setLocalAvatar function definition
rg -B 5 -A 10 "setLocalAvatar\|selectedAvatar" frontend/src 2>/dev/nullRepository: AOSSIE-Org/PictoPy
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check what selectedAvatar is and where it comes from
rg -B 10 -A 5 "selectedAvatar\|setLocalAvatar" frontend/src/components/OnboardingSteps/AvatarSelectionStep.tsxRepository: AOSSIE-Org/PictoPy
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check where userAvatar comes from
rg -B 3 "selectAvatar" frontend/src/featuresRepository: AOSSIE-Org/PictoPy
Length of output: 402
🏁 Script executed:
#!/bin/bash
# Check the onboarding state and reducer
cat frontend/src/features/onboardingSlice.tsRepository: AOSSIE-Org/PictoPy
Length of output: 1890
🏁 Script executed:
#!/bin/bash
# Read more of AvatarSelectionStep to understand the full context
cat frontend/src/components/OnboardingSteps/AvatarSelectionStep.tsx | head -100Repository: AOSSIE-Org/PictoPy
Length of output: 2944
🏁 Script executed:
#!/bin/bash
# Check if there are other places where userAvatar is used
rg "userAvatar" frontend/src --type tsx --type tsRepository: AOSSIE-Org/PictoPy
Length of output: 89
🏁 Script executed:
#!/bin/bash
# Check the constants/avatars.ts to understand the predefined avatars
cat frontend/src/constants/avatars.tsRepository: AOSSIE-Org/PictoPy
Length of output: 297
🏁 Script executed:
#!/bin/bash
# Check all places where avatar is used/displayed
rg "userAvatar\|selectedAvatar\|avatar" frontend/src/components --type ts -lRepository: AOSSIE-Org/PictoPy
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check settings page avatar display
find frontend/src -name "*[Ss]etting*" -type fRepository: AOSSIE-Org/PictoPy
Length of output: 211
🏁 Script executed:
#!/bin/bash
# Look for any other img tags displaying avatars
rg -B 2 -A 2 "src=.*avatar\|src=.*Avatar" frontend/src --type tsRepository: AOSSIE-Org/PictoPy
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check account-settings component
cat frontend/src/components/account-settings.tsxRepository: AOSSIE-Org/PictoPy
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check Settings page
cat frontend/src/pages/SettingsPage/Settings.tsxRepository: AOSSIE-Org/PictoPy
Length of output: 831
🏁 Script executed:
#!/bin/bash
# Check UserPreferencesCard to see if avatar is displayed there
cat frontend/src/pages/SettingsPage/components/UserPreferencesCard.tsxRepository: AOSSIE-Org/PictoPy
Length of output: 4114
🏁 Script executed:
#!/bin/bash
# Search all tsx/ts files for img elements that use avatar or userAvatar
rg "src=.*userAvatar\|src=.*selectedAvatar" frontend/src -A 1 -B 1Repository: AOSSIE-Org/PictoPy
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Verify the exact flow: what gets stored in localStorage and then retrieved
rg -B 5 -A 5 "localStorage.getItem.*avatar\|localStorage.setItem.*avatar" frontend/srcRepository: AOSSIE-Org/PictoPy
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check if avatar is used anywhere else besides Navbar
rg "selectAvatar\|userAvatar" frontend/src -B 2 -A 2Repository: AOSSIE-Org/PictoPy
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Search with simpler pattern
grep -r "localStorage.*avatar" frontend/src --include="*.ts" --include="*.tsx"Repository: AOSSIE-Org/PictoPy
Length of output: 534
🏁 Script executed:
#!/bin/bash
# Check the full Navbar code again for how it uses userAvatar
grep -A 5 "userAvatar" frontend/src/components/Navigation/Navbar/Navbar.tsxRepository: AOSSIE-Org/PictoPy
Length of output: 509
🏁 Script executed:
#!/bin/bash
# Verify what happens when selectedAvatar is custom (a file path)
sed -n '1,200p' frontend/src/components/OnboardingSteps/AvatarSelectionStep.tsx | tail -50Repository: AOSSIE-Org/PictoPy
Length of output: 1794
Fix custom avatar display by storing converted URL instead of raw file path.
Line 76 stores the raw file path in selectedAvatar, which gets saved to localStorage and later displayed in Navbar.tsx without convertFileSrc conversion. This causes custom avatars to fail to load. Predefined avatars (like /avatars/avatar1.png) work because they're bundled assets, but custom file paths require asset URL conversion.
Apply this fix:
if (selected && typeof selected === 'string') {
// Convert file path to Tauri asset URL for preview
const assetUrl = convertFileSrc(selected);
setCustomAvatarPreview(assetUrl);
- setLocalAvatar(selected);
+ setLocalAvatar(assetUrl);
}Alternatively, update Navbar.tsx to handle file path conversion:
- src={userAvatar || '/photo1.png'}
+ src={
+ userAvatar?.startsWith('/') && !userAvatar.startsWith('/avatars')
+ ? convertFileSrc(userAvatar)
+ : userAvatar || '/photo1.png'
+ }📝 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.
| const assetUrl = convertFileSrc(selected); | |
| setCustomAvatarPreview(assetUrl); | |
| setLocalAvatar(selected); | |
| const assetUrl = convertFileSrc(selected); | |
| setCustomAvatarPreview(assetUrl); | |
| setLocalAvatar(assetUrl); |
🤖 Prompt for AI Agents
frontend/src/components/OnboardingSteps/AvatarSelectionStep.tsx around lines
74-76: currently you convert the selected custom file to an asset URL for
preview but then store the raw file path in local state/selectedAvatar
(setLocalAvatar(selected)), causing the unconverted path to be persisted and
fail to load later; replace the stored value with the converted assetUrl (i.e.,
call setLocalAvatar(assetUrl) / setSelectedAvatar(assetUrl) so localStorage
receives the converted URL), or as an alternative modify Navbar.tsx to run
convertFileSrc on values loaded from localStorage before rendering.
1 participant
This PR addresses multiple high-priority UX and reliability issues identified by CodeRabbit in #785.
Navbar.tsx
<TooltipContent>→ Face Scanner tooltip now visible on hoverwindow.location.hrefwith React Router (useNavigate) → true SPA behavior, no full reloadFaceSearchDialog.tsx
try/catch→ prevents crashes (e.g., permission errors)aria-label: "Search for similar faces in your photo library"ApplicationControlsCard.tsx
AvatarSelectionStep.tsx
Technical Notes
react-router-domhacksImproves accessibility, reliability, and user confidence, making #785 merge-ready.
Complements PR #785 — does not replace it.
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.