diff --git a/ui/src/audioplayer/LyricsPanel.jsx b/ui/src/audioplayer/LyricsPanel.jsx index 09f00402a..9b2a45f57 100644 --- a/ui/src/audioplayer/LyricsPanel.jsx +++ b/ui/src/audioplayer/LyricsPanel.jsx @@ -36,6 +36,7 @@ import { finiteTime } from './lyricsTimeline' import useLyricsTimeline from './useLyricsTimeline' const KARAOKE_LAYER_OPACITY_TRANSITION = `opacity ${KARAOKE_LINE_OPACITY_MS}ms ${KARAOKE_EASING}` +const KARAOKE_LAYER_COLOR_TRANSITION = `color ${KARAOKE_LINE_OPACITY_MS}ms ${KARAOKE_EASING}, -webkit-text-fill-color ${KARAOKE_LINE_OPACITY_MS}ms ${KARAOKE_EASING}` const useStyles = makeStyles((theme) => ({ root: { @@ -58,15 +59,24 @@ const useStyles = makeStyles((theme) => ({ overflowX: 'hidden', padding: theme.spacing(4, 2.25, 3.25), overscrollBehavior: 'contain', - scrollbarWidth: 'none', - msOverflowStyle: 'none', + scrollbarGutter: 'stable', + scrollbarWidth: 'thin', + scrollbarColor: 'transparent transparent', maskImage: 'linear-gradient(to bottom, transparent 0, rgba(0, 0, 0, 0.15) 12px, #000 40px, #000 calc(100% - 120px), rgba(0, 0, 0, 0.12) calc(100% - 48px), transparent 100%)', WebkitMaskImage: 'linear-gradient(to bottom, transparent 0, rgba(0, 0, 0, 0.15) 12px, #000 40px, #000 calc(100% - 120px), rgba(0, 0, 0, 0.12) calc(100% - 48px), transparent 100%)', '&::-webkit-scrollbar': { - width: 0, - height: 0, + width: 8, + height: 8, + }, + // Player themes use two-class scrollbar selectors. + '&&&::-webkit-scrollbar-thumb': { + backgroundColor: 'transparent', + borderRadius: 999, + }, + '&::-webkit-scrollbar-track': { + backgroundColor: 'transparent', }, }, bodyTopFade: { @@ -76,18 +86,9 @@ const useStyles = makeStyles((theme) => ({ 'linear-gradient(to bottom, transparent 0, rgba(0, 0, 0, 0.15) 8px, #000 24px, #000 calc(100% - 120px), rgba(0, 0, 0, 0.12) calc(100% - 48px), transparent 100%)', }, bodyUserScrolling: { - scrollbarWidth: 'thin', - msOverflowStyle: 'auto', - '&::-webkit-scrollbar': { - width: 8, - height: 8, - }, - '&::-webkit-scrollbar-thumb': { + scrollbarColor: `${theme.palette.action.disabled} transparent`, + '&&&::-webkit-scrollbar-thumb': { backgroundColor: theme.palette.action.disabled, - borderRadius: 999, - }, - '&::-webkit-scrollbar-track': { - backgroundColor: 'transparent', }, }, inlineBody: { @@ -176,7 +177,7 @@ const useStyles = makeStyles((theme) => ({ transition: 'none', }, }, - auxLine: { + translationLine: { display: 'block', opacity: 1, fontWeight: 600, @@ -188,10 +189,12 @@ const useStyles = makeStyles((theme) => ({ color: 'var(--lyrics-translation-current-color, currentColor)', WebkitTextFillColor: 'var(--lyrics-translation-current-color, currentColor)', - transition: 'none', + transition: KARAOKE_LAYER_COLOR_TRANSITION, + '&&': { + marginTop: theme.spacing(0.6), + }, '@media (prefers-reduced-motion: reduce)': { transition: 'none', - transform: 'none', }, }, stackedToken: { @@ -247,9 +250,6 @@ const useStyles = makeStyles((theme) => ({ transition: 'none', }, }, - translationLine: { - fontWeight: 600, - }, token: { whiteSpace: 'pre-wrap', overflowWrap: 'anywhere', @@ -362,7 +362,7 @@ const buildUniqueLayerMap = (mainLines, layerLines) => { const getLineLanes = (line) => Array.isArray(line?.lanes) && line.lanes.length > 0 ? line.lanes : [line] -const buildSynchronizedTranslationLine = (_mainLine, translationLine) => { +const buildStaticTranslationLine = (translationLine) => { if (!translationLine) return null return { ...translationLine, @@ -382,8 +382,7 @@ const buildLineGroupStyle = (canSeekLine, layerStyles) => ({ layerStyles.pronunciation.color, '--lyrics-translation-idle-color': layerStyles.translation.color, '--lyrics-translation-active-color': - layerStyles.translation['--lyrics-active-color'] || - layerStyles.translation.color, + layerStyles.translation['--lyrics-active-color'], }) const usePrefersReducedMotion = () => { @@ -500,10 +499,6 @@ const LyricsPanel = ({ () => ({ main: theme.palette.text.primary, pronunciation: theme.palette.primary.main, - translation: - theme.palette.text.secondary || - theme.palette.secondary?.main || - theme.palette.text.primary, }), [theme], ) @@ -511,14 +506,6 @@ const LyricsPanel = ({ const layerStyles = useMemo(() => { const styleFor = (layer) => { const sourceColor = colors[layer] - if (layer === 'translation') { - return { - opacity: 1, - marginTop: theme.spacing(0.6), - color: sourceColor, - '--lyrics-active-color': sourceColor, - } - } if (!hasTimedMainLines) { return { opacity: 1, @@ -537,7 +524,10 @@ const LyricsPanel = ({ return { main: styleFor('main'), pronunciation: styleFor('pronunciation'), - translation: styleFor('translation'), + translation: { + color: theme.palette.text.disabled, + '--lyrics-active-color': theme.palette.text.secondary, + }, } }, [colors, hasTimedMainLines, theme]) @@ -932,10 +922,9 @@ const LyricsPanel = ({ {showTr && ( diff --git a/ui/src/audioplayer/LyricsPanel.test.jsx b/ui/src/audioplayer/LyricsPanel.test.jsx index d37e31d95..cb5e0712c 100644 --- a/ui/src/audioplayer/LyricsPanel.test.jsx +++ b/ui/src/audioplayer/LyricsPanel.test.jsx @@ -19,6 +19,7 @@ import { KARAOKE_HIGHLIGHT_LEAD_MS, KARAOKE_LINE_LIFT_PX, KARAOKE_MANUAL_SCROLL_PAUSE_MS, + KARAOKE_SCROLLBAR_VISIBLE_MS, TOKEN_FUTURE_ALPHA, } from './lyricsKaraokeConstants' @@ -29,6 +30,11 @@ const theme = createTheme({ }, }) +const translationColorsFor = (selectedTheme) => ({ + active: selectedTheme.palette.text.secondary, + idle: selectedTheme.palette.text.disabled, +}) + const panel = (props, selectedTheme = theme) => ( @@ -355,7 +361,7 @@ describe('', () => { expect(pronunciation.style.color).toBe('') }) - it('keeps translations subordinate and stable between line states', () => { + it('highlights translations with the shared layer transition', () => { const propsAt = (currentTime) => ({ mainLyric, pronunciationLyric: { @@ -386,26 +392,29 @@ describe('', () => { expect(window.getComputedStyle(mainRow).transition).toContain( `opacity ${KARAOKE_LINE_OPACITY_MS}ms ${KARAOKE_EASING}`, ) - expect(window.getComputedStyle(translationRow).transition).toBe('none') + expect(window.getComputedStyle(translationRow).transition).toContain( + `color ${KARAOKE_LINE_OPACITY_MS}ms ${KARAOKE_EASING}`, + ) pronunciation.forEach((token) => expect(token).toHaveAttribute('data-timed', 'false'), ) expect(window.getComputedStyle(translationRow).opacity).toBe('1') + const translationColors = translationColorsFor(theme) expect( lineGroup.style.getPropertyValue('--lyrics-translation-idle-color'), - ).toBe(theme.palette.text.secondary) + ).toBe(translationColors.idle) expect( lineGroup.style.getPropertyValue('--lyrics-translation-active-color'), - ).toBe(theme.palette.text.secondary) + ).toBe(translationColors.active) expect(window.getComputedStyle(translationRow).transform).toBe('') - expect(window.getComputedStyle(translationRow).marginTop).toBe( - `${theme.spacing(0.6)}px`, - ) rerenderPanel(propsAt(1.1)) + expect(lineGroup).toHaveAttribute('data-active', 'false') expect(window.getComputedStyle(translationRow).opacity).toBe('1') - expect(window.getComputedStyle(translationRow).transition).toBe('none') + expect(window.getComputedStyle(translationRow).transition).toContain( + `color ${KARAOKE_LINE_OPACITY_MS}ms ${KARAOKE_EASING}`, + ) expect(window.getComputedStyle(translationRow).transform).toBe('') }) @@ -877,6 +886,28 @@ describe('', () => { }) }) + it('keeps scrollbar geometry stable before, during, and after manual scroll', () => { + vi.useFakeTimers() + renderPanel({ mainLyric }) + + const body = screen.getByTestId('lyrics-scroll-body') + const initialStyle = window.getComputedStyle(body) + expect(initialStyle.scrollbarGutter).toBe('stable') + expect(initialStyle.scrollbarWidth).toBe('thin') + + fireEvent.wheel(body) + expect(body).toHaveAttribute('data-scrollbar-visible', 'true') + expect(window.getComputedStyle(body).scrollbarGutter).toBe('stable') + expect(window.getComputedStyle(body).scrollbarWidth).toBe('thin') + + act(() => { + vi.advanceTimersByTime(KARAOKE_SCROLLBAR_VISIBLE_MS) + }) + expect(body).toHaveAttribute('data-scrollbar-visible', 'false') + expect(window.getComputedStyle(body).scrollbarGutter).toBe('stable') + expect(window.getComputedStyle(body).scrollbarWidth).toBe('thin') + }) + it('resets scroll position when lyric content changes', () => { const { rerenderPanel } = renderPanel({ mainLyric }) const body = screen.getByTestId('lyrics-scroll-body') @@ -914,7 +945,7 @@ describe('', () => { }) it.each(['light', 'dark'])( - 'keeps future tokens and translations readable in the %s theme', + 'keeps future tokens readable and translations subordinate in the %s theme', (type) => { const accessibleTheme = createTheme({ palette: { type } }) const background = accessibleTheme.palette.background.default @@ -930,7 +961,6 @@ describe('', () => { }) const { rerenderPanel } = renderPanel(propsAt(0.25), accessibleTheme) - const group = screen.getByTestId('lyrics-line-group') const translation = screen .getByText('Readable translation') .closest('[data-tokenized]') @@ -945,33 +975,23 @@ describe('', () => { (match) => match[0], ) const futureColor = gradientColors.at(-1) + const translationColors = translationColorsFor(accessibleTheme) - expect( - group.style.getPropertyValue('--lyrics-translation-idle-color'), - ).toBe(accessibleTheme.palette.text.secondary) - expect( - group.style.getPropertyValue('--lyrics-translation-active-color'), - ).toBe(accessibleTheme.palette.text.secondary) expect(parseCssColor(futureColor).alpha).toBe(TOKEN_FUTURE_ALPHA) expect(contrastRatio(futureColor, background)).toBeGreaterThanOrEqual(4.5) expect( - contrastRatio(accessibleTheme.palette.text.secondary, background), + contrastRatio(translationColors.active, background), ).toBeGreaterThanOrEqual(4.5) - expect( - contrastRatio(accessibleTheme.palette.text.secondary, background), - ).toBeLessThan( - contrastRatio(accessibleTheme.palette.text.primary, background), + expect(contrastRatio(translationColors.idle, background)).toBeLessThan( + contrastRatio(translationColors.active, background), ) rerenderPanel(propsAt(1.25)) - expect(group).toHaveAttribute('data-active', 'false') + expect(screen.getByTestId('lyrics-line-group')).toHaveAttribute( + 'data-active', + 'false', + ) expect(window.getComputedStyle(translation).opacity).toBe('1') - expect( - group.style.getPropertyValue('--lyrics-translation-idle-color'), - ).toBe(accessibleTheme.palette.text.secondary) - expect( - group.style.getPropertyValue('--lyrics-translation-active-color'), - ).toBe(accessibleTheme.palette.text.secondary) }, ) }) diff --git a/ui/src/audioplayer/lyricsKaraokeConstants.js b/ui/src/audioplayer/lyricsKaraokeConstants.js index 28dc1b917..ff4983cbd 100644 --- a/ui/src/audioplayer/lyricsKaraokeConstants.js +++ b/ui/src/audioplayer/lyricsKaraokeConstants.js @@ -1,7 +1,7 @@ export const KARAOKE_CLOCK_DRIFT_RESET_MS = 140 export const KARAOKE_HIGHLIGHT_LEAD_MS = 120 export const KARAOKE_ANIMATION_MS = 500 -export const KARAOKE_LINE_OPACITY_MS = 200 +export const KARAOKE_LINE_OPACITY_MS = 300 export const KARAOKE_SCROLLBAR_VISIBLE_MS = 1400 export const KARAOKE_MANUAL_SCROLL_PAUSE_MS = 2200 export const KARAOKE_SCROLL_ANIMATION_MS = 300