feat(ui): toggle Hi-Res badge + add minimal label in PlayerBar#147
Merged
Conversation
Two related UI bumps: - New `useHiResBadgeVisibility` hook backed by `profile_setting['ui.show_hi_res_badge']` (default ON). All three HiResBadge variants now skip render when the flag is off, so the toggle is a one-line global hide for users who find the audiophile chrome noisy. Per-profile so a kid's profile can stay clean while the audiophile profile keeps the pills. - New `text` variant on HiResBadge — Spotify-style minimal green uppercase text, no pill background. Used under the artist name in the PlayerBar metadata stack so users instantly know whether the current track is being served at Hi-Res 24-bit / DSD spec without pinning the audio-quality footer. - New Settings → Appearance card (`HiResBadgeCard`) hosting the toggle, with i18n across all 17 locales. Implementation is intentionally minimal: the visibility hook lives inside HiResBadge so existing call sites stay unchanged (no prop drilling). The pattern mirrors `useWrappedBannerVisibility` — profile setting + window event for refresh + same default-ON shape.
InstaZDLL
added
scope: frontend
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAjout d'un badge Hi-Res/DSD affichable dans le player, contrôlé globalement par ChangesBadge Hi-Res/DSD avec contrôle global
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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 docstrings
🧪 Generate unit tests (beta)
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
🤖 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.
Inline comments:
In `@src/components/common/HiResBadge.tsx`:
- Around line 40-47: The HiResBadge component currently calls
useHiResBadgeVisibility for each instance causing N listeners; remove the local
hook call inside HiResBadge (reference HiResBadge and useHiResBadgeVisibility)
and instead accept a boolean prop visible (e.g. prop name visible) that the
parent provides; implement a single provider/store (e.g.
HiResBadgeVisibilityProvider or a shared hook) at a higher level to subscribe
once to profile_setting and compute visibility, then pass that visible prop into
each HiResBadge instance and remove any per-badge Tauri calls and listener
setup.
In `@src/hooks/useHiResBadgeVisibility.ts`:
- Around line 58-65: La mise à jour optimiste dans la fonction setVisible met à
jour l'état local avant d'écrire avec setProfileSetting, mais en cas d'échec
l'UI ne revient pas en arrière ; modifiez setVisible (et l'utilisation de KEY,
setProfileSetting, HI_RES_BADGE_EVENT et window.dispatchEvent) pour soit
effectuer l setVisibleState(next) après un write réussi, soit conserver la
valeur précédente (capturée avant le changement) et la restaurer dans le bloc
catch (p.ex. setVisibleState(previousValue)) après l'échec afin d'éviter une
divergence entre l'UI et l'état persistant.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 15374187-8139-4434-8582-09d88c399c10
📒 Files selected for processing (23)
docs/features/ui.mdsrc/components/common/HiResBadge.tsxsrc/components/player/PlayerBar.tsxsrc/components/views/SettingsView.tsxsrc/components/views/settings/HiResBadgeCard.tsxsrc/hooks/useHiResBadgeVisibility.tssrc/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/ko.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
Two findings from the PR #147 review: 1. Every mounted HiResBadge called the hook, which fired its own `useEffect` with a Tauri `getProfileSetting` read + a window event listener. On a virtualised library view with ~20-50 badges mounted at once that's 20-50 redundant fetches plus 20-50 listeners for a single boolean. Refactor the hook to a module-level store + `useSyncExternalStore`: the first subscriber kicks off the one-and-only Tauri read, the window listener is attached lazily (and only once), and every `HiResBadge` shares the same `currentVisible` value through cheap `Set<callback>` notifications. Call sites stay unchanged — same public interface, same `visible`/`setVisible` shape. 2. The optimistic `setVisible` flipped the local state before the `setProfileSetting` write and never rolled back on failure, so a rejected Tauri call left the UI showing a value that didn't match what was actually persisted. Capture the previous value before the flip and restore it inside the catch so the visible state tracks the truth on the disk. `refreshHiResBadgeVisibility` is also now exported for callers that need to force a re-read (profile switch — separate follow-up since the original hook had the same limitation, and the no-op cost of the new cache between switches is benign).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@src/hooks/useHiResBadgeVisibility.ts`:
- Line 126: The call to window.dispatchEvent in setVisible can throw in non-DOM
environments (SSR/tests) because the earlier guard only checked
addEventListener; update setVisible to first ensure a DOM-backed dispatch is
available (e.g., verify typeof window !== 'undefined' && typeof
window.dispatchEvent === 'function' or wrap the call in a safe try/catch) before
invoking window.dispatchEvent(new CustomEvent(HI_RES_BADGE_EVENT)); reference
the setVisible function and the HI_RES_BADGE_EVENT symbol when making the change
so the dispatch is properly protected.
- Around line 42-49: hydrateFromBackend can apply stale async results over newer
state and calls window.dispatchEvent without SSR/test guards; fix by
implementing a latest-read-wins guard: before awaiting getProfileSetting(KEY)
capture/advance a request token (e.g., a local incrementing id or timestamp
stored on the module) and after parseVisible only set currentVisible and call
notify() if the token matches the module's latest token, referencing
hydrateFromBackend, currentVisible, getProfileSetting, parseVisible, notify and
HI_RES_BADGE_EVENT; also wrap any window.dispatchEvent(...) calls with a guard
(typeof window !== "undefined") similar to ensureWindowListener to avoid crashes
in non-window environments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 300bc820-408a-4d4b-8462-68f84c399cc1
📒 Files selected for processing (1)
src/hooks/useHiResBadgeVisibility.ts
Two findings from the latest review: - `hydrateFromBackend` could apply a stale `getProfileSetting` reply if two reads raced (e.g. the settings toggle dispatching the window event right next to a profile-switch `refreshHiResBadgeVisibility`). Capture a monotonic token at the top of the function and bail when a newer read has already superseded the in-flight one before committing to `currentVisible`. - `window.dispatchEvent` in `setVisible` was unguarded, so a non-DOM caller (Node tests, hypothetical future SSR) would crash. Wrap with the same `typeof window !== "undefined"` check that `ensureWindowListener` already uses. Skipped finding: the CodeRabbit "fan-out coûteux" pass ran on the pre-`efa19bb` revision where each badge held its own listener + Tauri call. The store-with-useSyncExternalStore refactor already collapses that to one fetch + one listener total, so prop-drilling a `visible` flag would add noise without any measurable win.
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.
What
Two related UX bumps around the Hi-Res / DSD badge:
1. Settings toggle to hide the badge globally
New per-profile setting `ui.show_hi_res_badge` (default on). When flipped off, every mounted `HiResBadge` returns `null` — list rows, album / artist grid overlays, and the new player-bar label collapse in one render. Per-profile so a kid's profile can stay clean while the audiophile profile keeps the pills.
UI lives in `HiResBadgeCard` under Settings → Appearance, next to `WrappedBannerCard`. Backed by `useHiResBadgeVisibility` which mirrors the pattern used by `useWrappedBannerVisibility`: profile setting + window event for cross-mount refresh.
2. Minimal green label under the artist in the PlayerBar
Third variant added to `HiResBadge` — `variant="text"`. No pill background, just a tiny uppercase green text block that nests cleanly into the player-bar metadata column:
Renders for Hi-Res (≥ 24-bit, ≥ 44.1 kHz) and DSD tracks; renders `null` for lossy / 16-bit lossless / when the user has hidden the badge.
Wiring
Out of scope
The existing two variants (`overlay`, `inline`) keep their visuals unchanged so no track list / album grid is visually disturbed.
Test plan
Summary by CodeRabbit
Nouvelles fonctionnalités
Internationalisation
Documentation