fix(ui): immersive view skin neutrality + long-title marquee#334
Conversation
…oss all skins The immersive view has role="dialog" + a shadow-2xl cover, so skins captured it with their modal / surface / text-colour chrome — Liquid (light) and Editorial repainted it as a light glass slab with dark, invisible text and washed-out controls; Lounge / Pulse flattened the cover backdrop. - mark the immersive root with data-immersive + a `dark` context (so the shared PlaybackControls / VolumeControl / ProgressBar render their dark-theme variants over the always-dark backdrop) - exclude [data-immersive] (and its subtree) from each skin's dialog background, surface neutralization, and text-colour overrides so the view renders with its own styling, identical across skins
Long track titles now glide end-to-end and back instead of being cut by an ellipsis, in both the PlayerBar and the immersive now-playing title. - MarqueeText: measures overflow via ResizeObserver, animates only when the text actually overflows (ping-pong with a pause at each end), static + truncated otherwise; respects prefers-reduced-motion - per-profile Settings -> Appearance toggle (ui.scroll_long_titles, default ON) via useScrollLongTitles + ScrollTitlesCard; off = truncate - also fixes descenders (g / y / p) being clipped in the immersive title (leading-tight + pb-1 under the overflow:hidden clip) - i18n 17 locales
InstaZDLL
added
scope: frontend
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 du défilement conditionnel des titres longs avec préférence persistée par profil, intégration dans la barre du lecteur et la vue immersive, marquage de la vue immersive, exclusions CSS des skins, et nouvelles chaînes de traduction avec documentation. ChangesDéfilement des titres longs et isolation skin immersive
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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/MarqueeText.tsx`:
- Around line 32-79: The reduced-motion fallback in MarqueeText is broken
because animate is still true even when the CSS animation is disabled, leaving
the text in the scrolling layout without truncation or centering. Update the
MarqueeText component’s animate/branching logic so it also respects
prefers-reduced-motion, and when motion is reduced force the non-animated
truncate path instead of the inline-block whitespace-nowrap + overflow-hidden
path. Make sure the fix preserves centered alignment for the immersive title and
restores ellipsis behavior in PlayerBar by tying the render branch to the
reduced-motion state alongside enabled, shift, and the existing useEffect
measurement logic.
In `@src/hooks/useScrollLongTitles.ts`:
- Around line 52-64: The write flow in useScrollLongTitles’s setEnabled
currently allows overlapping setProfileSetting calls, which can let an older
response overwrite the latest user intent. Update setEnabled to serialize or
coalesce preference writes, and ensure only the most recent request is allowed
to call setEnabledState and dispatch SCROLL_LONG_TITLES_EVENT. Use the existing
setEnabled callback, enabled state, and rollback path to guard against stale
responses before applying any success or failure side effects.
🪄 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: 15fb2a3a-a59a-4aec-8483-524e989493b0
📒 Files selected for processing (30)
docs/features/ui.mdsrc/app.csssrc/components/common/MarqueeText.tsxsrc/components/player/ImmersiveNowPlaying.tsxsrc/components/player/ImmersiveView.tsxsrc/components/player/PlayerBar.tsxsrc/components/views/SettingsView.tsxsrc/components/views/settings/ScrollTitlesCard.tsxsrc/hooks/useScrollLongTitles.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.jsonsrc/styles/skins/editorial.csssrc/styles/skins/liquid.csssrc/styles/skins/lounge.csssrc/styles/skins/pulse.css
|
@coderabbitai both valid, fixed:
typecheck + lint + build green. |
|
Tip For best results, initiate chat on the files or code changes.
Noté aussi pour la validation : typecheck, lint et build sont verts. (•ᴗ•)🐇 ✏️ Learnings added
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/hooks/useScrollLongTitles.ts (1)
36-37: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSérialisez vraiment les écritures Tauri.
writeSeqRefempêche seulement les effets locaux obsolètes. LessetProfileSetting(...)de Line 67 partent encore en parallèle : si l’utilisateur basculetrue → falseet que l’écrituretruetermine aprèsfalse, le profil peut rester persisté àtruesans nouvel événement, car Line 70 ignore seulement le side effect local. Chaînez/coalescez les writes, et rollbackez vers une valeur confirmée plutôt que versenabledRefoptimiste.Correctif possible
const enabledRef = useRef(enabled); + const confirmedEnabledRef = useRef(enabled); + const writeChainRef = useRef<Promise<void>>(Promise.resolve()); const writeSeqRef = useRef(0); @@ - setEnabledState(parseEnabled(raw)); + const parsed = parseEnabled(raw); + enabledRef.current = parsed; + confirmedEnabledRef.current = parsed; + setEnabledState(parsed); @@ const setEnabled = useCallback(async (next: boolean) => { const seq = ++writeSeqRef.current; - const previous = enabledRef.current; enabledRef.current = next; setEnabledState(next); try { - await setProfileSetting(KEY, next ? "true" : "false", "bool"); + const write = writeChainRef.current.then(async () => { + await setProfileSetting(KEY, next ? "true" : "false", "bool"); + confirmedEnabledRef.current = next; + }); + writeChainRef.current = write.catch(() => undefined); + await write; // A newer toggle superseded this one mid-write — let it own the // outcome so an older response can't clobber the latest intent. if (seq !== writeSeqRef.current) return; window.dispatchEvent(new CustomEvent(SCROLL_LONG_TITLES_EVENT)); } catch (err) { console.error("[useScrollLongTitles] write failed", err); if (seq !== writeSeqRef.current) return; - enabledRef.current = previous; - setEnabledState(previous); + const rollback = confirmedEnabledRef.current; + enabledRef.current = rollback; + setEnabledState(rollback); } }, []);As per path instructions,
src/**: « Vérifie les erreurs de state/effects, les handlers qui lancent des commandes Tauri ».Also applies to: 48-48, 61-76
🤖 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/hooks/useScrollLongTitles.ts` around lines 36 - 37, `useScrollLongTitles` ne sérialise pas réellement les écritures Tauri: `writeSeqRef` ne filtre que les effets locaux obsolètes, mais les appels à `setProfileSetting(...)` peuvent encore s’exécuter en parallèle et laisser un état persistant incorrect. Modifie l’effet/la logique autour de `enabledRef`, `writeSeqRef` et `setProfileSetting` pour chaîner ou coalescer les writes, puis reviens à une valeur confirmée par la dernière écriture réussie plutôt que de restaurer directement la valeur optimiste de `enabledRef`.Source: Path instructions
🤖 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.
Duplicate comments:
In `@src/hooks/useScrollLongTitles.ts`:
- Around line 36-37: `useScrollLongTitles` ne sérialise pas réellement les
écritures Tauri: `writeSeqRef` ne filtre que les effets locaux obsolètes, mais
les appels à `setProfileSetting(...)` peuvent encore s’exécuter en parallèle et
laisser un état persistant incorrect. Modifie l’effet/la logique autour de
`enabledRef`, `writeSeqRef` et `setProfileSetting` pour chaîner ou coalescer les
writes, puis reviens à une valeur confirmée par la dernière écriture réussie
plutôt que de restaurer directement la valeur optimiste de `enabledRef`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3d7760fc-f4bf-4d24-b57a-adfc95d74e6b
📒 Files selected for processing (2)
src/components/common/MarqueeText.tsxsrc/hooks/useScrollLongTitles.ts
|
@coderabbitai valid, fixed — the Tauri writes are now serialized on a |
|
Tip For best results, initiate chat on the files or code changes.
La sérialisation via ✏️ Learnings added
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/useScrollLongTitles.ts`:
- Around line 37-43: The write queue in useScrollLongTitles can apply an
outdated toggle after activeProfile changes, causing persistence to hit the
wrong profile. Update the toggle/write flow around confirmedEnabledRef,
writeChainRef, and setProfileSetting so each queued write captures the current
profile id at dispatch time and aborts before the Tauri call if
activeProfile?.id has changed, or otherwise pass an explicit profile id through
the persistence API to avoid writing to a newly selected profile.
🪄 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: 8ef86ce2-6220-4364-9d46-3cbcc59e0cc2
📒 Files selected for processing (1)
src/hooks/useScrollLongTitles.ts
|
@coderabbitai valid, fixed in 104cd0f — |
|
Tip For best results, initiate chat on the files or code changes.
Capturer ✏️ 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/hooks/useScrollLongTitles.ts (1)
91-109: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winIgnore aussi les effets de bord après un changement de profil en vol.
Le garde de Line 91 protège seulement avant l’appel Tauri. Si le profil change pendant
setProfileSetting(...), Line 93 peut encore confirmer une valeur de l’ancien profil, et lecatchpeut rollback l’UI du nouveau profil vers cette valeur obsolète.Correctif proposé
if (activeProfileIdRef.current !== profileId) return; await setProfileSetting(KEY, next ? "true" : "false", "bool"); + if (activeProfileIdRef.current !== profileId) return; confirmedEnabledRef.current = next; }); @@ await write; + if (activeProfileIdRef.current !== profileId) return; // A newer toggle superseded this one mid-write — let it own the // outcome so an older response can't clobber the latest intent. if (seq !== writeSeqRef.current) return; @@ } catch (err) { console.error("[useScrollLongTitles] write failed", err); + if (activeProfileIdRef.current !== profileId) return; if (seq !== writeSeqRef.current) return;As per path instructions,
src/**:Vérifie les erreurs de state/effects, les handlers qui lancent des commandes Tauri....🤖 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/hooks/useScrollLongTitles.ts` around lines 91 - 109, The write path in useScrollLongTitles still applies side effects after a profile switch because the guard only runs before setProfileSetting; add a second active-profile check after the await in the write closure and again in the catch/rollback path so stale writes from an old profile cannot confirm or revert the new profile’s state. Update the logic around writeSeqRef, activeProfileIdRef, confirmedEnabledRef, and setEnabledState so only the current profile can dispatch SCROLL_LONG_TITLES_EVENT or perform rollback.Source: Path instructions
🤖 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/hooks/useScrollLongTitles.ts`:
- Around line 91-109: The write path in useScrollLongTitles still applies side
effects after a profile switch because the guard only runs before
setProfileSetting; add a second active-profile check after the await in the
write closure and again in the catch/rollback path so stale writes from an old
profile cannot confirm or revert the new profile’s state. Update the logic
around writeSeqRef, activeProfileIdRef, confirmedEnabledRef, and setEnabledState
so only the current profile can dispatch SCROLL_LONG_TITLES_EVENT or perform
rollback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f7899f0e-4b87-4541-9f18-9fecc59bfdc5
📒 Files selected for processing (1)
src/hooks/useScrollLongTitles.ts
|
@coderabbitai valid, fixed in cf89452 — added a profile re-check after the |
|
Tip For best results, initiate chat on the files or code changes.
Le re-check après le Noté aussi pour ✏️ Learnings added
|
1 participant
Summary
Polish pass on the immersive view + skins, reported visually during testing.
Immersive view was wrecked by the skins
The immersive view (
role="dialog"+ ashadow-2xlcover) was captured by every skin's modal / surface / text-colour chrome:Fix: the root is tagged
data-immersive+ a nesteddarkcontext (so the sharedPlaybackControls/VolumeControl/ProgressBarrender their dark-theme variants over the always-dark backdrop), and each skin's dialog / surface / text-colour rules carry:not([data-immersive]):not([data-immersive] *). The view now renders identically across all skins.Long titles scroll instead of truncating
New
MarqueeText— measures overflow viaResizeObserverand, only when the text actually overflows, glides it end-to-end and back (ping-pong with a pause at each end,prefers-reduced-motionrespected). Wired into the now-playing title + the bottomPlayerBartitle.ui.scroll_long_titles, default ON) viauseScrollLongTitles+ScrollTitlesCard; off = ellipsis truncation.g/y/p) being clipped in the immersive title.Notes
docs/features/ui.mdupdated.Validation
bun run typecheck✅ ·bun run lint✅ ·bun run build✅Summary by CodeRabbit