mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-12 04:17:16 +02:00
fix(ui): polish lyrics presentation
Signed-off-by: ranokay <github@ranokay.com>
This commit is contained in:
parent
04fffca1e8
commit
c0f06b292b
3 changed files with 78 additions and 69 deletions
|
|
@ -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 && (
|
||||
<KaraokeLineRow
|
||||
lineIndex={idx}
|
||||
line={buildSynchronizedTranslationLine(line, trLine)}
|
||||
line={buildStaticTranslationLine(trLine)}
|
||||
nextLineStart={null}
|
||||
className={clsx(classes.auxLine, classes.translationLine)}
|
||||
style={layerStyles.translation}
|
||||
className={classes.translationLine}
|
||||
tokenClassName={classes.token}
|
||||
rowKey="translation"
|
||||
/>
|
||||
|
|
|
|||
|
|
@ -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) => (
|
||||
<ThemeProvider theme={selectedTheme}>
|
||||
<LyricsPanel visible {...props} />
|
||||
|
|
@ -355,7 +361,7 @@ describe('<LyricsPanel />', () => {
|
|||
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('<LyricsPanel />', () => {
|
|||
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('<LyricsPanel />', () => {
|
|||
})
|
||||
})
|
||||
|
||||
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('<LyricsPanel />', () => {
|
|||
})
|
||||
|
||||
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('<LyricsPanel />', () => {
|
|||
})
|
||||
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('<LyricsPanel />', () => {
|
|||
(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)
|
||||
},
|
||||
)
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue