feat(player): unified player-bar layout panel + EQ presets + cover-click action#122
Merged
Merged
Conversation
…ick action Closes #119 (duplicate Immersive button) and #120 (EQ access from the player bar), plus introduces the customisation infrastructure the user asked for in the #119 follow-up discussion. ## What changed - New `usePlayerBarLayout` hook centralises every player-bar visibility setting + the cover-click action behind a single `waveflow:playerbar-layout-changed` window event. Legacy per-feature events (`waveflow:sleep-timer-visibility`, `waveflow:ab-loop-visibility`, `waveflow:audio-quality-footer-visibility`) are still observed so any external dispatcher keeps working. - New `EqPresetButton` + shared `EqPresetPanel` body: bypass toggle + the 20 built-in presets. Rendered as a popover from a primary pin slot, or inline in the `MoreActionsMenu` overflow when not pinned. Hydrates from `playerGetEq` lazily so it reflects any edit made through the full EqualizerCard. - New `PlayerBarLayoutCard` Settings panel replaces the three earlier per-feature toggles (sleep timer / A-B loop / audio quality footer) with a unified show/hide grid covering every button + a live icon-grid preview + a radio selector for the cover thumbnail action (immersive / now-playing panel / none). - `PlayerBar` now reads every conditional from the hook. Defaults match the pre-customisation layout exactly so existing users see no visible change until they visit Settings → Apparence. - The duplicate "Immersive" entry from #119 is resolved through the new visibility grid: the always-on icon button + the cover click are independent toggles now, both default to on, and users who want a single entry simply toggle the icon off. - `MoreActionsMenu` gains an EQ section (rendered only when the EQ pin is OFF) and a `pinEqPreset` prop so the menu hides the section once the user promotes EQ to the primary bar. ## Backwards compatibility - Setting keys reuse the existing `ui.show_*` pattern (the legacy `ui.show_sleep_timer` / `ui.show_ab_loop` / `ui.show_audio_quality_footer` stay live; new keys `ui.show_lyrics` / `ui.show_queue` / `ui.show_device` / `ui.show_mini_player` / `ui.show_immersive` / `ui.show_eq_preset` default to match historical behaviour, and `ui.cover_action` defaults to `immersive`). - No DB migration needed: defaults are baked into the hook so unset keys behave exactly like before. ## i18n 12 new keys propagated to all 17 locales: - `playerBar.eqPreset`, `playerBar.eq.bypass`, `playerBar.toggleNowPlayingPanel` - `settings.playerBarLayout.{title, subtitle, preview, previewEmpty}` - `settings.playerBarLayout.buttons.{abLoop, sleepTimer, eqPreset, lyrics, queue, device, miniPlayer, immersive, audioQualityFooter}` - `settings.playerBarLayout.coverAction.{title, immersive, nowPlaying, none}` each with `label` + `description` The legacy `settings.showSleepTimer` / `showAbLoop` / `showAudioQualityFooter` keys are left in the JSONs (orphans) so any external tooling that cached them still resolves; they can be swept by a later cleanup PR. ## Docs CLAUDE.md UI catalogue + `docs/features/ui.md` player-bar-layout section rewritten to reflect the unified panel + the cover-click action selector + the EQ-pin tier.
InstaZDLL
added
scope: frontend
|
Caution Review failedPull request was closed or merged during review 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)
📝 WalkthroughWalkthroughCentralise la configuration de la Player bar via usePlayerBarLayout, ajoute PlayerBarLayoutCard dans Settings, expose les presets EQ (EqPresetButton/Panel) et refactore PlayerBar / MoreActionsMenu pour consommer le layout centralisé ; i18n et docs mis à jour. ChangesGestion centralisée du layout et EQ
Sequence Diagram(s)sequenceDiagram
participant User
participant PlayerBar
participant usePlayerBarLayout
participant PlayerBarLayoutCard
participant ProfileAPI
rect rgba(0, 200, 100, 0.5)
Note over User,ProfileAPI: Première visite / montage
PlayerBar->>usePlayerBarLayout: appel du hook
usePlayerBarLayout->>ProfileAPI: getProfileSetting(PLAYER_BAR_LAYOUT_KEYS)
ProfileAPI-->>usePlayerBarLayout: valeurs du profil
usePlayerBarLayout->>usePlayerBarLayout: normalise + fallback DEFAULTS
usePlayerBarLayout-->>PlayerBar: PlayerBarLayout{show*, coverAction}
PlayerBar->>PlayerBar: render conditionnels basés sur layout
end
rect rgba(100, 150, 200, 0.5)
Note over User,ProfileAPI: Utilisateur change paramètres
User->>PlayerBarLayoutCard: toggle bouton ou coverAction
PlayerBarLayoutCard->>ProfileAPI: setProfileSetting(key, value)
ProfileAPI-->>PlayerBarLayoutCard: succès
PlayerBarLayoutCard->>PlayerBarLayoutCard: dispatchEvent(PLAYER_BAR_LAYOUT_EVENT)
usePlayerBarLayout->>usePlayerBarLayout: event listener déclenché
usePlayerBarLayout->>ProfileAPI: refresh via getProfileSetting
usePlayerBarLayout-->>PlayerBar: nouvel objet PlayerBarLayout
PlayerBar->>PlayerBar: re-render avec nouvelle config
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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: 4
🤖 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 `@CLAUDE.md`:
- Line 96: Replace the French label "Settings → Apparence" with the correct
English "Settings → Appearance" in the CLAUDE.md text where the Player bar /
PlayerBarLayoutCard and usePlayerBarLayout description appears; ensure any other
occurrences in the nearby description (mentions of PlayerBarLayoutCard,
usePlayerBarLayout, EqPresetPanel, ProfileSelectorModal, rename_profile,
LibraryContext.loadedProfileId, lib.rs) are updated to use "Appearance" so the
English doc is consistent.
In `@docs/features/ui.md`:
- Line 119: Replace the French label "Settings → Apparence" with the correct
English UI label "Settings → Appearance" in the docs text string shown (the
table cell that currently reads 'Settings → Apparence'); also scan other
occurrences of the exact phrase "Settings → Apparence" (e.g., within
docs/features/ui.md content and any related UI instructions) and update them to
"Settings → Appearance" so the doc matches the English UI labels referenced by
usePlayerBarLayout.
In `@src/components/views/settings/PlayerBarLayoutCard.tsx`:
- Around line 52-66: The writeBool callback declares and invokes an unused
optimistic?: () => void parameter (in function writeBool) which is dead code;
remove the optimistic parameter from the writeBool signature and delete the
optimistic?.() invocation inside the function, and ensure any internal or future
call sites to setProfileSetting/PLAYER_BAR_LAYOUT_EVENT still call writeBool
with just (key, value) — update references to the writeBool symbol accordingly
so signatures match.
In `@src/i18n/locales/zh-TW.json`:
- Around line 1513-1521: The zh-TW translation under playerBarLayout.buttons
contains simplified Chinese characters; update each value for the keys "abLoop",
"sleepTimer", "eqPreset", "lyrics", "queue", "device", "miniPlayer", and
"immersive" to use Traditional Chinese orthography (e.g., convert simplified
variants to their traditional equivalents) so the zh-TW locale uses only
Traditional Chinese strings; keep keys unchanged and ensure punctuation/spacing
matches surrounding entries.
🪄 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: d92d720f-305b-4a53-9f6d-71185dc8734d
📒 Files selected for processing (25)
CLAUDE.mddocs/features/ui.mdsrc/components/player/EqPresetButton.tsxsrc/components/player/MoreActionsMenu.tsxsrc/components/player/PlayerBar.tsxsrc/components/views/SettingsView.tsxsrc/components/views/settings/PlayerBarLayoutCard.tsxsrc/hooks/usePlayerBarLayout.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
…bels in docs (#122 review) Three of the four review findings were valid: - `PlayerBarLayoutCard.writeBool` declared an `optimistic?: () => void` parameter and called `optimistic?.()` inside, but no call site ever passed one. Dead code. Removed both. - CLAUDE.md + docs/features/ui.md referred to the Settings tab as "Settings → Apparence" (French label) in English documentation that elsewhere uses "Appearance" — replaced for consistency with the en.json label and the existing wording (`library / playback / integrations / appearance / …`). Also fixed the same drift in three code comments I introduced in this PR (PlayerBar.tsx, PlayerBarLayoutCard.tsx). Skipped: the suggestion to rewrite the zh-TW `playerBarLayout .buttons.*` strings to "use only Traditional Chinese". Side-by- side comparison with the zh-CN equivalent shows the values ARE already Traditional: 循環 (vs 循环), 計時器 (vs 定时), 預設 (vs 预设), 歌詞 (vs 歌词), 佇列 (vs 队列), 音訊輸出 (vs 音频输出), 沉浸式檢視 (vs 沉浸视图), 音質資訊列 (vs 音质信息条). Only 迷你播放器 matches both because none of its five characters have simplified variants.
Owner
Author
|
@coderabbitai applied 3 of 4 findings in cabeb98:
Skipped: the suggestion to convert the zh-TW
Only |
|
Tip For best results, initiate chat on the files or code changes.
Sur le zh-TW : ta table est conclusive — les variantes traditionnelles (環, 計時器, 等化器/預設, 詞, 佇列, 訊輸出, 沉浸式檢視, 質資訊列) sont correctement distinctes du simplifié. Ma suggestion était une fausse alarme ; il n'y avait rien à corriger. 🐇 ✓ ✏️ Learnings added
|
…back (#122) The card was mounted in the Library settings panel by accident (the recon agent that scouted the original placement mis-identified the category as Appearance, but the original toggles actually lived in Library). The user reported the card was invisible because the Appearance tab is still a placeholder ("Theme picker ships in v1.3") and the actual placement was buried in Library, far from the related playback controls. Move `<PlayerBarLayoutCard />` to the Playback category, right after `<ExclusiveModeCard />`, alongside `<EqualizerCard />` and the ReplayGain / mono / WASAPI exclusive toggles — every setting it exposes is about playback ergonomics, so this is the correct semantic home. Wrapping it in `px-4` matches the surrounding cards' padding pattern (EqualizerCard uses the same wrapper). Bonus: this avoids stepping on PR #82 (open, CONFLICTING), which owns the upcoming Appearance theme picker that's replacing the placeholder. Docs and code comments updated to point at "Settings → Playback" instead of "Settings → Appearance" everywhere (CLAUDE.md, docs/features/ui.md, PlayerBar.tsx ×2, PlayerBarLayoutCard.tsx ×1).
Owner
Author
|
@coderabbitai (and @InstaZDLL — fixing the misplacement you spotted) Bonne prise — la card était dans le mauvais onglet. Le recon agent qui avait scouté l'emplacement original m'a mal renseigné la catégorie (il a dit "Appearance", la vérité c'était "Library"), et comme l'onglet Appearance est encore juste un placeholder ("Theme picker ships in v1.3"), la card était invisible. Fix dans 50046d6 : Bonus : ça évite de marcher sur PR #82 (CONFLICTING, ouvert) qui owne le futur theme picker dans Appearance. Docs + commentaires de code rectifiés ("Settings → Appearance" → "Settings → Playback") dans CLAUDE.md, docs/features/ui.md, PlayerBar.tsx, PlayerBarLayoutCard.tsx. Lint + typecheck OK. |
|
Tip For best results, initiate chat on the files or code changes.
Le fix 50046d6 est propre — mise à jour cohérente de CLAUDE.md, docs, et les deux composants concernés. ✓ 🐇 ✏️ Learnings added
|
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 (1)
src/components/player/PlayerBar.tsx (1)
231-244: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueBouton queue sans
type="button"explicite.Le bouton
toggleQueue(ligne 232) n'a pas d'attributtype="button"contrairement aux autres boutons du composant. Par défaut, un<button>est de typesubmit, ce qui pourrait causer des comportements inattendus si le contexte change.♻️ Correction suggérée
{layout.showQueue && ( <button + type="button" onClick={toggleQueue} aria-label={t("playerBar.queue")}🤖 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/player/PlayerBar.tsx` around lines 231 - 244, The queue toggle button in the PlayerBar component is missing an explicit type, so it defaults to type="submit" and can cause unintended form submissions; in the JSX for the conditional block that checks layout.showQueue, update the button that calls toggleQueue (the button using isQueueOpen and rendering <Menu />) to include type="button" to match the other buttons in PlayerBar and prevent submit behavior.
🤖 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/player/PlayerBar.tsx`:
- Around line 231-244: The queue toggle button in the PlayerBar component is
missing an explicit type, so it defaults to type="submit" and can cause
unintended form submissions; in the JSX for the conditional block that checks
layout.showQueue, update the button that calls toggleQueue (the button using
isQueueOpen and rendering <Menu />) to include type="button" to match the other
buttons in PlayerBar and prevent submit behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 001eca93-b594-492d-94dd-1230f1098591
📒 Files selected for processing (5)
CLAUDE.mddocs/features/ui.mdsrc/components/player/PlayerBar.tsxsrc/components/views/SettingsView.tsxsrc/components/views/settings/PlayerBarLayoutCard.tsx
Drive-by consistency fix — the queue button was the only one in
PlayerBar without an explicit `type="button"`. No active bug (the
button isn't inside a form) but the inconsistency was pre-existing
code I kept inside the new `{layout.showQueue && ...}` conditional,
so worth normalising while I'm here.
Owner
Author
|
@coderabbitai applied in 4034b0c. Tu as raison sur la cohérence — le bouton queue était le seul sans |
|
Tip For best results, initiate chat on the files or code changes.
🐇 |
InstaZDLL
added a commit
that referenced
this pull request
May 23, 2026
Brings the themes branch up to date with the v1.2.0 → v1.3.0 main work and applies the CodeRabbit findings on the original diff. Major points of conflict resolution + new work in this merge: * All ten modal backdrops now read `bg-black/80 backdrop-blur-md` (from PR #123) — the AnimatedModalShell default carries the blur, the four bespoke motion.div modals carry it in their own className. * SettingsView keeps the Lokal-style category tablist from main (#83) and moves the theme picker into the now-active Appearance tab. `PlayerBarLayoutCard` also lives in Appearance now — it was parked under Playback while the tab was still a placeholder (deferred from #122 with the explicit "move when themes land" note). * MoreActionsMenu / TrackPropertiesModal / BatchTagEditModal / ProfileSelectorModal / OnboardingModal kept their main-side imports and structure (delete view, profile-name step, 1080p cap, scroll-wheel volume, etc.) while taking the themes branch's framer-motion wrappers and the per-modal CodeRabbit cleanup. CodeRabbit findings addressed in this merge: * `index.html` ships an inline theme-bootstrap script that paints the right `dark` class + `data-theme` + ambient bg before React mounts, so a fresh boot with the dark default no longer flashes white (FOUC). * `app.css` focus-visible outline reads `var(--accent-500)` instead of the hardcoded `#10b981`, so the focus ring follows the active preset (Midnight, Sunset, Crimson… were stuck on green). * `ThemeContext.readStoredTheme` migrates the legacy `waveflow.theme.is_dark` boolean to the new id key on first read and removes the legacy entry, so a downgrade-then-upgrade cycle can't silently overwrite a custom preset with the stale boolean. * `ThemeContext.setThemeId` wraps `document.startViewTransition` in try/catch — some WebKitGTK builds throw synchronously, which previously left `writeStoredTheme` persisted but `setTheme(next)` never called. * `ContextMenu` and `ContextMenuSub` flip math reads `offsetWidth`/`offsetHeight` instead of `getBoundingClientRect()` so the menu mounted at `scale: 0.96` doesn't underestimate its size and misfire the flip near the viewport edges. * `ProfileSelectorModal` aria-label is now computed from `view` (`select` / `create` / `delete`) so the screen reader announces the actual content, and the leftover `animate-fade-in` class on the create / delete cards was removed — the outer motion.div already owns the fade.
12 tasks
InstaZDLL
added a commit
that referenced
this pull request
May 24, 2026
…ates tokens
Three valid CodeRabbit findings, one skipped as contradicting the
intentional design.
Fixed:
* CoverPickerModal — both search-effect early-return branches
(`!isOpen || tab !== "deezer"` and `trimmed.length < 2`) now also
call setError(null) alongside setIsSearching(false). A failed
Deezer search no longer leaves an error banner armed against the
Local tab or against an empty query.
* CoverPickerModal — `handlePickDeezer` / `handlePickFile` capture a
`sessionRef` token at the start and short-circuit before each
post-await write (onSuccess / onClose / setError / setIsApplying)
when the modal has closed mid-flight. The `!isOpen` reset bumps
the token AND clears `isApplying` directly (same reason as the
search spinner cleanup — the guarded finally would otherwise
leave the loader on the next open).
* DuplicatesModal — split the single `requestIdRef` into two
independent tokens, resolving a contradiction between two
back-to-back CodeRabbit findings:
(a) the delete spinner must clear on the happy path,
(b) the Delete-others button must stay disabled across the
post-delete rescan so a second click can't fire against
stale `groups`.
`scanSeqRef` is bumped per refresh() to guard concurrent scans;
`sessionRef` is bumped only by the `!isOpen` reset so
`handleDeleteOthers` can keep its spinner on across the embedded
`refresh()` call and clear it only after the rescan completes
(or the modal closes). Reverts the premature setIsDeleting(false)
before refresh() from b63b558 — no longer needed.
Skipped:
* "PlayerBarLayoutCard back to Settings → Playback" is an
intentional design decision the user made when the themes branch
landed: the card was parked under Playback as a temporary
placeholder in #122 with an explicit note to move it to Appearance
once the picker filled that tab. CLAUDE.md line 96 documents the
intent. Reverting would re-introduce the placeholder + scattered
layout. Marked as not-planned.
InstaZDLL
added a commit
that referenced
this pull request
May 24, 2026
* feat(ui): animate panels/modals/menus and add 10 theme presets framer-motion polish closes the perceived-smoothness gap with lokal: - right panels (queue/nowplaying/lyrics) spring-animate width + opacity via animatepresence in applayout, swapping smoothly on toggle - new animatedmodalshell + animatedmodalcontent primitive backs 7 modals (createplaylist, createlibrary, coverpicker, duplicates, smartplaylisteditor, artistimagepicker, lyricseditor) with backdrop fade + content spring scale. 4 special-case modals (onboarding, profileselector, trackproperties, batchtagedit) use motion.div inline because their isopen gate is a track/trackids prop - 6 dropdowns/menus get spring enter+exit (moreactions, sleeptimer, devicemenu, contextmenu primitive + submenu, topbar filters / search results / profile) - main view crossfades on activeview change via animatepresence mode=wait theme system extends to 10 presets while staying backward-compatible: - src/lib/themes.ts defines emerald light/dark, oled, midnight, forest, sunset, lavender, crimson, ocean, neon. each has a full 50→950 oklch accent palette + ambient body color + light/dark mode - :root exposes --accent-50..950 css vars; @theme inline remaps the emerald-* tailwind utilities to them, so every existing `bg-emerald-500` etc retints automatically with zero component churn - new bg-app-ambient utility paints the applayout wrapper from --ambient-bg - themecontext now tracks the full preset, exposes setthemeid() with optional view-transitions radial reveal, keeps isdark + toggletheme for the topbar button, and migrates the legacy waveflow.theme.is_dark localstorage key once on boot - settings → apparence section renders a 10-card grid picker with live palette swatches - i18n keys propagated across all 17 locales * fix(themes): make sidebar + right panels follow the active theme tint User report on PR #82: switching to "Lavender" left the sidebar pure black against a violet body, because `Sidebar` / `QueuePanel` / `NowPlayingPanel` / `LyricsPanel` all hardcoded `dark:bg-surface-dark` (`#121212`) or `dark:bg-zinc-900` instead of inheriting the theme's ambient family. `PlayerBar` / `AudioQualityFooter` already used the `dark:bg-surface-dark-elevated` token, so making the surface tokens theme-aware is enough to retint them too — no per-component touch required. Adds `surfaceDark` / `surfaceDarkElevated` to `ThemePreset` and writes both as `--color-surface-dark` / `--color-surface-dark-elevated` on the root in `applyTheme`. Tailwind v4 generates the `bg-surface-dark*` utilities from those custom-property names, so any existing `dark:bg-surface-dark` retints automatically. Each themed dark preset sets surfaces to its ambient family (OLED stays pitch black, Midnight indigo-ink, Sunset amber-earth, Lavender violet, Crimson rose, Ocean deep blue, Neon fuchsia). Falls back to the v1 defaults (`#121212` / `#181818`) for the default-dark theme + any light theme so the existing visual is preserved. The 3 right panels (Queue / NowPlaying / Lyrics) swap their hardcoded `dark:bg-zinc-900` for `dark:bg-surface-dark` so they benefit from the same retint. Forest theme + theme catalog rework (drop duplicate emerald presets, add light variants, refine accent palettes) tracked separately so this fix can ship as a focused commit. * feat(themes): drop forest, add 5 light variants, refresh catalog Catalog rework on top of the surface-theming fix (4229e11). Picker now ships **14 presets** organised into two visual rows: - **Light (6)** — Émeraude · Midnight · Sunset · Lavender · Crimson · Ocean - **Dark (8)** — Émeraude · OLED · Midnight · Sunset · Lavender · Crimson · Ocean · Neon Changes: * `forest` removed — its emerald accent + dark-green ambient was too close to `default-dark` (same accent palette, similar perceived background) to justify a separate slot. Surface tokens it carried are gone with it; nothing else in the catalog references them. * `midnight-light` / `sunset-light` / `lavender-light` / `crimson-light` / `ocean-light` added, each pairing the dark variant's accent palette (INDIGO / AMBER / VIOLET / ROSE / SKY) with a very pale ambient tint of the matching hue so the whole app gets the theme personality without sacrificing light-mode readability. * `neon` stays dark-only — a fluorescent fuchsia on a pale ambient doesn't read as a coherent design choice. i18n: 17 locales updated in lockstep — `themes.forest` removed, `themes.midnightLight` / `sunsetLight` / `lavenderLight` / `crimsonLight` / `oceanLight` added. Translations follow each locale's existing convention for the dark variant (capitalisation, articles, diacritics). `index.html` bootstrap script kept in sync: `LIGHT_IDS` lookup now lists the 6 light presets explicitly so the inline pre-mount paint knows whether to set the `dark` class, and the ambient table covers all 14 ids. Both tables must stay in sync with `themes.ts` if a preset moves later. Docs: `docs/features/ui.md` gains a "Theme system" section covering the catalog, the CSS-variable plumbing, the surface tokens, the FOUC bootstrap, and the View Transitions fallback. CLAUDE.md UI summary mentions the 14-preset count and the Apparence panel migration. * fix(themes): mirror the surface tokens for light mode Follow-up to 4229e11 (dark-mode surface theming) — same bug existed in light mode, just mirrored: themed light presets (Sunset Light / Lavender Light / Midnight Light / Crimson Light / Ocean Light) tinted only the accent and the body, leaving a stark white sidebar / panels / player bar against a pale-tinted background because those surfaces all hardcoded `bg-white` / `bg-[#FAFAFA]`. Adds the light-mode mirrors of the dark tokens: * `--color-surface-light` (default `#ffffff`) and `--color-surface-light-elevated` (default `#fafafa`) declared in `app.css`'s `@theme` block so Tailwind v4 generates the `bg-surface-light{-elevated}?` utilities at build time. * `ThemePreset` gains optional `surfaceLight` / `surfaceLightElevated` fields; `applyTheme` writes both CSS vars on `<html>` (always re-set, so a swap back to default clears any custom override). * Each themed light preset sets `surfaceLight = ambient` so sidebar / panels match the body and the theme reads as one surface, plus `surfaceLightElevated = #ffffff` so the player bar pops cleanly above the tinted body. * Sidebar, QueuePanel, NowPlayingPanel, LyricsPanel swap their hardcoded `bg-white` for `bg-surface-light`. * PlayerBar and AudioQualityFooter swap their hardcoded `bg-[#FAFAFA]` for `bg-surface-light-elevated`. Net result: every theme — light or dark — now tints sidebar + panels + player bar with the same family as the body, no remaining hardcoded neutrals on the body-level surfaces. * fix(themes): center column was masking the ambient + bump light tints Two follow-ups after the light-surface mirror (516972a): * AppLayout's center column used `bg-zinc-50` (fully opaque white) so on themed light presets the main scrollable area covered the body's amber / lavender / etc ambient with a flat white rectangle — the user reported "1 single part is yellow" because only the playlist header card (using `bg-accent-500/10` overlay) actually showed the amber tint. Switching to `bg-zinc-50/50` mirrors the existing `dark:bg-zinc-900/50` pattern: the ambient bleeds through, the subtle cool overlay still gives the column a slight separation from the sidebar. * Light ambients bumped from the Tailwind "-50" shade to "-100". The "-50" range (`#fff7ed`, `#f4f6ff`, `#faf5ff`, `#fff1f2`, `#eff6ff`) was so close to pure white that the theme was effectively invisible outside the accent buttons. "-100" gives a clearly readable tint without veering into saturated sticky-note territory: | Theme | -50 (old) | -100 (new) | |-----------------|-----------|------------| | midnight-light | `#f4f6ff` | `#e0e7ff` | | sunset-light | `#fff7ed` | `#fef3c7` | | lavender-light | `#faf5ff` | `#ede9fe` | | crimson-light | `#fff1f2` | `#ffe4e6` | | ocean-light | `#eff6ff` | `#e0f2fe` | `surfaceLight` updated in lockstep on each preset, and the inline `index.html` bootstrap script's ambient table now mirrors the new values so the pre-mount paint matches what `applyTheme` will set half a tick later. Default Émeraude Light is unchanged — it has no ambient override and falls back to `#ffffff`. * fix(themes): let the player bar inherit the light-theme tint Previous commit (bf5281e) set `surfaceLightElevated = #ffffff` on the 5 themed light variants thinking a pure-white player bar / audio quality footer would "pop" cleanly above the tinted body. In practice it broke the unified theme look — user pointed out the player bar was the only surface NOT carrying the lavender / amber / etc tint. Align `surfaceLightElevated = ambient` so all three surfaces (body, sidebar + right panels, player bar + footer) share the same tinted value. Separation between them comes from the borders that already exist (`border-t` on the player bar, `border-r` on the sidebar, `border-l` on the right panels) plus the slight cool blend of `bg-zinc-50/50` on the center column. Default Émeraude Light is unchanged — no overrides, surfaceLightElevated falls back to the `#fafafa` default which still gives a subtle elevation contrast against pure white. * fix(modals): guard stale Deezer search + reset cover menu on close Two CodeRabbit findings on PR #82 — both still-valid against the post-merge code, fixes are minimal and self-contained. * `CoverPickerModal.tsx` — the debounced Deezer search effect could fire `setResults` / `setError` / `setIsSearching` after the user closed the modal or moved on to a new query. `AnimatedModalShell` keeps the component mounted across its exit animation, so the request that was in-flight when the modal closed would still land and clobber the freshly-reset state. Adds a local `cancelled` token set in the effect cleanup and checked before each setter. * `CreatePlaylistModal.tsx` — `coverMenuOpen` was never reset when the modal closed. Call sites (Sidebar, LibraryView, PlaylistView, every detail view) mount the component unconditionally and toggle it via the `isOpen` prop, so the state survives close → reopen and the menu would silently reappear on the next launch with its click-outside listener still attached. The existing reset effect (`useEffect(() => if (!isOpen) ...)`) gains a `setCoverMenuOpen(false)` call; the dedicated click-outside listener auto-detaches when the value flips to false, so no extra cleanup is needed. * fix(themes): keep player bar white, tint search/pills/dropdowns instead User feedback on the previous attempt to theme `surfaceLightElevated`: the playback controls became hard to read against a tinted player bar. Roll the player bar + audio quality footer back to `bg-white` and narrow the theming to the surfaces that were actually flagged as "why isn't this ambient?": search bar, Settings category pills, profile dropdown menu, and the two TopBar popovers (filters + search results). Revised surface model for themed light presets: * `surfaceLight = ambient` (-100 shade) — sidebar, right panels, search bar, Settings pills, profile menu, TopBar popovers. The entire body-level + interactive-pill UI shares one tinted canvas. * `surfaceLightElevated` left at the default (`#fafafa`) — no themed preset overrides it anymore, the property is kept on `ThemePreset` for future presets that might want a separate elevated tier. * `PlayerBar` + `AudioQualityFooter` keep their hardcoded `bg-white` in light mode (the original `#FAFAFA` rolled back to pure white for cleaner contrast against the tinted body). `TopBar.tsx` swaps 4 hardcoded `bg-white` for `bg-surface-light` (search input, filters popover, search-results popover, profile dropdown). `SettingsView.tsx` swaps the inactive category pill's `bg-white` for `bg-surface-light` (the active variant already uses the themed `bg-emerald-50` which retints automatically). * revert(themes): roll back the search/pills/dropdown tints User-tested the cfaa7b7 attempt and confirmed the original split was the right call: tinting the search bar, Settings pills, profile menu and TopBar popovers hurt the contrast of the controls inside them (same issue as the player bar that just rolled back to white). Restores `bg-white` on: - TopBar search input - TopBar filters popover - TopBar search results popover - TopBar profile dropdown menu - SettingsView inactive category pill Net surface model now: - Body / sidebar / right panels = theme-tinted via `bg-surface-light` - Player bar, audio quality footer, search bar, pills, dropdown popovers = pure white in light mode for control contrast - Borders + the slight `bg-zinc-50/50` blend on the center column still provide visual separation between the tinted body and the white interactive elements. * fix: address remaining CodeRabbit findings on PR #82 Three valid issues fixed, three findings skipped as invalid against the current code. Fixed: * `CLAUDE.md` line 96 — two occurrences of "Settings → Apparence" rewritten as "Settings → Appearance" so the English overview doesn't mix the French UI label into prose. The localised label itself ("Apparence" in fr.json) stays unchanged. * `AnimatedModalShell` declared a `className?: string` prop but the component never destructured or applied it; no caller passed it either (grepped — every AnimatedModalShell call site under `src/components/common/` only sets `isOpen` / `onBackdropClick`). Dropped the dead prop from the interface; if a future caller needs per-instance class targeting, `backdropClassName` covers the backdrop and `AnimatedModalContent` already accepts `className`. * `ProfileSelectorModal` swapped views instantly because the outer `AnimatePresence` only animated the backdrop. Added an inner `AnimatePresence mode="wait"` around the three conditional view blocks (select / create / delete), each wrapped in a `motion.div` with a unique key + per-view enter/exit (opacity for select, fade + light scale for the centered create/delete cards). The outgoing view finishes its exit before the incoming one mounts, which is the standard cross-fade pattern for stepping through nested modal states. Skipped (still-valid CodeRabbit checks but the underlying issue is already resolved or the suggestion doesn't apply): * `docs/features/ui.md` claims `PlayerBarLayoutCard` is under Settings → Playback. Verified against `SettingsView.tsx` (line 2487, inside the `activeCategory === "appearance"` panel) — the card IS in Appearance, the doc is correct. The finding is reading a pre-merge snapshot of the file. * The stylelint Tailwind v4 at-rules suggestion has no setup to amend — the repo doesn't ship a `.stylelintrc*` and stylelint isn't a dependency. Nothing to ignoreAtRules in. * fix(modals): clear stale spinner + guard duplicates scan races Two more CodeRabbit findings on PR #82, both still-valid against the post-merge code. Two more findings skipped as no-ops. Fixed: * `CoverPickerModal`: when the user deletes letters back below 2 chars (the search-effect's early return path), the spinner stayed on forever — the previous request's `.finally` short-circuits on the `cancelled` token to avoid clobbering newer state, but that also skipped the `setIsSearching(false)` it would normally run. Added an explicit `setIsSearching(false)` before that early return, plus another in the `!isOpen` reset block so a closed-mid-fetch modal doesn't reopen with a stale spinner. * `DuplicatesModal`: `refresh()` could let a slow `findDuplicates` response land after the user closed the modal (the component is kept mounted by `AnimatedModalShell` for the exit animation) or after the user re-triggered the scan via `handleDeleteOthers`. A monotonic `requestIdRef` is captured at call time and checked before each setter — stale responses are dropped on the floor. The `!isOpen` reset bumps the same counter so any in-flight request becomes stale immediately, and clears `isScanning` for the same reason as the CoverPicker spinner cleanup. Skipped (invalid against current code): * `SettingsView.tsx` line 2432 `focus-visible:ring-emerald-500` is flagged as theme-mismatched, but `app.css`'s `@theme inline` block remaps `--color-emerald-50..950` to `--accent-*`. Tailwind v4 derives ALL emerald-* utilities (bg, text, border, ring, outline, fill, stroke, focus-visible:ring) from the same token, so the focus ring already follows the active accent — no change needed. * `en.json` theme keys (`defaultDark`, `midnightLight`…) flagged as not matching `themes.ts` ids (`default-dark`, `midnight-light`). These are deliberately different fields: `id` is the persisted kebab-case preset id (also used as the `data-theme` attribute on `<html>`), `labelKey` is the camelCase i18n path that `t()` looks up. Every `labelKey` in `themes.ts` already matches its JSON key character-for-character — nothing to align. * fix(duplicates): guard handleDeleteOthers against mid-delete close Follow-up to 18c9b52 — the staleness contract added on `refresh` only covered the scan path. `handleDeleteOthers` could close mid-delete (AnimatedModalShell keeps the component mounted) and still: * fire a wasted `findDuplicates` call after `deleteTracks` returned (the post-delete `await refresh()` ran unconditionally), and * land `setError` / `setIsDeleting(false)` on a closed modal. Snapshot `requestIdRef.current` at the start of the handler and short-circuit before each post-`await` setter / refresh call when the captured id no longer matches the latest (i.e. the modal's `!isOpen` effect already bumped the counter). Same pattern as `refresh` itself, so the two call sites participate in one staleness contract. `!isOpen` reset also clears `isDeleting` for the same reason it already cleared `isScanning`: the guarded `finally` skips the reset when the request is stale, so the spinner would otherwise linger on the next open. * fix(cover-picker): clear stale spinner when switching tabs mid-fetch Follow-up to 18c9b52 — that commit clipped the spinner-leak in the "close modal" and "trimmed < 2 chars" paths but missed the third exit path: the user switching from the Deezer tab to the Local tab while a fetch was in flight. Sequence that broke: 1. Type "abc" on Deezer → setTimeout fires → setIsSearching(true) → fetch begins. 2. Click the Local tab → effect re-runs → cleanup sets cancelled=true. 3. New effect body hits `!isOpen || tab !== "deezer"` and returns early without touching `isSearching`. 4. Original fetch resolves → `.finally` short-circuits on `cancelled` (would clobber the new tab's state otherwise) → `setIsSearching(false)` never runs. 5. Loader keeps spinning on the Local tab forever. Adds a single `setIsSearching(false)` before the early return so the spinner is force-cleared on tab swap (and redundantly on close — the reset effect above already handles that path, but two cheap writes are simpler than splitting the branch). CodeRabbit also suggested mirroring the reset in the effect cleanup; skipped because cleanup runs on every keystroke (the effect re-runs with the new `query`), which would blink the spinner off-then-on across the 300 ms debounce for the normal typing path. The early-return reset is enough to cover the actual exit conditions. * fix(duplicates): clear delete spinner before refresh() bumps requestId ebe71a0's staleness guard introduced a regression I missed: refresh() bumps requestIdRef.current internally, so on the happy path handleDeleteOthers's finally checks N === N+1, fails, and never calls setIsDeleting(false). The spinner stays on and the action button stays disabled after a successful delete. Move setIsDeleting(false) right after the post-deleteTracks staleness check, before await refresh(). The finally guard stays to cover the error / early-exit paths where refresh() isn't reached. * fix(modals): clear stale errors + guard apply handlers + split duplicates tokens Three valid CodeRabbit findings, one skipped as contradicting the intentional design. Fixed: * CoverPickerModal — both search-effect early-return branches (`!isOpen || tab !== "deezer"` and `trimmed.length < 2`) now also call setError(null) alongside setIsSearching(false). A failed Deezer search no longer leaves an error banner armed against the Local tab or against an empty query. * CoverPickerModal — `handlePickDeezer` / `handlePickFile` capture a `sessionRef` token at the start and short-circuit before each post-await write (onSuccess / onClose / setError / setIsApplying) when the modal has closed mid-flight. The `!isOpen` reset bumps the token AND clears `isApplying` directly (same reason as the search spinner cleanup — the guarded finally would otherwise leave the loader on the next open). * DuplicatesModal — split the single `requestIdRef` into two independent tokens, resolving a contradiction between two back-to-back CodeRabbit findings: (a) the delete spinner must clear on the happy path, (b) the Delete-others button must stay disabled across the post-delete rescan so a second click can't fire against stale `groups`. `scanSeqRef` is bumped per refresh() to guard concurrent scans; `sessionRef` is bumped only by the `!isOpen` reset so `handleDeleteOthers` can keep its spinner on across the embedded `refresh()` call and clear it only after the rescan completes (or the modal closes). Reverts the premature setIsDeleting(false) before refresh() from b63b558 — no longer needed. Skipped: * "PlayerBarLayoutCard back to Settings → Playback" is an intentional design decision the user made when the themes branch landed: the card was parked under Playback as a temporary placeholder in #122 with an explicit note to move it to Appearance once the picker filled that tab. CLAUDE.md line 96 documents the intent. Reverting would re-introduce the placeholder + scattered layout. Marked as not-planned.
9 tasks
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.
Closes #119, closes #120.
Summary
Per the user's follow-up on #119, this is a single PR that:
The user picked fixed order and icon grid preview in the design discussion, both of which this PR implements.
New / changed components
Setting keys
All per-profile, all default to match the pre-customisation layout (zero visible change on upgrade):
i18n
12 new keys propagated to all 17 locales (real translations, not placeholders — verified the same way as previous PRs via a one-shot Node script). Brand tokens preserved (`Spotify`, `EQ`).
Validation
Test plan
Defaults / migration
Settings panel
EQ pinning (#120)
Immersive dedup (#119)
Legacy compatibility
Summary by CodeRabbit
New Features
Refactor
Documentation
Chores