From 57b0d418d7aa98648b512e580ce085514aafb0f2 Mon Sep 17 00:00:00 2001 From: ranokay Date: Wed, 29 Jul 2026 14:17:46 +0300 Subject: [PATCH] refactor(ui): simplify lyrics sidebar internals Signed-off-by: ranokay --- ui/src/audioplayer/LyricsLayerControls.jsx | 1 - ui/src/audioplayer/LyricsLineRows.jsx | 12 +---- ui/src/audioplayer/LyricsPanel.jsx | 7 +-- ui/src/audioplayer/LyricsPanel.test.jsx | 1 - ui/src/audioplayer/PlayerToolbar.test.jsx | 48 +++++++------------- ui/src/audioplayer/playerBreakpoints.js | 2 - ui/src/audioplayer/playerBreakpoints.test.js | 14 +----- ui/src/audioplayer/useEnhancedLyrics.js | 8 +--- ui/src/audioplayer/usePlayerLyrics.jsx | 11 +---- 9 files changed, 23 insertions(+), 81 deletions(-) diff --git a/ui/src/audioplayer/LyricsLayerControls.jsx b/ui/src/audioplayer/LyricsLayerControls.jsx index 56d5e27f0..62c43f37a 100644 --- a/ui/src/audioplayer/LyricsLayerControls.jsx +++ b/ui/src/audioplayer/LyricsLayerControls.jsx @@ -120,7 +120,6 @@ const LyricsLayerControls = ({ [classes.sidebarPlacement]: placement !== 'mobile', })} data-testid={testId} - data-placement={placement} onClick={(event) => event.stopPropagation()} > {segments.map((segment, idx) => { @@ -485,9 +482,6 @@ export const KaraokeStackedLineRow = memo( data-wrapped={isWrapped ? 'true' : 'false'} data-testid={testId} data-tokenized={hasTimedTokens ? 'true' : 'false'} - data-layer-animation={ - hasTimedTokens ? 'token-gradient' : 'shared-opacity' - } ref={rowRef} style={lineStyle} > @@ -537,11 +531,7 @@ export const KaraokeStackedLineRow = memo( )} ) : ( - + {segment.text} ) diff --git a/ui/src/audioplayer/LyricsPanel.jsx b/ui/src/audioplayer/LyricsPanel.jsx index d8f04dd1d..09f00402a 100644 --- a/ui/src/audioplayer/LyricsPanel.jsx +++ b/ui/src/audioplayer/LyricsPanel.jsx @@ -100,7 +100,6 @@ const useStyles = makeStyles((theme) => ({ alignItems: 'stretch', gap: theme.spacing(3), }, - inlineLines: {}, lineGroup: { width: '100%', borderRadius: theme.shape.borderRadius, @@ -692,7 +691,6 @@ const LyricsPanel = ({
event.stopPropagation() : undefined} >
event.stopPropagation() : undefined} >
0 || scrollStartPadding > 0 diff --git a/ui/src/audioplayer/LyricsPanel.test.jsx b/ui/src/audioplayer/LyricsPanel.test.jsx index 49705ad13..d37e31d95 100644 --- a/ui/src/audioplayer/LyricsPanel.test.jsx +++ b/ui/src/audioplayer/LyricsPanel.test.jsx @@ -380,7 +380,6 @@ describe('', () => { const pronunciation = screen.getAllByTestId('lyrics-pronunciation-token') ;[mainRow, translationRow].forEach((row) => { - expect(row).toHaveAttribute('data-layer-animation', 'shared-opacity') expect(row).toHaveAttribute('data-tokenized', 'false') expect(row.style.opacity).toBe('') }) diff --git a/ui/src/audioplayer/PlayerToolbar.test.jsx b/ui/src/audioplayer/PlayerToolbar.test.jsx index d4779c771..ef03b1cad 100644 --- a/ui/src/audioplayer/PlayerToolbar.test.jsx +++ b/ui/src/audioplayer/PlayerToolbar.test.jsx @@ -77,22 +77,6 @@ describe('', () => { expect(listItems[0].className).toContain('toolbar') }) - it('disables save queue button when isRadio is true', () => { - render() - - const saveQueueButton = screen.getByTestId('save-queue-button') - expect(saveQueueButton).toBeDisabled() - }) - - it('disables love button when conditions are met', () => { - useGetOne.mockReturnValue({ data: mockSongData, loading: true }) - - render() - - const loveButton = screen.getByTestId('love-button') - expect(loveButton).toBeDisabled() - }) - it('opens save queue dialog when save button is clicked', () => { render() @@ -154,22 +138,6 @@ describe('', () => { expect(listItems[2].className).toContain('mobileListItem') }) - it('disables save queue button when isRadio is true', () => { - render() - - const saveQueueButton = screen.getByTestId('save-queue-button') - expect(saveQueueButton).toBeDisabled() - }) - - it('disables love button when conditions are met', () => { - useGetOne.mockReturnValue({ data: mockSongData, loading: true }) - - render() - - const loveButton = screen.getByTestId('love-button') - expect(loveButton).toBeDisabled() - }) - it('disables lyrics toggle when lyrics are unavailable', () => { render( , @@ -192,6 +160,22 @@ describe('', () => { }) describe('Common behavior', () => { + it('disables save queue button when isRadio is true', () => { + useMediaQuery.mockReturnValue(true) + render() + + expect(screen.getByTestId('save-queue-button')).toBeDisabled() + }) + + it('disables love button when conditions are met', () => { + useMediaQuery.mockReturnValue(true) + useGetOne.mockReturnValue({ data: mockSongData, loading: true }) + + render() + + expect(screen.getByTestId('love-button')).toBeDisabled() + }) + it('renders global hotkeys in both layouts', () => { // Test desktop layout useMediaQuery.mockReturnValue(true) diff --git a/ui/src/audioplayer/playerBreakpoints.js b/ui/src/audioplayer/playerBreakpoints.js index 72dd2e68a..a646b76fa 100644 --- a/ui/src/audioplayer/playerBreakpoints.js +++ b/ui/src/audioplayer/playerBreakpoints.js @@ -2,9 +2,7 @@ export const PLAYER_DESKTOP_MIN_WIDTH = 810 export const PLAYER_MEDIA_QUERY_STEP = 0.05 export const PLAYER_MOBILE_MAX_WIDTH = PLAYER_DESKTOP_MIN_WIDTH - PLAYER_MEDIA_QUERY_STEP -export const LYRICS_SIDEBAR_DESKTOP_MIN_WIDTH = PLAYER_DESKTOP_MIN_WIDTH export const PLAYER_DESKTOP_MEDIA_QUERY = `(min-width:${PLAYER_DESKTOP_MIN_WIDTH}px)` export const PLAYER_MOBILE_MATCH_MEDIA_QUERY = `(max-width:${PLAYER_MOBILE_MAX_WIDTH}px)` export const PLAYER_MOBILE_MEDIA_QUERY = `@media screen and ${PLAYER_MOBILE_MATCH_MEDIA_QUERY}` -export const LYRICS_SIDEBAR_DESKTOP_MEDIA_QUERY = PLAYER_DESKTOP_MEDIA_QUERY diff --git a/ui/src/audioplayer/playerBreakpoints.test.js b/ui/src/audioplayer/playerBreakpoints.test.js index 4d19e5e68..4d40b56c8 100644 --- a/ui/src/audioplayer/playerBreakpoints.test.js +++ b/ui/src/audioplayer/playerBreakpoints.test.js @@ -1,7 +1,5 @@ import { describe, expect, it } from 'vitest' import { - LYRICS_SIDEBAR_DESKTOP_MEDIA_QUERY, - LYRICS_SIDEBAR_DESKTOP_MIN_WIDTH, PLAYER_DESKTOP_MEDIA_QUERY, PLAYER_DESKTOP_MIN_WIDTH, PLAYER_MOBILE_MATCH_MEDIA_QUERY, @@ -12,6 +10,8 @@ import { describe('player breakpoints', () => { it('keeps compact and desktop player ranges from overlapping', () => { + expect(PLAYER_DESKTOP_MIN_WIDTH).toBe(810) + expect(PLAYER_MOBILE_MAX_WIDTH).toBe(809.95) expect(PLAYER_DESKTOP_MIN_WIDTH - PLAYER_MOBILE_MAX_WIDTH).toBeCloseTo( PLAYER_MEDIA_QUERY_STEP, ) @@ -25,14 +25,4 @@ describe('player breakpoints', () => { `(max-width:${PLAYER_MOBILE_MAX_WIDTH}px)`, ) }) - - it('keeps tablet layouts on the desktop player and lyrics sidebar', () => { - expect(PLAYER_DESKTOP_MIN_WIDTH).toBe(810) - expect(PLAYER_MOBILE_MAX_WIDTH).toBe(809.95) - expect(LYRICS_SIDEBAR_DESKTOP_MIN_WIDTH).toBe(810) - expect(LYRICS_SIDEBAR_DESKTOP_MEDIA_QUERY).toBe( - `(min-width:${LYRICS_SIDEBAR_DESKTOP_MIN_WIDTH}px)`, - ) - expect(LYRICS_SIDEBAR_DESKTOP_MIN_WIDTH).toBe(PLAYER_DESKTOP_MIN_WIDTH) - }) }) diff --git a/ui/src/audioplayer/useEnhancedLyrics.js b/ui/src/audioplayer/useEnhancedLyrics.js index 69343c4ac..735922583 100644 --- a/ui/src/audioplayer/useEnhancedLyrics.js +++ b/ui/src/audioplayer/useEnhancedLyrics.js @@ -8,7 +8,6 @@ export const emptyLyricLayers = Object.freeze({ pronunciation: null, }) -const LYRICS_CACHE_SCHEMA_VERSION = 3 const MAX_LYRIC_CACHE_ENTRIES = 75 const NEGATIVE_CACHE_TTL_MS = 30_000 @@ -25,12 +24,7 @@ const readStructuredLyrics = (response) => response?.json?.['subsonic-response']?.lyricsList?.structuredLyrics || [] const buildCacheKey = ({ trackId, preferredLanguage, updatedAt }) => - [ - trackId || '', - updatedAt || '', - preferredLanguage || '', - LYRICS_CACHE_SCHEMA_VERSION, - ].join('\u0000') + [trackId || '', updatedAt || '', preferredLanguage || ''].join('\u0000') const rememberLyrics = (cacheKey, layers, expiresAt = null) => { cache.delete(cacheKey) diff --git a/ui/src/audioplayer/usePlayerLyrics.jsx b/ui/src/audioplayer/usePlayerLyrics.jsx index e80f4cbfd..edbb6ae3d 100644 --- a/ui/src/audioplayer/usePlayerLyrics.jsx +++ b/ui/src/audioplayer/usePlayerLyrics.jsx @@ -1,4 +1,4 @@ -import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import React, { useCallback, useMemo, useRef, useState } from 'react' import LyricsLayerControls from './LyricsLayerControls' import LyricsPanel from './LyricsPanel' import MobileKaraokeLyricsPortal from './MobileKaraokeLyricsPortal' @@ -19,15 +19,10 @@ const usePlayerLyrics = ({ translate, }) => { const [lyricsVisiblePreference, setLyricsVisiblePreference] = useState(false) - const [lyricsRequested, setLyricsRequested] = useState(false) const [translationPreference, setTranslationPreference] = useState(null) const [pronunciationPreference, setPronunciationPreference] = useState(null) const lyricsToggleRef = useRef(null) - useEffect(() => { - setLyricsRequested(lyricsVisiblePreference) - }, [trackId]) // eslint-disable-line react-hooks/exhaustive-deps - const { layers: lyricLayers, loading: lyricsLoading, @@ -37,7 +32,7 @@ const usePlayerLyrics = ({ trackId, updatedAt: trackUpdatedAt, disabled: isRadio, - requested: lyricsRequested || lyricsVisiblePreference, + requested: lyricsVisiblePreference, }) const hasTranslationLyric = hasStructuredLyricContent(lyricLayers.translation) @@ -74,7 +69,6 @@ const usePlayerLyrics = ({ const toggleLyrics = useCallback(() => { const next = !lyricsVisiblePreference setLyricsVisiblePreference(next) - setLyricsRequested(next) if (next) { if (lyricsError) retryLyrics() } @@ -82,7 +76,6 @@ const usePlayerLyrics = ({ const closeLyrics = useCallback(() => { setLyricsVisiblePreference(false) - setLyricsRequested(false) }, []) const toggleTranslation = useCallback(() => {