From 5ef4bbefcf03b680bd53ab4325f3e3b8d6cc9625 Mon Sep 17 00:00:00 2001 From: ranokay Date: Sun, 26 Jul 2026 11:14:39 +0300 Subject: [PATCH] fix(ui): improve lyrics mobile UX and code quality - Center lyrics start position on mobile via dynamic paddingTop - Fix oval panel shape on mobile by forcing borderRadius: 0 - Remove touch highlight from PR/TR toggle buttons - Separate line-by-line animation timing (260ms) from syllable (500ms) - Deduplicate sameIndexes, finiteTime, and transition string constants - Move KARAOKE_IDLE_LAYER_OPACITY to shared constants file - Remove redundant useTheme hook and inlineLine class - Extract DEFAULT_RGB constant in LyricsLineRows - Use nullish coalescing in lyricsSidebarState - Fix pointer-events kebab-case inconsistency in styles - Link KARAOKE_CHARACTER_LIFT_PX to KARAOKE_LINE_LIFT_PX Signed-off-by: ranokay --- ui/src/audioplayer/LyricsLayerControls.jsx | 158 ++++++++++++++++++ ui/src/audioplayer/LyricsLineRows.jsx | 5 +- ui/src/audioplayer/LyricsPanel.jsx | 59 ++++--- ui/src/audioplayer/LyricsPanel.test.jsx | 20 ++- ui/src/audioplayer/LyricsSidebar.jsx | 137 ++------------- ui/src/audioplayer/LyricsSidebar.test.jsx | 8 +- .../MobileKaraokeLyricsPortal.test.jsx | 69 +++++++- ui/src/audioplayer/lyricsKaraokeConstants.js | 8 +- ui/src/audioplayer/lyricsSidebarState.js | 10 +- ui/src/audioplayer/lyricsTimeline.js | 4 +- ui/src/audioplayer/styles.js | 12 +- ui/src/audioplayer/useLyricsTimeline.js | 5 +- ui/src/audioplayer/useLyricsTimeline.test.js | 24 ++- ui/src/audioplayer/usePlayerLyrics.jsx | 16 ++ 14 files changed, 336 insertions(+), 199 deletions(-) create mode 100644 ui/src/audioplayer/LyricsLayerControls.jsx diff --git a/ui/src/audioplayer/LyricsLayerControls.jsx b/ui/src/audioplayer/LyricsLayerControls.jsx new file mode 100644 index 000000000..56d5e27f0 --- /dev/null +++ b/ui/src/audioplayer/LyricsLayerControls.jsx @@ -0,0 +1,158 @@ +import IconButton from '@material-ui/core/IconButton' +import Tooltip from '@material-ui/core/Tooltip' +import { alpha, makeStyles } from '@material-ui/core/styles' +import RecordVoiceOverIcon from '@material-ui/icons/RecordVoiceOver' +import TranslateIcon from '@material-ui/icons/Translate' +import clsx from 'clsx' +import React from 'react' + +const useStyles = makeStyles( + (theme) => ({ + controls: { + position: 'absolute', + bottom: theme.spacing(1), + zIndex: 2, + display: 'flex', + alignItems: 'center', + gap: theme.spacing(0.25), + padding: theme.spacing(0.25), + borderRadius: theme.shape.borderRadius * 2, + backgroundColor: 'transparent', + }, + sidebarPlacement: { + right: theme.spacing(0.75), + }, + mobilePlacement: { + left: '50%', + transform: 'translateX(-50%)', + }, + controlButton: { + padding: theme.spacing(0.75), + color: alpha(theme.palette.text.primary, 0.58), + backgroundColor: 'transparent', + WebkitTapHighlightColor: 'transparent', + transition: + 'color 160ms ease, background-color 160ms ease, transform 160ms cubic-bezier(0.23, 1, 0.32, 1)', + '@media (hover: hover) and (pointer: fine)': { + '&:hover': { + color: theme.palette.text.primary, + backgroundColor: alpha(theme.palette.primary.main, 0.08), + }, + '&$controlActive:hover': { + color: theme.palette.primary.main, + }, + }, + '&:focus-visible': { + color: theme.palette.text.primary, + backgroundColor: alpha(theme.palette.primary.main, 0.1), + }, + '&:active:not(:disabled)': { + transform: 'scale(0.97)', + }, + '&$controlActive': { + color: theme.palette.primary.main, + }, + '&$controlActive:focus-visible': { + color: theme.palette.primary.main, + }, + '&:disabled': { + color: alpha(theme.palette.text.primary, 0.28), + }, + '@media (prefers-reduced-motion: reduce)': { + transition: 'none', + '&:active:not(:disabled)': { + transform: 'none', + }, + }, + }, + controlActive: {}, + }), + { name: 'NDLyricsLayerControls' }, +) + +const LayerButton = ({ + active, + classes, + disabled, + label, + onClick, + testId, + children, +}) => { + return ( + + + + {children} + + + + ) +} + +const LyricsLayerControls = ({ + placement = 'sidebar', + showTranslation, + showPronunciation, + translationEnabled, + pronunciationEnabled, + onToggleTranslation, + onTogglePronunciation, + labels = {}, + testId = 'lyrics-layer-controls', +}) => { + const classes = useStyles() + + return ( +
event.stopPropagation()} + > + + + + + + +
+ ) +} + +export default LyricsLayerControls diff --git a/ui/src/audioplayer/LyricsLineRows.jsx b/ui/src/audioplayer/LyricsLineRows.jsx index 77d9fa00b..964025e51 100644 --- a/ui/src/audioplayer/LyricsLineRows.jsx +++ b/ui/src/audioplayer/LyricsLineRows.jsx @@ -24,6 +24,7 @@ import { resolveKaraokeTokenWindows } from './lyricsTimeline' const EMPHASIS_TONE = 0.7 const EMPHASIS_PAINT_OVERHANG = '0.18em' +const DEFAULT_RGB = [255, 255, 255] const emphasisPaintOverhangStyle = { marginInlineEnd: `-${EMPHASIS_PAINT_OVERHANG}`, @@ -225,7 +226,7 @@ export const KaraokeLineRow = memo( [line, nextLineStart], ) const tokenRGB = useMemo( - () => (style?.color ? parseColorRGB(style.color) : [255, 255, 255]), + () => (style?.color ? parseColorRGB(style.color) : DEFAULT_RGB), [style?.color], ) const hasTimedTokens = windows.some( @@ -444,7 +445,7 @@ export const KaraokeStackedLineRow = memo( () => pronunciationStyle?.color ? parseColorRGB(pronunciationStyle.color) - : [255, 255, 255], + : DEFAULT_RGB, [pronunciationStyle?.color], ) const hasTimedTokens = mainWindows.some( diff --git a/ui/src/audioplayer/LyricsPanel.jsx b/ui/src/audioplayer/LyricsPanel.jsx index 8a73dda21..fa47bf519 100644 --- a/ui/src/audioplayer/LyricsPanel.jsx +++ b/ui/src/audioplayer/LyricsPanel.jsx @@ -15,10 +15,11 @@ import { } from './lyrics' import { KaraokeLineRow, KaraokeStackedLineRow } from './LyricsLineRows' import { - KARAOKE_ANIMATION_MS, + KARAOKE_LINE_OPACITY_MS, KARAOKE_AUX_LINE_HEIGHT, KARAOKE_DESKTOP_ACTIVE_LINE_ANCHOR_RATIO, KARAOKE_EASING, + KARAOKE_IDLE_LAYER_OPACITY, KARAOKE_INLINE_ACTIVE_LINE_ANCHOR_RATIO, KARAOKE_LINE_ENTER_MS, KARAOKE_LINE_LIFT_PX, @@ -33,10 +34,10 @@ import { getAnchoredScrollTop, getScrollEndPadding, } from './lyricsScroll' +import { finiteTime } from './lyricsTimeline' import useLyricsTimeline from './useLyricsTimeline' -const KARAOKE_LAYER_OPACITY_TRANSITION = `opacity ${KARAOKE_ANIMATION_MS}ms ${KARAOKE_EASING}` -const KARAOKE_IDLE_LAYER_OPACITY = 0.49 +const KARAOKE_LAYER_OPACITY_TRANSITION = `opacity ${KARAOKE_LINE_OPACITY_MS}ms ${KARAOKE_EASING}` const KARAOKE_TRANSLATION_IDLE_OPACITY = KARAOKE_IDLE_LAYER_OPACITY * KARAOKE_TRANSLATION_OPACITY @@ -103,6 +104,7 @@ const useStyles = makeStyles((theme) => ({ alignItems: 'stretch', gap: theme.spacing(3), }, + inlineLines: {}, lineGroup: { width: '100%', borderRadius: theme.shape.borderRadius, @@ -187,9 +189,6 @@ const useStyles = makeStyles((theme) => ({ transition: 'none', }, }, - inlineLine: { - fontSize: 24, - }, auxLine: { display: 'block', marginTop: theme.spacing(0.8), @@ -307,18 +306,12 @@ const shouldShowAuxLine = (mainLine, auxLine) => normalizeLineText(auxLine.value) !== normalizeLineText(mainLine?.value), ) -const finiteLineTime = (value) => { - if (value == null || value === '') return null - const number = Number(value) - return Number.isFinite(number) ? number : null -} - const getLayerMatchWindow = (lines, index) => { const line = lines[index] if (!line) return { start: null, end: null } return { - start: finiteLineTime(line.start), - end: finiteLineTime(line.end) ?? finiteLineTime(lines[index + 1]?.start), + start: finiteTime(line.start), + end: finiteTime(line.end) ?? finiteTime(lines[index + 1]?.start), } } @@ -454,6 +447,7 @@ const LyricsPanel = ({ const [layoutVersion, setLayoutVersion] = useState(0) const [hasTopFade, setHasTopFade] = useState(false) const [scrollEndPadding, setScrollEndPadding] = useState(0) + const [scrollStartPadding, setScrollStartPadding] = useState(0) const prefersReducedMotion = usePrefersReducedMotion() const activeLineAnchorRatio = inline ? KARAOKE_INLINE_ACTIVE_LINE_ANCHOR_RATIO @@ -617,13 +611,24 @@ const LyricsPanel = ({ const body = bodyRef.current if (!visible || !body) { setScrollEndPadding(0) + setScrollStartPadding(0) return } - const nextPadding = getScrollEndPadding(body, activeLineAnchorRatio) + const nextEndPadding = getScrollEndPadding(body, activeLineAnchorRatio) setScrollEndPadding((current) => - current === nextPadding ? current : nextPadding, + current === nextEndPadding ? current : nextEndPadding, ) - }, [activeLineAnchorRatio, layoutVersion, mainLines.length, visible]) + if (inline) { + const nextStartPadding = Math.round( + body.clientHeight * activeLineAnchorRatio, + ) + setScrollStartPadding((current) => + current === nextStartPadding ? current : nextStartPadding, + ) + } else { + setScrollStartPadding(0) + } + }, [activeLineAnchorRatio, inline, layoutVersion, mainLines.length, visible]) useLayoutEffect(() => { const body = bodyRef.current @@ -726,11 +731,16 @@ const LyricsPanel = ({ onTouchMove={markManualScrollIntent} >
0 - ? { paddingBottom: scrollEndPadding } + scrollEndPadding > 0 || scrollStartPadding > 0 + ? { + paddingTop: scrollStartPadding || undefined, + paddingBottom: scrollEndPadding || undefined, + } : undefined } > @@ -807,7 +817,6 @@ const LyricsPanel = ({ > {lineLanes.map((lane, laneIdx) => { const laneClassName = clsx(classes.line, { - [classes.inlineLine]: inline, [classes.secondaryVoiceLane]: laneIdx > 0, }) const rowKey = lane.key || `lane-${laneIdx}` @@ -855,9 +864,7 @@ const LyricsPanel = ({ pronunciationLine={prLine} pronunciationStyle={layerStyles.pronunciation} nextLineStart={mainNextLineStart} - className={clsx(classes.line, { - [classes.inlineLine]: inline, - })} + className={classes.line} style={layerStyles.main} tokenClassName={classes.token} waveCharacterClassName={classes.waveCharacter} @@ -871,9 +878,7 @@ const LyricsPanel = ({ lineIndex={idx} line={line} nextLineStart={mainNextLineStart} - className={clsx(classes.line, { - [classes.inlineLine]: inline, - })} + className={classes.line} style={layerStyles.main} tokenClassName={classes.token} waveCharacterClassName={classes.waveCharacter} diff --git a/ui/src/audioplayer/LyricsPanel.test.jsx b/ui/src/audioplayer/LyricsPanel.test.jsx index 032e369e3..3a0533382 100644 --- a/ui/src/audioplayer/LyricsPanel.test.jsx +++ b/ui/src/audioplayer/LyricsPanel.test.jsx @@ -12,8 +12,11 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import LyricsPanel from './LyricsPanel' import { KaraokeLineRow } from './LyricsLineRows' import { - KARAOKE_ANIMATION_MS, + KARAOKE_LINE_OPACITY_MS, KARAOKE_CHARACTER_LIFT_PX, + KARAOKE_CHARACTER_RISE_MS, + KARAOKE_HIGHLIGHT_LEAD_MS, + KARAOKE_IDLE_LAYER_OPACITY, KARAOKE_LINE_LIFT_PX, KARAOKE_MANUAL_SCROLL_PAUSE_MS, KARAOKE_TRANSLATION_OPACITY, @@ -318,7 +321,7 @@ describe('', () => { expect(row).toHaveAttribute('data-tokenized', 'false') expect(row.style.opacity).toBe('') expect(window.getComputedStyle(row).transition).toContain( - `opacity ${KARAOKE_ANIMATION_MS}ms`, + `opacity ${KARAOKE_LINE_OPACITY_MS}ms`, ) }) pronunciation.forEach((token) => @@ -372,6 +375,8 @@ describe('', () => { }) it('keeps every word-timed layer on the same rise and release lifecycle', () => { + const settledFirstCharacterTime = + (KARAOKE_CHARACTER_RISE_MS - KARAOKE_HIGHLIGHT_LEAD_MS) / 1000 const propsAt = (currentTime) => ({ mainLyric: tokenizedMainLyric, pronunciationLyric: tokenizedPronunciationLyric, @@ -383,7 +388,7 @@ describe('', () => { showTranslation: true, audioInstance: { currentTime, paused: true }, }) - const { rerenderPanel } = renderPanel(propsAt(0.15)) + const { rerenderPanel } = renderPanel(propsAt(settledFirstCharacterTime)) const group = screen.getByTestId('lyrics-line-group') const translation = screen @@ -790,16 +795,19 @@ describe('', () => { .getComputedStyle(group) .getPropertyValue('--lyrics-layer-opacity') .trim() - expect(groups.map(layerOpacity)).toEqual(['1', '0.49']) + expect(groups.map(layerOpacity)).toEqual([ + '1', + String(KARAOKE_IDLE_LAYER_OPACITY), + ]) expect(window.getComputedStyle(activeTranslation).opacity).toBe( String(KARAOKE_TRANSLATION_OPACITY), ) expect(Number(window.getComputedStyle(futureTranslation).opacity)).toBe( - 0.49 * KARAOKE_TRANSLATION_OPACITY, + KARAOKE_IDLE_LAYER_OPACITY * KARAOKE_TRANSLATION_OPACITY, ) ;[activeTranslation, futureTranslation].forEach((translation) => expect(window.getComputedStyle(translation).transition).toContain( - `opacity ${KARAOKE_ANIMATION_MS}ms`, + `opacity ${KARAOKE_LINE_OPACITY_MS}ms`, ), ) }) diff --git a/ui/src/audioplayer/LyricsSidebar.jsx b/ui/src/audioplayer/LyricsSidebar.jsx index cfb398b6c..0ad0aa908 100644 --- a/ui/src/audioplayer/LyricsSidebar.jsx +++ b/ui/src/audioplayer/LyricsSidebar.jsx @@ -1,10 +1,6 @@ -import IconButton from '@material-ui/core/IconButton' -import Tooltip from '@material-ui/core/Tooltip' -import { alpha, makeStyles, useTheme } from '@material-ui/core/styles' -import RecordVoiceOverIcon from '@material-ui/icons/RecordVoiceOver' -import TranslateIcon from '@material-ui/icons/Translate' -import clsx from 'clsx' +import { alpha, makeStyles } from '@material-ui/core/styles' import React, { useCallback, useEffect, useRef, useState } from 'react' +import LyricsLayerControls from './LyricsLayerControls' import LyricsPanel from './LyricsPanel' import { LYRICS_SIDEBAR_MAX_WIDTH, @@ -79,57 +75,6 @@ const useStyles = makeStyles((theme) => ({ outline: 'none', }, }, - controls: { - position: 'absolute', - bottom: theme.spacing(1), - right: theme.spacing(0.75), - zIndex: 2, - display: 'flex', - alignItems: 'center', - gap: theme.spacing(0.25), - padding: theme.spacing(0.25), - borderRadius: theme.shape.borderRadius * 2, - backgroundColor: 'transparent', - }, - controlButton: { - padding: theme.spacing(0.75), - color: alpha(theme.palette.text.primary, 0.58), - backgroundColor: 'transparent', - transition: - 'color 160ms ease, background-color 160ms ease, transform 160ms cubic-bezier(0.23, 1, 0.32, 1)', - '@media (hover: hover) and (pointer: fine)': { - '&:hover': { - color: theme.palette.text.primary, - backgroundColor: alpha(theme.palette.primary.main, 0.08), - }, - '&$controlActive:hover': { - color: theme.palette.primary.main, - }, - }, - '&:focus-visible': { - color: theme.palette.text.primary, - backgroundColor: alpha(theme.palette.primary.main, 0.1), - }, - '&:active:not(:disabled)': { - transform: 'scale(0.97)', - }, - '&$controlActive': { - color: theme.palette.primary.main, - }, - '&$controlActive:focus-visible': { - color: theme.palette.primary.main, - }, - '&:disabled': { - color: alpha(theme.palette.text.primary, 0.28), - }, - '@media (prefers-reduced-motion: reduce)': { - transition: 'none', - '&:active:not(:disabled)': { - transform: 'none', - }, - }, - }, - controlActive: {}, panel: { flex: 1, minHeight: 0, @@ -137,41 +82,6 @@ const useStyles = makeStyles((theme) => ({ }, })) -const LayerButton = ({ - active, - classes, - disabled, - label, - onClick, - testId, - children, -}) => { - const theme = useTheme() - const activeStyle = - active && !disabled ? { color: theme.palette.primary.main } : undefined - - return ( - - - - {children} - - - - ) -} - const LyricsSidebar = ({ visible, mainLyric, @@ -362,39 +272,16 @@ const LyricsSidebar = ({ onKeyDown={handleResizeKeyDown} />
-
- - - - - - -
+ ', () => { expect(screen.getByLabelText('Resize song words')).toBeInTheDocument() expect(pronunciation).toHaveAttribute('aria-label', 'Conceal pronunciation') expect(pronunciation).toHaveAttribute('aria-pressed', 'true') - expect(window.getComputedStyle(pronunciation).color).toBe( - 'rgb(53, 170, 102)', - ) + expect(pronunciation.className).toContain('controlActive') fireEvent.click(pronunciation) expect(onTogglePronunciation).toHaveBeenCalledTimes(1) expect(translation).toHaveAttribute('aria-label', 'Reveal translation') expect(translation).toHaveAttribute('aria-pressed', 'false') expect(translation).toBeDisabled() - expect(window.getComputedStyle(translation).color).not.toBe( - 'rgb(53, 170, 102)', - ) + expect(translation.className).not.toContain('controlActive') fireEvent.click(translation) expect(onToggleTranslation).not.toHaveBeenCalled() }) diff --git a/ui/src/audioplayer/MobileKaraokeLyricsPortal.test.jsx b/ui/src/audioplayer/MobileKaraokeLyricsPortal.test.jsx index 935d822dd..3bc0d24da 100644 --- a/ui/src/audioplayer/MobileKaraokeLyricsPortal.test.jsx +++ b/ui/src/audioplayer/MobileKaraokeLyricsPortal.test.jsx @@ -18,8 +18,8 @@ import MobileKaraokeLyricsPortal, { } from './MobileKaraokeLyricsPortal' import usePlayerLyrics from './usePlayerLyrics' -const { useEnhancedLyricsMock } = vi.hoisted(() => ({ - useEnhancedLyricsMock: vi.fn(() => ({ +const { defaultLyricsResponse, useEnhancedLyricsMock } = vi.hoisted(() => { + const defaultLyricsResponse = { layers: { main: { synced: true, @@ -31,8 +31,12 @@ const { useEnhancedLyricsMock } = vi.hoisted(() => ({ loading: false, error: null, retry: vi.fn(), - })), -})) + } + return { + defaultLyricsResponse, + useEnhancedLyricsMock: vi.fn(() => defaultLyricsResponse), + } +}) vi.mock('./useEnhancedLyrics', () => ({ default: useEnhancedLyricsMock, @@ -77,7 +81,8 @@ const MobileLyricsHarness = ({ trackId = 'track-1', isRadio = false }) => { describe('', () => { afterEach(() => { vi.useRealTimers() - useEnhancedLyricsMock.mockClear() + useEnhancedLyricsMock.mockReset() + useEnhancedLyricsMock.mockImplementation(() => defaultLyricsResponse) cleanup() document.body.innerHTML = '' }) @@ -188,6 +193,60 @@ describe('', () => { ).not.toBeInTheDocument() }) + it('centers working pronunciation and translation toggles below mobile lyrics', () => { + useEnhancedLyricsMock.mockImplementation(() => ({ + ...defaultLyricsResponse, + layers: { + ...defaultLyricsResponse.layers, + pronunciation: { + synced: true, + line: [{ start: 0, end: 1000, value: 'Spoken lyrics' }], + }, + translation: { + synced: true, + line: [{ start: 0, end: 1000, value: 'Translated lyrics' }], + }, + }, + })) + const host = createHost() + render() + + fireEvent.click(screen.getByRole('button', { name: 'Toggle lyrics' })) + + const controls = within(host).getByTestId('lyrics-mobile-layer-controls') + expect(controls).toHaveAttribute('data-placement', 'mobile') + expect(controls).toHaveStyle({ + bottom: '8px', + left: '50%', + transform: 'translateX(-50%)', + }) + + const pronunciation = within(host).getByTestId( + 'toggle-pronunciation-button', + ) + const translation = within(host).getByTestId('toggle-translation-button') + expect(pronunciation).toHaveAttribute( + 'aria-label', + 'player.hideLyricsPronunciationText', + ) + expect(pronunciation).toHaveAttribute('aria-pressed', 'true') + expect(translation).toHaveAttribute( + 'aria-label', + 'player.hideLyricsTranslationText', + ) + expect(translation).toHaveAttribute('aria-pressed', 'true') + + fireEvent.click(pronunciation) + fireEvent.click(translation) + + expect(pronunciation).toHaveAttribute('aria-pressed', 'false') + expect(translation).toHaveAttribute('aria-pressed', 'false') + expect(within(host).queryByText('Spoken lyrics')).not.toBeInTheDocument() + expect( + within(host).queryByText('Translated lyrics'), + ).not.toBeInTheDocument() + }) + it('keeps open lyrics closable after playback switches to radio', () => { vi.useFakeTimers() const host = createHost() diff --git a/ui/src/audioplayer/lyricsKaraokeConstants.js b/ui/src/audioplayer/lyricsKaraokeConstants.js index bce62f151..629ccfa6b 100644 --- a/ui/src/audioplayer/lyricsKaraokeConstants.js +++ b/ui/src/audioplayer/lyricsKaraokeConstants.js @@ -1,22 +1,24 @@ 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_SCROLLBAR_VISIBLE_MS = 1400 export const KARAOKE_MANUAL_SCROLL_PAUSE_MS = 2200 export const KARAOKE_SCROLL_ANIMATION_MS = 300 -export const KARAOKE_LINE_ENTER_MS = KARAOKE_ANIMATION_MS +export const KARAOKE_LINE_ENTER_MS = KARAOKE_LINE_OPACITY_MS export const KARAOKE_LINE_LIFT_PX = 1.5 const KARAOKE_EASE_IN_OUT_X1 = 0.77 const KARAOKE_EASE_IN_OUT_Y1 = 0 const KARAOKE_EASE_IN_OUT_X2 = 0.175 const KARAOKE_EASE_IN_OUT_Y2 = 1 export const KARAOKE_LINE_MOTION_EASING = `cubic-bezier(${KARAOKE_EASE_IN_OUT_X1}, ${KARAOKE_EASE_IN_OUT_Y1}, ${KARAOKE_EASE_IN_OUT_X2}, ${KARAOKE_EASE_IN_OUT_Y2})` -export const KARAOKE_CHARACTER_LIFT_PX = 1.5 +export const KARAOKE_CHARACTER_LIFT_PX = KARAOKE_LINE_LIFT_PX export const KARAOKE_CHARACTER_RISE_MS = KARAOKE_ANIMATION_MS export const KARAOKE_CHARACTER_WAVE_SPAN_RATIO = 0.36 export const KARAOKE_CHARACTER_WAVE_SPAN_MAX_MS = 140 export const KARAOKE_TRANSLATION_OPACITY = 0.62 -export const KARAOKE_LINE_RELEASE_MS = KARAOKE_ANIMATION_MS +export const KARAOKE_IDLE_LAYER_OPACITY = 0.49 +export const KARAOKE_LINE_RELEASE_MS = KARAOKE_LINE_OPACITY_MS export const KARAOKE_SCROLL_PRE_ROLL_MS = 320 export const KARAOKE_DESKTOP_ACTIVE_LINE_ANCHOR_RATIO = 0.1 export const KARAOKE_INLINE_ACTIVE_LINE_ANCHOR_RATIO = 0.42 diff --git a/ui/src/audioplayer/lyricsSidebarState.js b/ui/src/audioplayer/lyricsSidebarState.js index c4fbbc6ba..15322a366 100644 --- a/ui/src/audioplayer/lyricsSidebarState.js +++ b/ui/src/audioplayer/lyricsSidebarState.js @@ -7,12 +7,10 @@ export const resolveLyricsSidebarState = ({ }) => ({ lyricsVisible: Boolean(lyricsVisiblePreference), showTranslation: Boolean( - (translationPreference == null ? true : translationPreference) && - hasTranslationLyric, + (translationPreference ?? true) && hasTranslationLyric, ), showPronunciation: Boolean( - (pronunciationPreference == null ? true : pronunciationPreference) && - hasPronunciationLyric, + (pronunciationPreference ?? true) && hasPronunciationLyric, ), }) @@ -22,7 +20,5 @@ export const toggleLayerPreference = ( defaultEnabled = true, ) => { if (!hasLayer) return false - const currentPreference = - previousPreference == null ? defaultEnabled : previousPreference - return !currentPreference + return !(previousPreference ?? defaultEnabled) } diff --git a/ui/src/audioplayer/lyricsTimeline.js b/ui/src/audioplayer/lyricsTimeline.js index c7a7157a2..325d6d494 100644 --- a/ui/src/audioplayer/lyricsTimeline.js +++ b/ui/src/audioplayer/lyricsTimeline.js @@ -1,6 +1,6 @@ import { KARAOKE_SCROLL_PRE_ROLL_MS } from './lyricsKaraokeConstants' -const finiteTime = (value) => { +export const finiteTime = (value) => { if (value == null || value === '') return null const number = Number(value) return Number.isFinite(number) ? number : null @@ -17,7 +17,7 @@ const upperBound = (items, time, getTime) => { return low } -const sameIndexes = (left, right) => +export const sameIndexes = (left, right) => left.length === right.length && left.every((value, index) => value === right[index]) diff --git a/ui/src/audioplayer/styles.js b/ui/src/audioplayer/styles.js index 5ceea079b..bc5283b0f 100644 --- a/ui/src/audioplayer/styles.js +++ b/ui/src/audioplayer/styles.js @@ -1,5 +1,8 @@ import { makeStyles } from '@material-ui/core/styles' +const MOBILE_LYRICS_TRANSITION = + 'opacity 260ms cubic-bezier(0.22, 1, 0.36, 1), transform 260ms cubic-bezier(0.22, 1, 0.36, 1)' + const useStyle = makeStyles( (theme) => ({ audioTitle: { @@ -42,7 +45,7 @@ const useStyle = makeStyles( flexDirection: 'column', }, '& .play-mode-title': { - 'pointer-events': 'none', + pointerEvents: 'none', }, '& .music-player-panel .panel-content div.img-rotate': { // Customize desktop player when cover animation is disabled @@ -70,6 +73,7 @@ const useStyle = makeStyles( maxWidth: '100%', height: '100%', aspectRatio: 'auto', + borderRadius: 0, alignItems: 'stretch', justifyContent: 'stretch', }, @@ -79,8 +83,7 @@ const useStyle = makeStyles( inset: 0, opacity: 0, transform: 'scale(0.985)', - transition: - 'opacity 260ms cubic-bezier(0.22, 1, 0.36, 1), transform 260ms cubic-bezier(0.22, 1, 0.36, 1)', + transition: MOBILE_LYRICS_TRANSITION, '@media (prefers-reduced-motion: reduce)': { transition: 'opacity 120ms linear', transform: 'none', @@ -98,8 +101,7 @@ const useStyle = makeStyles( { animationDuration: (props) => !props.enableCoverAnimation && '0s', objectFit: 'contain', // Fix cover display when image is not square - transition: - 'opacity 260ms cubic-bezier(0.22, 1, 0.36, 1), transform 260ms cubic-bezier(0.22, 1, 0.36, 1)', + transition: MOBILE_LYRICS_TRANSITION, '@media (prefers-reduced-motion: reduce)': { transition: 'opacity 120ms linear', transform: 'none', diff --git a/ui/src/audioplayer/useLyricsTimeline.js b/ui/src/audioplayer/useLyricsTimeline.js index 508da1069..6667426a9 100644 --- a/ui/src/audioplayer/useLyricsTimeline.js +++ b/ui/src/audioplayer/useLyricsTimeline.js @@ -20,6 +20,7 @@ import { buildLyricsTimeline, getTimelineScrollTarget, LyricTimelineCursor, + sameIndexes, tokenProgressAt, } from './lyricsTimeline' @@ -33,10 +34,6 @@ const mediaDurationMs = (audio) => { return Number.isFinite(seconds) && seconds > 0 ? seconds * 1000 : null } -const sameIndexes = (left, right) => - left.length === right.length && - left.every((value, index) => value === right[index]) - const setProgress = (record, value) => { const next = Math.max(0, Math.min(1, value)) if (record.progress != null && Math.abs(record.progress - next) < 0.001) diff --git a/ui/src/audioplayer/useLyricsTimeline.test.js b/ui/src/audioplayer/useLyricsTimeline.test.js index 4c39a45fb..841fedbd9 100644 --- a/ui/src/audioplayer/useLyricsTimeline.test.js +++ b/ui/src/audioplayer/useLyricsTimeline.test.js @@ -6,7 +6,9 @@ import { KARAOKE_CHARACTER_RISE_MS, KARAOKE_CHARACTER_WAVE_SPAN_MAX_MS, KARAOKE_CHARACTER_WAVE_SPAN_RATIO, + KARAOKE_CLOCK_DRIFT_RESET_MS, KARAOKE_HIGHLIGHT_LEAD_MS, + KARAOKE_LINE_RELEASE_MS, } from './lyricsKaraokeConstants' import useLyricsTimeline from './useLyricsTimeline' @@ -233,7 +235,8 @@ describe('useLyricsTimeline', () => { 'first', ) - syncNow(result, 1259) + const releaseEnd = 1000 + KARAOKE_LINE_RELEASE_MS + syncNow(result, releaseEnd - 1) const gradient = tokenNode.style.backgroundImage const releaseAlpha = Number( tokenNode.style.getPropertyValue('--lyrics-token-active-alpha'), @@ -242,7 +245,7 @@ describe('useLyricsTimeline', () => { expect(tokenNode.style.color).toBe('transparent') expect(tokenNode.style.opacity).toBe('1') - syncNow(result, 1260) + syncNow(result, releaseEnd) const pastAlpha = Number( tokenNode.style.getPropertyValue('--lyrics-token-active-alpha'), @@ -286,7 +289,7 @@ describe('useLyricsTimeline', () => { expect(character.style.transform).toBe(''), ) - syncNow(result, tokenStart + KARAOKE_CHARACTER_RISE_MS / 2) + syncNow(result, tokenStart + KARAOKE_CHARACTER_WAVE_SPAN_MAX_MS / 2) expect(characters[0].style.transform).not.toBe(fullCharacterLift) expect(characters[0].style.transform).not.toBe('') expect(characters[4].style.transform).toBe('') @@ -376,9 +379,9 @@ describe('useLyricsTimeline', () => { /^translate3d\(0, -\d+\.\d{4}px, 0\)$/, ) expect(character.style.transform).not.toBe(fullCharacterLift) - syncNow(result, tokenStart + 240) + syncNow(result, tokenStart + KARAOKE_CHARACTER_RISE_MS - 20) expect(character.style.transform).not.toBe(fullCharacterLift) - syncNow(result, tokenStart + 260) + syncNow(result, tokenStart + KARAOKE_CHARACTER_RISE_MS) expect(character.style.transform).toBe(fullCharacterLift) expect( Number(tokenNode.style.getPropertyValue('--lyrics-progress')), @@ -485,7 +488,7 @@ describe('useLyricsTimeline', () => { const character = tokenNode.querySelector('[data-lyrics-character="true"]') expect(character.style.transform).toBe('') - now = KARAOKE_CHARACTER_RISE_MS / 4 + now = KARAOKE_CLOCK_DRIFT_RESET_MS / 4 act(() => frameCallback()) expect(character.style.transform).toMatch( /^translate3d\(0, -\d+\.\d{4}px, 0\)$/, @@ -525,7 +528,14 @@ describe('useLyricsTimeline', () => { act(() => { window.document.dispatchEvent(new Event('visibilitychange')) }) - audio.currentTime = 0.8 + const characterMotionEnd = + 500 + + Math.min( + 500 * KARAOKE_CHARACTER_WAVE_SPAN_RATIO, + KARAOKE_CHARACTER_WAVE_SPAN_MAX_MS, + ) + + KARAOKE_CHARACTER_RISE_MS + audio.currentTime = (characterMotionEnd - KARAOKE_HIGHLIGHT_LEAD_MS) / 1000 visibility = 'visible' act(() => { window.document.dispatchEvent(new Event('visibilitychange')) diff --git a/ui/src/audioplayer/usePlayerLyrics.jsx b/ui/src/audioplayer/usePlayerLyrics.jsx index ad9971e27..4cf6630dd 100644 --- a/ui/src/audioplayer/usePlayerLyrics.jsx +++ b/ui/src/audioplayer/usePlayerLyrics.jsx @@ -1,4 +1,5 @@ import React, { useCallback, useEffect, useMemo, useState } from 'react' +import LyricsLayerControls from './LyricsLayerControls' import LyricsPanel from './LyricsPanel' import MobileKaraokeLyricsPortal from './MobileKaraokeLyricsPortal' import { hasStructuredLyricContent } from './lyrics' @@ -157,10 +158,23 @@ const usePlayerLyrics = ({ labels={labels} inline /> + ), [ audioInstance, + hasPronunciationLyric, + hasTranslationLyric, lyricLayers.main, lyricLayers.pronunciation, lyricLayers.translation, @@ -169,6 +183,8 @@ const usePlayerLyrics = ({ labels, showPronunciation, showTranslation, + togglePronunciation, + toggleTranslation, useInlineMobileLyrics, ], )