diff --git a/ui/src/audioplayer/LyricsPanel.jsx b/ui/src/audioplayer/LyricsPanel.jsx index 41483f6a2..d8f04dd1d 100644 --- a/ui/src/audioplayer/LyricsPanel.jsx +++ b/ui/src/audioplayer/LyricsPanel.jsx @@ -25,7 +25,6 @@ import { KARAOKE_LINE_MOTION_EASING, KARAOKE_MANUAL_SCROLL_PAUSE_MS, KARAOKE_SCROLLBAR_VISIBLE_MS, - KARAOKE_TRANSLATION_IDLE_OPACITY, } from './lyricsKaraokeConstants' import { animateScrollTop, @@ -112,7 +111,6 @@ const useStyles = makeStyles((theme) => ({ '--lyrics-translation-current-color': 'var(--lyrics-translation-idle-color, currentColor)', '--lyrics-layer-opacity': KARAOKE_IDLE_LAYER_OPACITY, - '--lyrics-translation-opacity': KARAOKE_TRANSLATION_IDLE_OPACITY, transform: 'translateY(0)', transition: `background-color 150ms ${KARAOKE_LINE_MOTION_EASING}`, '&[role="button"]:focus-visible': { @@ -136,7 +134,6 @@ const useStyles = makeStyles((theme) => ({ '--lyrics-translation-current-color': 'var(--lyrics-translation-active-color, var(--lyrics-translation-idle-color, currentColor))', '--lyrics-layer-opacity': 1, - '--lyrics-translation-opacity': 1, }, '@media (prefers-reduced-motion: reduce)': { transition: 'none', @@ -182,17 +179,17 @@ const useStyles = makeStyles((theme) => ({ }, auxLine: { display: 'block', + opacity: 1, fontWeight: 600, fontSize: 15, lineHeight: KARAOKE_AUX_LINE_HEIGHT, overflowWrap: 'anywhere', whiteSpace: 'pre-wrap', letterSpacing: 0, - opacity: 'var(--lyrics-translation-opacity)', color: 'var(--lyrics-translation-current-color, currentColor)', WebkitTextFillColor: 'var(--lyrics-translation-current-color, currentColor)', - transition: KARAOKE_LAYER_OPACITY_TRANSITION, + transition: 'none', '@media (prefers-reduced-motion: reduce)': { transition: 'none', transform: 'none', @@ -412,6 +409,7 @@ const usePrefersReducedMotion = () => { const LyricsPanel = ({ visible = true, + obscured = false, mainLyric, translationLyric, pronunciationLyric, @@ -436,7 +434,9 @@ const LyricsPanel = ({ const [hasTopFade, setHasTopFade] = useState(false) const [scrollEndPadding, setScrollEndPadding] = useState(0) const [scrollStartPadding, setScrollStartPadding] = useState(0) + const [focusedLineIndex, setFocusedLineIndex] = useState(null) const prefersReducedMotion = usePrefersReducedMotion() + const timelineVisible = visible && !obscured const activeLineAnchorRatio = inline ? KARAOKE_INLINE_ACTIVE_LINE_ANCHOR_RATIO : KARAOKE_DESKTOP_ACTIVE_LINE_ANCHOR_RATIO @@ -466,10 +466,26 @@ const LyricsPanel = ({ } = useLyricsTimeline({ lines: mainLines, audioInstance, - visible: visible && hasTimedMainLines, + visible: timelineVisible && hasTimedMainLines, reducedMotion: prefersReducedMotion, }) const activeIndexSet = useMemo(() => new Set(activeIndexes), [activeIndexes]) + const seekableLineIndexes = useMemo(() => { + if (!audioInstance) return [] + return mainLines.reduce((indexes, line, index) => { + if (line.renderable !== false && line.start != null) indexes.push(index) + return indexes + }, []) + }, [audioInstance, mainLines]) + const tabbableLineIndex = seekableLineIndexes.includes(focusedLineIndex) + ? focusedLineIndex + : seekableLineIndexes.includes(activeIndex) + ? activeIndex + : seekableLineIndexes[0] + + useEffect(() => { + setFocusedLineIndex(null) + }, [mainLyric]) const trByMainIndex = useMemo(() => { if (!showTranslation || translationLines.length === 0) return {} @@ -630,7 +646,7 @@ const LyricsPanel = ({ }, [clearManualScrollTimer, mainLyric, visible]) useEffect(() => { - if (!visible || !hasTimedMainLines || scrollTargetIndex < 0) { + if (!timelineVisible || !hasTimedMainLines || scrollTargetIndex < 0) { cancelScrollAnimation(scrollAnimationRef) return undefined } @@ -661,7 +677,7 @@ const LyricsPanel = ({ layoutVersion, prefersReducedMotion, scrollTargetIndex, - visible, + timelineVisible, ]) if (!visible) return null @@ -697,6 +713,38 @@ const LyricsPanel = ({ resumeAutoScroll() } + const focusSeekableLine = (index) => { + setFocusedLineIndex(index) + getLineNode(index)?.focus() + } + + const handleLineKeyDown = (event, index, line) => { + if (event.key === 'Enter' || event.key === ' ') { + event.preventDefault() + seekToLine(line) + return + } + + const currentPosition = seekableLineIndexes.indexOf(index) + if (currentPosition < 0) return + let nextPosition = null + if (event.key === 'ArrowDown' || event.key === 'ArrowRight') { + nextPosition = Math.min( + currentPosition + 1, + seekableLineIndexes.length - 1, + ) + } else if (event.key === 'ArrowUp' || event.key === 'ArrowLeft') { + nextPosition = Math.max(currentPosition - 1, 0) + } else if (event.key === 'Home') { + nextPosition = 0 + } else if (event.key === 'End') { + nextPosition = seekableLineIndexes.length - 1 + } + if (nextPosition == null) return + event.preventDefault() + focusSeekableLine(seekableLineIndexes[nextPosition]) + } + return (
seekToLine(line)} + onFocus={ + canSeekLine ? () => setFocusedLineIndex(idx) : undefined + } + onBlur={ + canSeekLine + ? (event) => { + if (!bodyRef.current?.contains(event.relatedTarget)) { + setFocusedLineIndex(null) + } + } + : undefined + } onMouseDown={ canSeekLine ? (event) => { @@ -789,15 +851,11 @@ const LyricsPanel = ({ } : undefined } - onKeyDown={(event) => { - if ( - canSeekLine && - (event.key === 'Enter' || event.key === ' ') - ) { - event.preventDefault() - seekToLine(line) - } - }} + onKeyDown={ + canSeekLine + ? (event) => handleLineKeyDown(event, idx, line) + : undefined + } > {lineLanes.length > 1 ? (
', () => { expect(pronunciation.style.color).toBe('') }) - it('dims inactive translations without moving them', () => { + it('keeps translations subordinate and stable between line states', () => { const propsAt = (currentTime) => ({ mainLyric, pronunciationLyric: { @@ -388,17 +387,17 @@ describe('', () => { expect(window.getComputedStyle(mainRow).transition).toContain( `opacity ${KARAOKE_LINE_OPACITY_MS}ms ${KARAOKE_EASING}`, ) - expect(window.getComputedStyle(translationRow).transition).toContain( - `opacity ${KARAOKE_LINE_OPACITY_MS}ms ${KARAOKE_EASING}`, - ) + expect(window.getComputedStyle(translationRow).transition).toBe('none') pronunciation.forEach((token) => expect(token).toHaveAttribute('data-timed', 'false'), ) + expect(window.getComputedStyle(translationRow).opacity).toBe('1') expect( - window - .getComputedStyle(lineGroup) - .getPropertyValue('--lyrics-translation-opacity'), - ).toBe('1') + lineGroup.style.getPropertyValue('--lyrics-translation-idle-color'), + ).toBe(theme.palette.text.secondary) + expect( + lineGroup.style.getPropertyValue('--lyrics-translation-active-color'), + ).toBe(theme.palette.text.secondary) expect(window.getComputedStyle(translationRow).transform).toBe('') expect(window.getComputedStyle(translationRow).marginTop).toBe( `${theme.spacing(0.6)}px`, @@ -406,12 +405,8 @@ describe('', () => { rerenderPanel(propsAt(1.1)) - const inactiveOpacity = Number( - window - .getComputedStyle(lineGroup) - .getPropertyValue('--lyrics-translation-opacity'), - ) - expect(inactiveOpacity).toBe(KARAOKE_TRANSLATION_IDLE_OPACITY) + expect(window.getComputedStyle(translationRow).opacity).toBe('1') + expect(window.getComputedStyle(translationRow).transition).toBe('none') expect(window.getComputedStyle(translationRow).transform).toBe('') }) @@ -766,6 +761,97 @@ describe('', () => { expect(audioInstance.currentTime).toBe(2.3) }) + it('uses roving focus for seekable lines without seeking on navigation', () => { + const audioInstance = createAudio({ currentTime: 1.5 }) + renderPanel({ + mainLyric: { + synced: true, + line: [ + { start: 0, end: 900, value: 'First seekable line' }, + { value: 'Untimed line' }, + { start: 1000, end: 1900, value: 'Active seekable line' }, + { start: 2000, end: 2900, value: 'Last seekable line' }, + ], + }, + audioInstance, + }) + + const first = screen + .getByText('First seekable line') + .closest('[data-testid="lyrics-line-group"]') + const untimed = screen + .getByText('Untimed line') + .closest('[data-testid="lyrics-line-group"]') + const active = screen + .getByText('Active seekable line') + .closest('[data-testid="lyrics-line-group"]') + const last = screen + .getByText('Last seekable line') + .closest('[data-testid="lyrics-line-group"]') + + expect(first).toHaveAttribute('tabindex', '-1') + expect(untimed).not.toHaveAttribute('tabindex') + expect(active).toHaveAttribute('tabindex', '0') + expect(last).toHaveAttribute('tabindex', '-1') + + active.focus() + fireEvent.keyDown(active, { key: 'ArrowDown' }) + expect(document.activeElement).toBe(last) + expect(last).toHaveAttribute('tabindex', '0') + expect(audioInstance.currentTime).toBe(1.5) + + fireEvent.keyDown(last, { key: 'ArrowRight' }) + expect(document.activeElement).toBe(last) + fireEvent.keyDown(last, { key: 'ArrowUp' }) + expect(document.activeElement).toBe(active) + fireEvent.keyDown(active, { key: 'ArrowLeft' }) + expect(document.activeElement).toBe(first) + expect(audioInstance.currentTime).toBe(1.5) + + fireEvent.keyDown(first, { key: 'End' }) + expect(document.activeElement).toBe(last) + fireEvent.keyDown(last, { key: 'Home' }) + expect(document.activeElement).toBe(first) + expect(audioInstance.currentTime).toBe(1.5) + + fireEvent.keyDown(first, { key: ' ' }) + expect(audioInstance.currentTime).toBe(0) + fireEvent.keyDown(last, { key: 'Enter' }) + expect(audioInstance.currentTime).toBe(2) + }) + + it('keeps lyrics mounted but pauses timeline frames while obscured', () => { + const audioInstance = createAudio({ currentTime: 0.25 }) + audioInstance.paused = false + window.requestAnimationFrame.mockImplementation(() => 7) + window.requestAnimationFrame.mockClear() + window.cancelAnimationFrame.mockClear() + + const { rerenderPanel } = renderPanel({ + mainLyric: tokenizedMainLyric, + audioInstance, + obscured: true, + }) + + expect(screen.getByTestId('karaoke-lyrics-panel')).toBeInTheDocument() + expect(window.requestAnimationFrame).not.toHaveBeenCalled() + + rerenderPanel({ + mainLyric: tokenizedMainLyric, + audioInstance, + obscured: false, + }) + expect(window.requestAnimationFrame).toHaveBeenCalled() + + rerenderPanel({ + mainLyric: tokenizedMainLyric, + audioInstance, + obscured: true, + }) + expect(screen.getByTestId('karaoke-lyrics-panel')).toBeInTheDocument() + expect(window.cancelAnimationFrame).toHaveBeenCalledWith(7) + }) + it('pauses auto-scroll only for genuine manual scroll intent', async () => { vi.useFakeTimers() const requestAnimationFrameSpy = vi @@ -834,21 +920,23 @@ describe('', () => { const accessibleTheme = createTheme({ palette: { type } }) const background = accessibleTheme.palette.background.default - renderPanel( - { - mainLyric: tokenizedMainLyric, - translationLyric: { - synced: true, - line: [{ start: 0, end: 1000, value: 'Readable translation' }], - }, - showTranslation: true, - audioInstance: { currentTime: 0.25, paused: true }, + const propsAt = (currentTime) => ({ + mainLyric: tokenizedMainLyric, + translationLyric: { + synced: true, + line: [{ start: 0, end: 1000, value: 'Readable translation' }], }, - accessibleTheme, - ) + showTranslation: true, + audioInstance: { currentTime, paused: true }, + }) + const { rerenderPanel } = renderPanel(propsAt(0.25), accessibleTheme) const group = screen.getByTestId('lyrics-line-group') - expect(screen.getByText('Readable translation')).toBeInTheDocument() + const translation = screen + .getByText('Readable translation') + .closest('[data-tokenized]') + expect(translation).toBeInTheDocument() + expect(window.getComputedStyle(translation).opacity).toBe('1') const token = screen.getAllByTestId('lyrics-token')[0] const gradientNode = [token, ...token.querySelectorAll('*')].find( (node) => node.style.backgroundImage.includes('linear-gradient'), @@ -865,11 +953,6 @@ describe('', () => { expect( group.style.getPropertyValue('--lyrics-translation-active-color'), ).toBe(accessibleTheme.palette.text.secondary) - expect( - window - .getComputedStyle(group) - .getPropertyValue('--lyrics-translation-opacity'), - ).toBe('1') expect(parseCssColor(futureColor).alpha).toBe(TOKEN_FUTURE_ALPHA) expect(contrastRatio(futureColor, background)).toBeGreaterThanOrEqual(4.5) expect( @@ -880,6 +963,16 @@ describe('', () => { ).toBeLessThan( contrastRatio(accessibleTheme.palette.text.primary, background), ) + + rerenderPanel(propsAt(1.25)) + expect(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/LyricsSidebar.jsx b/ui/src/audioplayer/LyricsSidebar.jsx index 04f42512e..cb4aa5867 100644 --- a/ui/src/audioplayer/LyricsSidebar.jsx +++ b/ui/src/audioplayer/LyricsSidebar.jsx @@ -1,16 +1,15 @@ import { alpha, makeStyles } from '@material-ui/core/styles' -import React, { useCallback, useEffect, useRef, useState } from 'react' +import React, { useCallback, useEffect, useRef } from 'react' import LyricsLayerControls from './LyricsLayerControls' import LyricsPanel from './LyricsPanel' import { + LYRICS_SIDEBAR_DEFAULT_WIDTH, LYRICS_SIDEBAR_MAX_WIDTH, LYRICS_SIDEBAR_MIN_WIDTH, LYRICS_SIDEBAR_TRANSITION_MS, LYRICS_SIDEBAR_WIDTH_STEP, + clampSidebarMaxWidth, clampSidebarWidth, - loadSidebarWidth, - notifySidebarWidthChange, - saveSidebarWidth, } from './lyricsSidebarWidth' import useEnterExitTransition from './useEnterExitTransition' import useRestoreFocusOnExit from './useRestoreFocusOnExit' @@ -83,8 +82,14 @@ const useStyles = makeStyles((theme) => ({ }, })) +const noop = () => {} + const LyricsSidebar = ({ visible, + obscuredByQueue = false, + width = LYRICS_SIDEBAR_DEFAULT_WIDTH, + maxWidth = LYRICS_SIDEBAR_MAX_WIDTH, + onWidthChange = noop, mainLyric, translationLyric, pronunciationLyric, @@ -100,27 +105,28 @@ const LyricsSidebar = ({ labels = {}, returnFocusRef, }) => { - const [width, setWidth] = useState(loadSidebarWidth) + const resolvedMaxWidth = clampSidebarMaxWidth(maxWidth) + const resolvedWidth = clampSidebarWidth(width, resolvedMaxWidth) const sidebarRef = useRef(null) const resizerRef = useRef(null) - const widthRef = useRef(width) + const widthRef = useRef(resolvedWidth) const resizeCleanupRef = useRef(null) const { rendered, entered } = useEnterExitTransition( visible, LYRICS_SIDEBAR_TRANSITION_MS, ) - const [isResizing, setIsResizing] = useState(false) + const interactive = entered && !obscuredByQueue const classes = useStyles() useRestoreFocusOnExit({ surfaceRef: sidebarRef, - entered, + entered: interactive, returnFocusRef, }) useEffect(() => { - widthRef.current = width - }, [width]) + widthRef.current = resolvedWidth + }, [resolvedWidth]) useEffect( () => () => { @@ -130,26 +136,21 @@ const LyricsSidebar = ({ [], ) - const applyWidth = useCallback((next) => { - const resolvedWidth = clampSidebarWidth( - typeof next === 'function' ? next(widthRef.current) : next, - ) - widthRef.current = resolvedWidth - if (sidebarRef.current) { - sidebarRef.current.style.width = `${resolvedWidth}px` - } - resizerRef.current?.setAttribute('aria-valuenow', String(resolvedWidth)) - notifySidebarWidthChange(resolvedWidth) - return resolvedWidth - }, []) - - const updateWidth = useCallback( + const applyWidth = useCallback( (next, { persist = false } = {}) => { - const resolvedWidth = applyWidth(next) - setWidth(resolvedWidth) - if (persist) saveSidebarWidth(resolvedWidth) + const nextWidth = clampSidebarWidth( + typeof next === 'function' ? next(widthRef.current) : next, + resolvedMaxWidth, + ) + widthRef.current = nextWidth + if (sidebarRef.current) { + sidebarRef.current.style.width = `${nextWidth}px` + } + resizerRef.current?.setAttribute('aria-valuenow', String(nextWidth)) + onWidthChange(nextWidth, { persist }) + return nextWidth }, - [applyWidth], + [onWidthChange, resolvedMaxWidth], ) const handleResizePointerDown = useCallback( @@ -166,7 +167,6 @@ const LyricsSidebar = ({ const canCapture = pointerId != null && typeof target.setPointerCapture === 'function' const listenerTarget = window - setIsResizing(true) const handlePointerMove = (moveEvent) => { latestWidth = applyWidth(startWidth + startX - moveEvent.clientX) @@ -190,9 +190,7 @@ const LyricsSidebar = ({ // Ignore stale pointer capture state. } } - setWidth(latestWidth) - if (persist) saveSidebarWidth(latestWidth) - setIsResizing(false) + if (persist) onWidthChange(latestWidth, { persist: true }) if (resizeCleanupRef.current === cleanupResize) { resizeCleanupRef.current = null } @@ -214,30 +212,30 @@ const LyricsSidebar = ({ } } }, - [applyWidth], + [applyWidth, onWidthChange], ) const handleResizeKeyDown = useCallback( (event) => { if (event.key === 'ArrowLeft') { event.preventDefault() - updateWidth((current) => current + LYRICS_SIDEBAR_WIDTH_STEP, { + applyWidth((current) => current + LYRICS_SIDEBAR_WIDTH_STEP, { persist: true, }) } else if (event.key === 'ArrowRight') { event.preventDefault() - updateWidth((current) => current - LYRICS_SIDEBAR_WIDTH_STEP, { + applyWidth((current) => current - LYRICS_SIDEBAR_WIDTH_STEP, { persist: true, }) } else if (event.key === 'Home') { event.preventDefault() - updateWidth(LYRICS_SIDEBAR_MIN_WIDTH, { persist: true }) + applyWidth(LYRICS_SIDEBAR_MIN_WIDTH, { persist: true }) } else if (event.key === 'End') { event.preventDefault() - updateWidth(LYRICS_SIDEBAR_MAX_WIDTH, { persist: true }) + applyWidth(resolvedMaxWidth, { persist: true }) } }, - [updateWidth], + [applyWidth, resolvedMaxWidth], ) if (!rendered) return null @@ -248,15 +246,14 @@ const LyricsSidebar = ({ data-testid="lyrics-sidebar" ref={sidebarRef} style={{ - width: widthRef.current, + width: resolvedWidth, transform: entered ? 'translateX(0)' : 'translateX(100%)', opacity: entered ? 1 : 0, - pointerEvents: entered ? 'auto' : 'none', + pointerEvents: interactive ? 'auto' : 'none', }} aria-label={labels.title || 'Lyrics'} - aria-hidden={!entered} - inert={entered ? undefined : ''} - data-resizing={isResizing ? 'true' : 'false'} + aria-hidden={!interactive} + inert={interactive ? undefined : ''} >
, diff --git a/ui/src/audioplayer/MobileKaraokeLyricsPortal.test.jsx b/ui/src/audioplayer/MobileKaraokeLyricsPortal.test.jsx index 7d5522d6a..2676db7f5 100644 --- a/ui/src/audioplayer/MobileKaraokeLyricsPortal.test.jsx +++ b/ui/src/audioplayer/MobileKaraokeLyricsPortal.test.jsx @@ -14,8 +14,8 @@ import MobileKaraokeLyricsPortal, { MOBILE_KARAOKE_LYRICS_ENTERED_CLASS, MOBILE_KARAOKE_LYRICS_HOST_SELECTOR, MOBILE_KARAOKE_LYRICS_LAYER_CLASS, - MOBILE_KARAOKE_LYRICS_TRANSITION_MS, } from './MobileKaraokeLyricsPortal' +import { MOBILE_KARAOKE_LYRICS_TRANSITION_MS } from './lyricsKaraokeConstants' import usePlayerLyrics from './usePlayerLyrics' const { defaultLyricsResponse, useEnhancedLyricsMock } = vi.hoisted(() => { @@ -49,19 +49,28 @@ const createHost = () => { return host } -const portal = (text, active = true, returnFocusRef) => ( - +const portal = (text, active = true, returnFocusRef, obscured = false) => ( + {text} ) -const MobileLyricsHarness = ({ trackId = 'track-1', isRadio = false }) => { +const MobileLyricsHarness = ({ + trackId = 'track-1', + isRadio = false, + obscuredByQueue = false, +}) => { const { toolbarLyricsProps, mobileLyricsSurface } = usePlayerLyrics({ trackId, trackUpdatedAt: 'now', isRadio, audioInstance: null, isDesktop: false, + obscuredByQueue, translate: (key) => key, }) return ( @@ -157,6 +166,38 @@ describe('', () => { expect(document.activeElement).toBe(returnTarget) }) + it('keeps open lyrics mounted and inert while the queue obscures them', async () => { + const host = createHost() + const { rerender } = render() + + fireEvent.click(screen.getByRole('button', { name: 'Toggle lyrics' })) + const layer = host.querySelector(`.${MOBILE_KARAOKE_LYRICS_LAYER_CLASS}`) + const panel = within(host).getByTestId('karaoke-lyrics-panel') + await waitFor(() => expect(layer).toHaveAttribute('aria-hidden', 'false')) + + rerender() + + expect(host).toHaveClass(MOBILE_KARAOKE_LYRICS_ACTIVE_CLASS) + expect(host).toHaveClass(MOBILE_KARAOKE_LYRICS_ENTERED_CLASS) + expect(host.querySelector(`.${MOBILE_KARAOKE_LYRICS_LAYER_CLASS}`)).toBe( + layer, + ) + expect(within(host).getByTestId('karaoke-lyrics-panel')).toBe(panel) + expect(layer).toHaveAttribute('aria-hidden', 'true') + expect(layer).toHaveAttribute('inert') + expect(layer).toHaveStyle({ pointerEvents: 'none' }) + expect(useEnhancedLyricsMock).toHaveBeenLastCalledWith( + expect.objectContaining({ requested: true }), + ) + + rerender() + + expect(within(host).getByTestId('karaoke-lyrics-panel')).toBe(panel) + await waitFor(() => expect(layer).toHaveAttribute('aria-hidden', 'false')) + expect(layer).not.toHaveAttribute('inert') + expect(layer).toHaveStyle({ pointerEvents: 'auto' }) + }) + it('attaches when the mobile cover host appears after activation', async () => { vi.useFakeTimers() @@ -248,7 +289,6 @@ describe('', () => { 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%', diff --git a/ui/src/audioplayer/Player.jsx b/ui/src/audioplayer/Player.jsx index 5aa9fdaf3..f20828d44 100644 --- a/ui/src/audioplayer/Player.jsx +++ b/ui/src/audioplayer/Player.jsx @@ -54,6 +54,7 @@ const Player = () => { const currentTrackIdRef = useRef(null) const stoppedRef = useRef(false) const [audioInstance, setAudioInstance] = useState(null) + const [audioListsPanelVisible, setAudioListsPanelVisible] = useState(false) const isDesktop = useMediaQuery(PLAYER_DESKTOP_MEDIA_QUERY) const isMobilePlayer = /Android|webOS|iPhone|iPad|iPod|BlackBerry|IEMobile|Opera Mini/i.test( @@ -158,6 +159,7 @@ const Player = () => { isRadio: playerState.current?.isRadio || false, audioInstance, isDesktop, + obscuredByQueue: audioListsPanelVisible, translate, }) @@ -501,6 +503,7 @@ const Player = () => { {...options} className={classes.player} onAudioListsChange={onAudioListsChange} + onAudioListsPanelChange={setAudioListsPanelVisible} onAudioVolumeChange={onAudioVolumeChange} onAudioProgress={onAudioProgress} onAudioPlay={onAudioPlay} diff --git a/ui/src/audioplayer/lyricsKaraokeConstants.js b/ui/src/audioplayer/lyricsKaraokeConstants.js index a542694e8..28dc1b917 100644 --- a/ui/src/audioplayer/lyricsKaraokeConstants.js +++ b/ui/src/audioplayer/lyricsKaraokeConstants.js @@ -19,9 +19,9 @@ 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_IDLE_OPACITY = 0.62 export const KARAOKE_IDLE_LAYER_OPACITY = 0.49 export const KARAOKE_LINE_RELEASE_MS = KARAOKE_LINE_OPACITY_MS +export const MOBILE_KARAOKE_LYRICS_TRANSITION_MS = 260 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/lyricsSidebarWidth.js b/ui/src/audioplayer/lyricsSidebarWidth.js index 9c72deacb..2d0152d69 100644 --- a/ui/src/audioplayer/lyricsSidebarWidth.js +++ b/ui/src/audioplayer/lyricsSidebarWidth.js @@ -1,14 +1,14 @@ export const LYRICS_SIDEBAR_STORAGE_KEY = 'nd.lyricsSidebar.width.v1' -export const LYRICS_SIDEBAR_WIDTH_EVENT = 'nd:lyrics-sidebar-width' export const LYRICS_SIDEBAR_DEFAULT_WIDTH = 360 export const LYRICS_SIDEBAR_MIN_WIDTH = 300 export const LYRICS_SIDEBAR_MAX_WIDTH = 520 +export const LYRICS_ROUTE_MIN_WIDTH = 320 export const LYRICS_SIDEBAR_WIDTH_STEP = 20 export const LYRICS_SIDEBAR_TRANSITION_MS = 260 -export const clampSidebarWidth = (value) => { +export const clampSidebarMaxWidth = (value) => { const numeric = Number(value) - if (!Number.isFinite(numeric)) return LYRICS_SIDEBAR_DEFAULT_WIDTH + if (!Number.isFinite(numeric)) return LYRICS_SIDEBAR_MAX_WIDTH return Math.round( Math.min( LYRICS_SIDEBAR_MAX_WIDTH, @@ -17,6 +17,28 @@ export const clampSidebarWidth = (value) => { ) } +export const clampSidebarWidth = ( + value, + maxWidth = LYRICS_SIDEBAR_MAX_WIDTH, +) => { + const resolvedMaxWidth = clampSidebarMaxWidth(maxWidth) + const numeric = Number(value) + if (!Number.isFinite(numeric)) { + return Math.min(LYRICS_SIDEBAR_DEFAULT_WIDTH, resolvedMaxWidth) + } + return Math.round( + Math.min(resolvedMaxWidth, Math.max(LYRICS_SIDEBAR_MIN_WIDTH, numeric)), + ) +} + +export const getSidebarMaxWidth = (contentWidth) => { + const numeric = Number(contentWidth) + if (!Number.isFinite(numeric) || numeric <= 0) { + return LYRICS_SIDEBAR_MAX_WIDTH + } + return clampSidebarMaxWidth(numeric - LYRICS_ROUTE_MIN_WIDTH) +} + const hasLocalStorage = () => { try { return typeof window !== 'undefined' && Boolean(window.localStorage) @@ -37,16 +59,6 @@ export const loadSidebarWidth = () => { } } -export const notifySidebarWidthChange = (width) => { - if (typeof window === 'undefined') return - - window.dispatchEvent( - new CustomEvent(LYRICS_SIDEBAR_WIDTH_EVENT, { - detail: { width: clampSidebarWidth(width) }, - }), - ) -} - export const saveSidebarWidth = (width) => { const resolvedWidth = clampSidebarWidth(width) if (!hasLocalStorage()) return diff --git a/ui/src/audioplayer/styles.js b/ui/src/audioplayer/styles.js index 84c5e1483..dae045807 100644 --- a/ui/src/audioplayer/styles.js +++ b/ui/src/audioplayer/styles.js @@ -1,8 +1,8 @@ import { makeStyles } from '@material-ui/core/styles' +import { MOBILE_KARAOKE_LYRICS_TRANSITION_MS } from './lyricsKaraokeConstants' import { PLAYER_MOBILE_MEDIA_QUERY } from './playerBreakpoints' -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 MOBILE_LYRICS_TRANSITION = `opacity ${MOBILE_KARAOKE_LYRICS_TRANSITION_MS}ms cubic-bezier(0.22, 1, 0.36, 1), transform ${MOBILE_KARAOKE_LYRICS_TRANSITION_MS}ms cubic-bezier(0.22, 1, 0.36, 1)` const useStyle = makeStyles( (theme) => ({ @@ -48,6 +48,9 @@ const useStyle = makeStyles( '& .play-mode-title': { pointerEvents: 'none', }, + '& .audio-lists-panel': { + zIndex: theme.zIndex.appBar, + }, '& .music-player-panel .panel-content div.img-rotate': { // Customize desktop player when cover animation is disabled animationDuration: (props) => !props.enableCoverAnimation && '0s', diff --git a/ui/src/audioplayer/usePlayerLyrics.jsx b/ui/src/audioplayer/usePlayerLyrics.jsx index 429d6ae1f..e80f4cbfd 100644 --- a/ui/src/audioplayer/usePlayerLyrics.jsx +++ b/ui/src/audioplayer/usePlayerLyrics.jsx @@ -15,6 +15,7 @@ const usePlayerLyrics = ({ isRadio, audioInstance, isDesktop, + obscuredByQueue = false, translate, }) => { const [lyricsVisiblePreference, setLyricsVisiblePreference] = useState(false) @@ -125,6 +126,7 @@ const usePlayerLyrics = ({ loading: lyricsLoading, error: lyricsError, labels, + obscuredByQueue, returnFocusRef: lyricsToggleRef, }), [ @@ -139,6 +141,7 @@ const usePlayerLyrics = ({ lyricsError, lyricsLoading, lyricsVisible, + obscuredByQueue, showPronunciation, showTranslation, togglePronunciation, @@ -150,9 +153,12 @@ const usePlayerLyrics = ({ () => ( ({ frame: { @@ -20,25 +28,126 @@ const useStyles = makeStyles( const LyricsLayoutFrame = ({ children }) => { const { desktopLyricsProps } = useLyricsLayout() const hasVisibleLyrics = Boolean(desktopLyricsProps?.visible) - const [sidebarWidth, setSidebarWidth] = useState(loadSidebarWidth) + const [preferredSidebarWidth, setPreferredSidebarWidth] = + useState(loadSidebarWidth) + const [contentWidth, setContentWidth] = useState(null) + const frameRef = useRef(null) + const autoCollapsedRef = useRef(false) + const manualSidebarOverrideRef = useRef(false) + const pendingSidebarStateRef = useRef(null) + const previousSidebarOpenRef = useRef(null) + const dispatch = useDispatch() + const sidebarOpen = useSelector( + (state) => state.admin?.ui?.sidebarOpen ?? false, + ) const classes = useStyles() + const sidebarMaxWidth = getSidebarMaxWidth(contentWidth) + const sidebarWidth = clampSidebarWidth(preferredSidebarWidth, sidebarMaxWidth) useEffect(() => { - if (!hasVisibleLyrics) return undefined + const parent = frameRef.current?.parentElement + if (!parent) return undefined - const handleSidebarWidth = (event) => { - const next = clampSidebarWidth(event.detail?.width) - setSidebarWidth((current) => (current === next ? current : next)) + const updateContentWidth = (nextWidth) => { + const numeric = Number(nextWidth) + if (!Number.isFinite(numeric) || numeric <= 0) return + const rounded = Math.round(numeric) + setContentWidth((current) => (current === rounded ? current : rounded)) } - setSidebarWidth(loadSidebarWidth()) - window.addEventListener(LYRICS_SIDEBAR_WIDTH_EVENT, handleSidebarWidth) - return () => - window.removeEventListener(LYRICS_SIDEBAR_WIDTH_EVENT, handleSidebarWidth) + const measureParent = () => { + updateContentWidth(parent.clientWidth) + } + + measureParent() + const ResizeObserverConstructor = window.ResizeObserver + if (!ResizeObserverConstructor) { + window.addEventListener('resize', measureParent) + return () => window.removeEventListener('resize', measureParent) + } + + const resizeObserver = new ResizeObserverConstructor((entries) => { + const entry = entries.find((candidate) => candidate.target === parent) + const borderBoxSize = Array.isArray(entry?.borderBoxSize) + ? entry.borderBoxSize[0] + : entry?.borderBoxSize + updateContentWidth( + borderBoxSize?.inlineSize || + entry?.target?.clientWidth || + entry?.contentRect?.width, + ) + }) + resizeObserver.observe(parent) + return () => resizeObserver.disconnect() + }, []) + + useEffect(() => { + if (hasVisibleLyrics) { + setPreferredSidebarWidth(loadSidebarWidth()) + } }, [hasVisibleLyrics]) + useEffect(() => { + const previousSidebarOpen = previousSidebarOpenRef.current + previousSidebarOpenRef.current = sidebarOpen + if (previousSidebarOpen == null || previousSidebarOpen === sidebarOpen) { + return + } + + if (pendingSidebarStateRef.current === sidebarOpen) { + pendingSidebarStateRef.current = null + return + } + + if (hasVisibleLyrics) { + manualSidebarOverrideRef.current = true + autoCollapsedRef.current = false + } + }, [hasVisibleLyrics, sidebarOpen]) + + useEffect(() => { + if (!hasVisibleLyrics) { + if ( + autoCollapsedRef.current && + !manualSidebarOverrideRef.current && + !sidebarOpen + ) { + autoCollapsedRef.current = false + pendingSidebarStateRef.current = true + dispatch(setSidebarVisibility(true)) + } + manualSidebarOverrideRef.current = false + return + } + + if ( + manualSidebarOverrideRef.current || + !Number.isFinite(contentWidth) || + contentWidth <= 0 + ) { + return + } + + if (sidebarOpen && contentWidth < MIN_CONTENT_WITH_LYRICS) { + autoCollapsedRef.current = true + pendingSidebarStateRef.current = false + dispatch(setSidebarVisibility(false)) + return + } + }, [contentWidth, dispatch, hasVisibleLyrics, sidebarOpen]) + + const handleSidebarWidthChange = useCallback( + (nextWidth, { persist = false } = {}) => { + const resolvedWidth = clampSidebarWidth(nextWidth, sidebarMaxWidth) + setPreferredSidebarWidth(resolvedWidth) + if (persist) saveSidebarWidth(resolvedWidth) + }, + [sidebarMaxWidth], + ) + return (
{ }} > {children} - {desktopLyricsProps && } + {desktopLyricsProps && ( + + )}
) } diff --git a/ui/src/layout/LyricsLayoutFrame.test.jsx b/ui/src/layout/LyricsLayoutFrame.test.jsx index bb8ee0bc5..632acf6b6 100644 --- a/ui/src/layout/LyricsLayoutFrame.test.jsx +++ b/ui/src/layout/LyricsLayoutFrame.test.jsx @@ -1,12 +1,25 @@ import React, { useEffect } from 'react' -import { act, cleanup, render, screen } from '@testing-library/react' +import { + act, + cleanup, + fireEvent, + render, + screen, + waitFor, +} from '@testing-library/react' import { ThemeProvider, createTheme } from '@material-ui/core/styles' +import { setSidebarVisibility } from 'react-admin' +import { Provider } from 'react-redux' +import { createStore } from 'redux' import { afterEach, describe, expect, it, vi } from 'vitest' import { LyricsLayoutProvider, useLyricsLayout, } from '../audioplayer/LyricsLayoutContext' -import { LYRICS_SIDEBAR_TRANSITION_MS } from '../audioplayer/lyricsSidebarWidth' +import { + LYRICS_SIDEBAR_STORAGE_KEY, + LYRICS_SIDEBAR_TRANSITION_MS, +} from '../audioplayer/lyricsSidebarWidth' import LyricsLayoutFrame from './LyricsLayoutFrame' const theme = createTheme({ @@ -32,6 +45,61 @@ const lyricsProps = { onTogglePronunciation: vi.fn(), } +const originalResizeObserver = window.ResizeObserver +const resizeObservers = [] + +class TestResizeObserver { + constructor(callback) { + this.callback = callback + this.target = null + this.disconnected = false + resizeObservers.push(this) + } + + observe(target) { + this.target = target + } + + disconnect() { + this.disconnected = true + } +} + +const notifyContentWidth = (width) => { + act(() => { + resizeObservers.forEach((observer) => { + if (observer.disconnected || !observer.target) return + observer.callback([ + { + target: observer.target, + contentRect: { width }, + borderBoxSize: [{ inlineSize: width }], + }, + ]) + }) + }) +} + +const sidebarVisibilityActionType = setSidebarVisibility(true).type + +const createLayoutStore = (sidebarOpen = false) => + createStore( + (state, action) => { + if (action.type !== sidebarVisibilityActionType) return state + return { + ...state, + admin: { + ...state.admin, + ui: { + ...state.admin.ui, + sidebarOpen: action.payload, + }, + }, + } + }, + { admin: { ui: { sidebarOpen } } }, + ) + const PublishLyricsProps = ({ props }) => { const { setDesktopLyricsProps } = useLyricsLayout() @@ -43,22 +111,27 @@ const PublishLyricsProps = ({ props }) => { return null } -const frame = (props) => ( - - - - -
Albums grid
- - - +const frame = (props, store) => ( + + + + + +
Albums grid
+ + + + ) -const renderFrame = (props) => { - const view = render(frame(props)) +const renderFrame = (props, { sidebarOpen = false, store } = {}) => { + window.ResizeObserver = TestResizeObserver + const layoutStore = store || createLayoutStore(sidebarOpen) + const view = render(frame(props, layoutStore)) return { ...view, - rerenderFrame: (nextProps) => view.rerender(frame(nextProps)), + store: layoutStore, + rerenderFrame: (nextProps) => view.rerender(frame(nextProps, layoutStore)), } } @@ -66,6 +139,9 @@ describe('', () => { afterEach(() => { vi.useRealTimers() cleanup() + localStorage.clear() + resizeObservers.splice(0) + window.ResizeObserver = originalResizeObserver }) it('reserves a native right-hand lyrics pane beside route content', async () => { @@ -82,6 +158,186 @@ describe('', () => { expect(window.getComputedStyle(sidebar).position).toBe('fixed') }) + it('caps a persisted width against the measured route reserve without overwriting it', async () => { + localStorage.setItem(LYRICS_SIDEBAR_STORAGE_KEY, '520') + renderFrame(lyricsProps) + const sidebar = await screen.findByTestId('lyrics-sidebar') + const resizer = screen.getByTestId('lyrics-sidebar-resizer') + + notifyContentWidth(755) + + expect(screen.getByTestId('lyrics-layout-frame')).toHaveStyle({ + marginRight: '435px', + }) + expect(sidebar).toHaveStyle({ width: '435px' }) + expect(resizer).toHaveAttribute('aria-valuemax', '435') + expect(resizer).toHaveAttribute('aria-valuenow', '435') + expect(localStorage.getItem(LYRICS_SIDEBAR_STORAGE_KEY)).toBe('520') + + notifyContentWidth(840) + + expect(screen.getByTestId('lyrics-layout-frame')).toHaveStyle({ + marginRight: '520px', + }) + expect(sidebar).toHaveStyle({ width: '520px' }) + expect(resizer).toHaveAttribute('aria-valuemax', '520') + expect(resizer).toHaveAttribute('aria-valuenow', '520') + expect(localStorage.getItem(LYRICS_SIDEBAR_STORAGE_KEY)).toBe('520') + }) + + it('keeps the layout width reserved while the queue obscures lyrics', async () => { + localStorage.setItem(LYRICS_SIDEBAR_STORAGE_KEY, '520') + renderFrame({ ...lyricsProps, obscuredByQueue: true }) + const sidebar = await screen.findByTestId('lyrics-sidebar') + + notifyContentWidth(755) + + expect(screen.getByTestId('lyrics-layout-frame')).toHaveStyle({ + marginRight: '435px', + }) + expect(sidebar).toHaveStyle({ + width: '435px', + transform: 'translateX(0)', + pointerEvents: 'none', + }) + expect(sidebar).toHaveAttribute('aria-hidden', 'true') + expect(sidebar).toHaveAttribute('inert') + expect(screen.getByTestId('karaoke-lyrics-panel')).toBeInTheDocument() + }) + + it('collapses an open app menu only when the minimum pane cannot fit', async () => { + localStorage.setItem(LYRICS_SIDEBAR_STORAGE_KEY, '520') + const { rerenderFrame, store } = renderFrame(lyricsProps, { + sidebarOpen: true, + }) + await screen.findByTestId('lyrics-sidebar') + + notifyContentWidth(570) + + await waitFor(() => + expect(store.getState().admin.ui.sidebarOpen).toBe(false), + ) + expect(screen.getByTestId('lyrics-layout-frame')).toHaveStyle({ + marginRight: '300px', + }) + expect(screen.getByTestId('lyrics-sidebar-resizer')).toHaveAttribute( + 'aria-valuemax', + '300', + ) + + notifyContentWidth(755) + + expect(screen.getByTestId('lyrics-layout-frame')).toHaveStyle({ + marginRight: '435px', + }) + expect(store.getState().admin.ui.sidebarOpen).toBe(false) + + rerenderFrame({ ...lyricsProps, visible: false }) + + await waitFor(() => + expect(store.getState().admin.ui.sidebarOpen).toBe(true), + ) + }) + + it('keeps the app menu open at the 960px and 1280px layout caps', async () => { + localStorage.setItem(LYRICS_SIDEBAR_STORAGE_KEY, '520') + const { store } = renderFrame(lyricsProps, { sidebarOpen: true }) + await screen.findByTestId('lyrics-sidebar') + + notifyContentWidth(720) + + expect(store.getState().admin.ui.sidebarOpen).toBe(true) + expect(screen.getByTestId('lyrics-layout-frame')).toHaveStyle({ + marginRight: '400px', + }) + expect(screen.getByTestId('lyrics-sidebar-resizer')).toHaveAttribute( + 'aria-valuemax', + '400', + ) + + notifyContentWidth(1040) + + expect(screen.getByTestId('lyrics-layout-frame')).toHaveStyle({ + marginRight: '520px', + }) + expect(screen.getByTestId('lyrics-sidebar-resizer')).toHaveAttribute( + 'aria-valuemax', + '520', + ) + }) + + it('keeps an automatic collapse until lyrics close when space returns', async () => { + const { rerenderFrame, store } = renderFrame(lyricsProps, { + sidebarOpen: true, + }) + await screen.findByTestId('lyrics-sidebar') + notifyContentWidth(570) + await waitFor(() => + expect(store.getState().admin.ui.sidebarOpen).toBe(false), + ) + + notifyContentWidth(1040) + + expect(store.getState().admin.ui.sidebarOpen).toBe(false) + + rerenderFrame({ ...lyricsProps, visible: false }) + + await waitFor(() => + expect(store.getState().admin.ui.sidebarOpen).toBe(true), + ) + }) + + it('does not restore the menu after a manual override', async () => { + const { rerenderFrame, store } = renderFrame(lyricsProps, { + sidebarOpen: true, + }) + await screen.findByTestId('lyrics-sidebar') + notifyContentWidth(570) + await waitFor(() => + expect(store.getState().admin.ui.sidebarOpen).toBe(false), + ) + + act(() => { + store.dispatch(setSidebarVisibility(true)) + }) + await waitFor(() => + expect(store.getState().admin.ui.sidebarOpen).toBe(true), + ) + act(() => { + store.dispatch(setSidebarVisibility(false)) + }) + await waitFor(() => + expect(store.getState().admin.ui.sidebarOpen).toBe(false), + ) + + rerenderFrame({ ...lyricsProps, visible: false }) + + expect(store.getState().admin.ui.sidebarOpen).toBe(false) + }) + + it('persists deliberate keyboard resizing at the measured cap', async () => { + localStorage.setItem(LYRICS_SIDEBAR_STORAGE_KEY, '520') + renderFrame(lyricsProps) + await screen.findByTestId('lyrics-sidebar') + notifyContentWidth(755) + const resizer = screen.getByTestId('lyrics-sidebar-resizer') + + fireEvent.keyDown(resizer, { key: 'Home' }) + + expect(screen.getByTestId('lyrics-layout-frame')).toHaveStyle({ + marginRight: '300px', + }) + expect(localStorage.getItem(LYRICS_SIDEBAR_STORAGE_KEY)).toBe('300') + + fireEvent.keyDown(resizer, { key: 'End' }) + + expect(screen.getByTestId('lyrics-layout-frame')).toHaveStyle({ + marginRight: '435px', + }) + expect(resizer).toHaveAttribute('aria-valuenow', '435') + expect(localStorage.getItem(LYRICS_SIDEBAR_STORAGE_KEY)).toBe('435') + }) + it('releases the layout frame after the sidebar exit transition', async () => { vi.useFakeTimers() const { rerenderFrame } = renderFrame(lyricsProps)