fix: post-release UX cleanup (lyrics feedback, version labels, dead button)#44
Merged
Conversation
github-actions
Bot
added
scope: frontend
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (21)
📝 WalkthroughWalkthroughCe PR améliore l'expérience utilisateur du préchargement des paroles en envoyant un événement de progression initial côté backend, affichant des résultats détaillés en frontend avec traductions multilingues. Il inclut aussi une simplification du flux d'import en bibliothèque et une mise à jour des versions de dépendances dans la page À propos. ChangesPréchargement des paroles et mises à jour connexes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/views/SettingsView.tsx (2)
741-746:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winÉviter qu’un ancien timeout efface une exécution en cours
Si l’utilisateur relance vite, le timeout précédent peut réinitialiser l’état pendant la nouvelle exécution. Annule le timer actif avant d’en créer un nouveau.
💡 Correctif proposé
+ const lyricsResultTimerRef = useRef<number | null>(null); const handlePrefetchLyrics = async () => { if (isPrefetchingLyrics) return; + if (lyricsResultTimerRef.current != null) { + window.clearTimeout(lyricsResultTimerRef.current); + lyricsResultTimerRef.current = null; + } setIsPrefetchingLyrics(true); setLyricsPrefetchProgress(null); setLyricsResultMsg(null); @@ } finally { setIsPrefetchingLyrics(false); - window.setTimeout(() => { + lyricsResultTimerRef.current = window.setTimeout(() => { setLyricsPrefetchProgress(null); setLyricsResultMsg(null); + lyricsResultTimerRef.current = null; }, 5000); } }; + + useEffect(() => { + return () => { + if (lyricsResultTimerRef.current != null) { + window.clearTimeout(lyricsResultTimerRef.current); + } + }; + }, []);Also applies to: 768-771
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/views/SettingsView.tsx` around lines 741 - 746, The prefetch handlers (e.g., handlePrefetchLyrics) can have an earlier setTimeout clear the state of a new run; introduce and use a persistent timer holder (a ref or module-scope variable) to clear the previous timeout before creating a new one: call clearTimeout(existingTimer) and replace it with the new setTimeout id, and ensure you clear it when the operation completes or on unmount; apply the same pattern to the other handler referenced (lines 768-771) so any in-flight timeout is cancelled before starting a new prefetch.
711-765:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNe pas afficher les erreurs “offline/failed” avec un style succès
Le rendu est toujours en vert dès que
lyricsResultMsgexiste, y compris pour les erreurs. Distingue succès/erreur dans l’état pour éviter un feedback trompeur.💡 Correctif proposé
- const [lyricsResultMsg, setLyricsResultMsg] = useState<string | null>(null); + const [lyricsResult, setLyricsResult] = useState<{ + kind: "success" | "error"; + message: string; + } | null>(null); @@ - setLyricsResultMsg(null); + setLyricsResult(null); @@ - setLyricsResultMsg( - t("settings.lyricsPrefetch.result", { + setLyricsResult({ + kind: "success", + message: t("settings.lyricsPrefetch.result", { hits: summary.hits, misses: summary.misses, failed: summary.failed, }), - ); + }); @@ - setLyricsResultMsg( - msg.includes("offline") - ? t("settings.lyricsPrefetch.offline") - : t("settings.lyricsPrefetch.failed"), - ); + setLyricsResult({ + kind: "error", + message: msg.includes("offline") + ? t("settings.lyricsPrefetch.offline") + : t("settings.lyricsPrefetch.failed"), + }); @@ - setLyricsResultMsg(null); + setLyricsResult(null); @@ - ) : lyricsResultMsg ? ( - <div className="text-xs text-emerald-600 dark:text-emerald-400 mt-1 truncate"> - {lyricsResultMsg} + ) : lyricsResult ? ( + <div + className={`text-xs mt-1 truncate ${ + lyricsResult.kind === "success" + ? "text-emerald-600 dark:text-emerald-400" + : "text-rose-600 dark:text-rose-400" + }`} + > + {lyricsResult.message} </div>Also applies to: 2487-2490
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/views/SettingsView.tsx` around lines 711 - 765, The UI currently treats any non-null lyricsResultMsg as a success (green) even for errors; add a separate status flag (e.g., lyricsResultStatus or isLyricsPrefetchError) alongside lyricsResultMsg to represent success vs error, set it to success when prefetchLibraryLyrics() returns (before calling setLyricsResultMsg) and set it to error in the catch block (alongside setting the error message), and update the render code that styles the message to use this new flag instead of merely checking lyricsResultMsg; apply the same change for the other occurrence that sets lyrics result messages (the block around the other setLyricsResultMsg usage referenced in the review).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/components/views/SettingsView.tsx`:
- Around line 741-746: The prefetch handlers (e.g., handlePrefetchLyrics) can
have an earlier setTimeout clear the state of a new run; introduce and use a
persistent timer holder (a ref or module-scope variable) to clear the previous
timeout before creating a new one: call clearTimeout(existingTimer) and replace
it with the new setTimeout id, and ensure you clear it when the operation
completes or on unmount; apply the same pattern to the other handler referenced
(lines 768-771) so any in-flight timeout is cancelled before starting a new
prefetch.
- Around line 711-765: The UI currently treats any non-null lyricsResultMsg as a
success (green) even for errors; add a separate status flag (e.g.,
lyricsResultStatus or isLyricsPrefetchError) alongside lyricsResultMsg to
represent success vs error, set it to success when prefetchLibraryLyrics()
returns (before calling setLyricsResultMsg) and set it to error in the catch
block (alongside setting the error message), and update the render code that
styles the message to use this new flag instead of merely checking
lyricsResultMsg; apply the same change for the other occurrence that sets lyrics
result messages (the block around the other setLyricsResultMsg usage referenced
in the review).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 20535902-00b2-4ba9-9fc3-fa2ceabae570
📒 Files selected for processing (21)
src-tauri/src/commands/lyrics.rssrc/components/views/AboutView.tsxsrc/components/views/LibraryView.tsxsrc/components/views/SettingsView.tsxsrc/i18n/locales/ar.jsonsrc/i18n/locales/de.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/hi.jsonsrc/i18n/locales/id.jsonsrc/i18n/locales/it.jsonsrc/i18n/locales/ja.jsonsrc/i18n/locales/kr.jsonsrc/i18n/locales/nl.jsonsrc/i18n/locales/pt-BR.jsonsrc/i18n/locales/pt.jsonsrc/i18n/locales/ru.jsonsrc/i18n/locales/tr.jsonsrc/i18n/locales/zh-CN.jsonsrc/i18n/locales/zh-TW.json
Same silent-success bug as the missing-covers batch action: when
every track already has cached lyrics (or the user is in offline
mode), the backend returned silently and the UI never moved —
the button looked dead.
- backend: emit an initial `lyrics:prefetch-progress` event with
total=N (including N=0) so the UI can show "nothing to do"
instead of staying frozen on the subtitle.
- frontend: capture the returned summary and show a short result
message in the row subtitle for 5s ("X synced, Y not found,
Z failed") or an explicit "Offline mode is enabled" / "Prefetch
failed" message on error.
- i18n: new `settings.lyricsPrefetch.result` / `.offline` /
`.failed` keys propagated to all 17 locales.
Frontend section was showing stale major versions that hadn't been bumped when the underlying deps were upgraded: - Vite: 6.x → 8.x (actual: ^8.0.13 in package.json) - TypeScript: 5.x → 6.x (actual: ~6.0.3 in package.json)
The "Import files" button rendered next to "Import folder" in the empty library state had no onClick handler — clicking it did nothing. The whole library architecture (`library_folder` table, filesystem watcher, periodic rescans) is folder-based, so importing isolated files would create orphan rows the scanner can't manage. Drop the button entirely and keep the working "Import folder" one, promoted to the primary emerald style since it's now the only call to action. Remove the now-unused UploadIcon import. The matching `library.actions.importFiles` i18n keys are kept in the locale files for now (cheap, and a future per-file import flow would reuse them).
InstaZDLL
force-pushed
the
fix/post-release-bugs-1.1.x
branch
from
a26dca1 to
535e41c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three small UX fixes reported after the v1.1.1 release:
package.json.Known follow-ups (parked, not in this PR)
User also reported during the same session — both need real-machine reproduction logs since the code paths look correct on inspection:
isLoadedto avoid flash). Possible cause: WAL not synced at hard shutdown; needstracinginstrumentation onset_profile_settingto confirm.lastfm_get_statusreadsauth_credential.username;lastfm_loginwritessession.username(always aString, never NULL). Needs DB inspection (SELECT * FROM auth_credential WHERE provider='lastfm') on the user's profile to see whether the row is present and what the username column looks like.Will spin those up as separate issues / PRs once we have repro data.
Test plan
bun run typecheckcleanbun run lintcleanSummary by CodeRabbit
Notes de version
New Features
Improvements
Localization