Bläddra i källkod

feat: artist and song list paging

    * fix: correct item count in artists view
    * feat: added album toggle and paging to help commands
    * feat: alternative colour for selected non-parent-active highlight items
    * fix: filter artists by unknown album
    * fix: hide overflowing song titles
    * feat: page up / down handler
    * fix: pass rendered fallback instead of function
Fela Maslen 5 år sedan
förälder
incheckning
a3777b753b

+ 1 - 1
gmus/src/components/app.tsx

@@ -47,7 +47,7 @@ export const App: React.FC<Props> = ({ socket, state, dispatch }) => {
       )}
       <StateContext.Provider value={state}>
         <DispatchContext.Provider value={dispatch}>
-          <Suspense fallback={LoadingWrapper}>
+          <Suspense fallback={<LoadingWrapper />}>
             <UI isMaster={isMaster(state)} currentSong={currentSong} />
           </Suspense>
         </DispatchContext.Provider>

+ 140 - 0
gmus/src/components/ui/cmus/reducer.spec.ts

@@ -384,6 +384,146 @@ describe(cmusUIReducer.name, () => {
       });
     });
 
+    describe(Keys.pageDown, () => {
+      const action: ActionKeyPressed = { type: ActionTypeKeyPressed, key: Keys.pageDown };
+
+      describe('when in library view', () => {
+        describe('when in the artist list mode', () => {
+          const stateArtistMode: CmusUIState = {
+            ...stateLibrary,
+            artists: Array(26)
+              .fill(0)
+              .map((_, index) => `Artist ${index + 1}`),
+            artistAlbums: {
+              'Artist 3': ['Album 1', 'Album 2'],
+              'Artist 4': ['Album Z'],
+              'Artist 18': ['Album 3'],
+            },
+            artistSongs: {
+              'Artist 18': [{ id: 123, album: 'Album 3' } as Song],
+            },
+            library: {
+              ...stateLibrary.library,
+              activeArtist: 'Artist 1',
+              activeAlbum: null,
+              expandedArtists: ['Artist 3', 'Artist 18'],
+              modeWindow: LibraryModeWindow.ArtistList,
+            },
+          };
+
+          it('should page the active artist and album by 20 rows down', () => {
+            expect.assertions(2);
+            const result = cmusUIReducer(stateArtistMode, action);
+
+            expect(result.library.activeArtist).toBe('Artist 18');
+            expect(result.library.activeAlbum).toBe('Album 3');
+          });
+
+          it('should set the active song ID to the first by the artist', () => {
+            expect.assertions(1);
+            const result = cmusUIReducer(stateArtistMode, action);
+
+            expect(result.library.activeSongId).toBe(123);
+          });
+        });
+
+        describe('when in the song list mode', () => {
+          const stateSongsMode: CmusUIState = {
+            ...stateLibrary,
+            artists: ['Artist A'],
+            artistSongs: {
+              'Artist A': Array(30)
+                .fill(0)
+                .map((_, index) => ({ id: index + 100 } as Song)),
+            },
+            library: {
+              ...stateLibrary.library,
+              activeArtist: 'Artist A',
+              activeSongId: 101,
+              modeWindow: LibraryModeWindow.SongList,
+            },
+          };
+
+          it('should set the active song ID to the one 20th after current', () => {
+            expect.assertions(1);
+            const result = cmusUIReducer(stateSongsMode, action);
+
+            expect(result.library.activeSongId).toBe(121);
+          });
+        });
+      });
+    });
+
+    describe(Keys.pageUp, () => {
+      const action: ActionKeyPressed = { type: ActionTypeKeyPressed, key: Keys.pageUp };
+
+      describe('when in library view', () => {
+        describe('when in the artist list mode', () => {
+          const stateArtistMode: CmusUIState = {
+            ...stateLibrary,
+            artists: Array(26)
+              .fill(0)
+              .map((_, index) => `Artist ${index + 1}`),
+            artistAlbums: {
+              'Artist 3': ['Album 1', 'Album 2'],
+              'Artist 4': ['Album X', 'Album Y', 'Album Z'],
+              'Artist 18': ['Album 3'],
+            },
+            artistSongs: {
+              'Artist 3': [{ id: 123, album: 'Album 1' } as Song],
+            },
+            library: {
+              ...stateLibrary.library,
+              activeArtist: 'Artist 18',
+              activeAlbum: 'Album 3',
+              expandedArtists: ['Artist 3', 'Artist 4', 'Artist 18'],
+              modeWindow: LibraryModeWindow.ArtistList,
+            },
+          };
+
+          it('should page the active artist and album by 20 rows down', () => {
+            expect.assertions(2);
+            const result = cmusUIReducer(stateArtistMode, action);
+
+            expect(result.library.activeArtist).toBe('Artist 3');
+            expect(result.library.activeAlbum).toBe('Album 1');
+          });
+
+          it('should set the active song ID to the first by the artist', () => {
+            expect.assertions(1);
+            const result = cmusUIReducer(stateArtistMode, action);
+
+            expect(result.library.activeSongId).toBe(123);
+          });
+        });
+
+        describe('when in the song list mode', () => {
+          const stateSongsMode: CmusUIState = {
+            ...stateLibrary,
+            artists: ['Artist A'],
+            artistSongs: {
+              'Artist A': Array(30)
+                .fill(0)
+                .map((_, index) => ({ id: index + 100 } as Song)),
+            },
+            library: {
+              ...stateLibrary.library,
+              activeArtist: 'Artist A',
+              activeSongId: 128,
+              modeWindow: LibraryModeWindow.SongList,
+            },
+          };
+
+          it('should set the active song ID to the one 20th prior to current', () => {
+            expect.assertions(1);
+            const result = cmusUIReducer(stateSongsMode, action);
+
+            expect(result.library.activeSongId).toBe(108);
+          });
+        });
+      });
+    });
+
     describe(Keys.space, () => {
       const action: ActionKeyPressed = { type: ActionTypeKeyPressed, key: Keys.space };
 

+ 11 - 19
gmus/src/components/ui/cmus/reducer.ts

@@ -71,7 +71,7 @@ function getActiveSongIdFromActiveArtistAlbum(
   return songs.find((compare) => compare.album === activeAlbum)?.id ?? null;
 }
 
-const scrollArtists = (state: CmusUIState, delta: 1 | -1): CmusUIState => {
+const scrollArtists = (state: CmusUIState, delta: number): CmusUIState => {
   const { artist, album } = getNextActiveArtistAndAlbum(
     state.artists,
     state.artistAlbums,
@@ -123,26 +123,13 @@ function toggleExpandArtist(library: CmusUIState['library']): CmusUIState['libra
   return { ...library, expandedArtists: [...library.expandedArtists, library.activeArtist] };
 }
 
-function handleScrollDown(state: CmusUIState): CmusUIState {
+function handleScroll(state: CmusUIState, delta: number): CmusUIState {
   if (state.view === View.Library) {
     if (state.library.modeWindow === LibraryModeWindow.ArtistList) {
-      return scrollArtists(state, 1);
+      return scrollArtists(state, delta);
     }
     if (state.library.modeWindow === LibraryModeWindow.SongList) {
-      return scrollSongs(state, 1);
-    }
-  }
-
-  return state;
-}
-
-function handleScrollUp(state: CmusUIState): CmusUIState {
-  if (state.view === View.Library) {
-    if (state.library.modeWindow === LibraryModeWindow.ArtistList) {
-      return scrollArtists(state, -1);
-    }
-    if (state.library.modeWindow === LibraryModeWindow.SongList) {
-      return scrollSongs(state, -1);
+      return scrollSongs(state, delta);
     }
   }
 
@@ -201,9 +188,14 @@ function handleKeyPress(state: CmusUIState, key: string): CmusUIState {
       return withGlobalAction(state, playPaused());
 
     case Keys.J:
-      return handleScrollDown(state);
+      return handleScroll(state, 1);
     case Keys.K:
-      return handleScrollUp(state);
+      return handleScroll(state, -1);
+
+    case Keys.pageDown:
+      return handleScroll(state, 20);
+    case Keys.pageUp:
+      return handleScroll(state, -20);
 
     default:
       return state;

+ 3 - 0
gmus/src/components/ui/cmus/styled/layout.ts

@@ -38,6 +38,9 @@ export const ActiveHighlightRow = styled(FlexRow)<{
 
   color: ${({ active, highlight, parentActive }): string => {
     if (highlight) {
+      if (active && !parentActive) {
+        return colors.active.parentInactive;
+      }
       return colors.active.color;
     }
     if (active && !parentActive) {

+ 1 - 0
gmus/src/components/ui/cmus/styled/variables.ts

@@ -10,5 +10,6 @@ export const colors = {
   },
   active: {
     color: rgb(255, 255, 130),
+    parentInactive: rgb(102, 0, 165),
   },
 };

+ 38 - 0
gmus/src/components/ui/cmus/utils/scroll.spec.ts

@@ -223,6 +223,44 @@ describe(getNextActiveArtistAndAlbum.name, () => {
       });
     });
   });
+
+  describe('paging', () => {
+    describe.each`
+      expandedArtists | from           | delta | to
+      ${[]}           | ${['A', null]} | ${2}  | ${['C', null]}
+      ${[]}           | ${['B', null]} | ${2}  | ${['C', null]}
+      ${[]}           | ${['C', null]} | ${2}  | ${['C', null]}
+      ${['A']}        | ${['A', null]} | ${2}  | ${['A', 'a2']}
+      ${['A']}        | ${['A', 'a1']} | ${4}  | ${['C', null]}
+      ${['A']}        | ${['A', 'a1']} | ${5}  | ${['C', null]}
+      ${['C']}        | ${['A', null]} | ${3}  | ${['C', 'c1']}
+      ${['A', 'C']}   | ${['A', 'a1']} | ${5}  | ${['C', 'c1']}
+      ${['A', 'C']}   | ${['A', 'a1']} | ${6}  | ${['C', 'c2']}
+      ${['A', 'C']}   | ${['A', 'a1']} | ${11} | ${['C', 'c2']}
+      ${['A']}        | ${['C', null]} | ${-2} | ${['A', 'a3']}
+      ${['A', 'C']}   | ${['C', 'c2']} | ${-2} | ${['C', null]}
+      ${['A', 'C']}   | ${['C', 'c2']} | ${-3} | ${['B', null]}
+      ${['A', 'C']}   | ${['C', 'c2']} | ${-4} | ${['A', 'a3']}
+      ${['A', 'C']}   | ${['C', 'c2']} | ${-5} | ${['A', 'a2']}
+    `(
+      'when expandedArtists=$expandedArtists, delta=$delta',
+      ({ expandedArtists, from, delta, to }) => {
+        it(`should page from ${from.join(',')} to ${to.join(',')}`, () => {
+          expect.assertions(1);
+          expect(
+            getNextActiveArtistAndAlbum(
+              artists,
+              artistAlbums,
+              from[0],
+              from[1],
+              expandedArtists,
+              delta,
+            ),
+          ).toStrictEqual({ artist: to[0], album: to[1] });
+        });
+      },
+    );
+  });
 });
 
 describe(getArtistAlbumScrollIndex.name, () => {

+ 70 - 30
gmus/src/components/ui/cmus/utils/scroll.ts

@@ -1,6 +1,5 @@
 import { RefObject, useEffect } from 'react';
 import { Song } from '../../../../types';
-import { scrollThroughItems } from '../../../../utils/delta';
 
 const getArtistAlbums = (
   artist: string | null,
@@ -14,9 +13,11 @@ export function getNextActiveArtistAndAlbum(
   activeArtist: string | null,
   activeAlbum: string | null,
   expandedArtists: string[],
-  delta: -1 | 1,
+  delta: number,
 ): { artist: string | null; album: string | null } {
-  if (activeArtist === null) {
+  const activeArtistIndex = activeArtist === null ? -1 : artists.indexOf(activeArtist);
+
+  if (activeArtistIndex === -1) {
     if (delta === 1) {
       return { artist: artists[0] ?? null, album: null };
     }
@@ -32,39 +33,78 @@ export function getNextActiveArtistAndAlbum(
     return { artist: lastArtist, album: lastAlbum };
   }
 
-  const nextArtist = scrollThroughItems(artists, (compare) => compare === activeArtist, delta);
-  const atEnd = nextArtist === activeArtist;
-
   const activeArtistAlbums = getArtistAlbums(activeArtist, artistAlbums, expandedArtists);
-  const nextArtistAlbums = getArtistAlbums(nextArtist, artistAlbums, expandedArtists);
-
-  if (activeAlbum === null) {
-    if (
-      (delta === 1 && !activeArtistAlbums.length) ||
-      (delta === -1 && (atEnd || !nextArtistAlbums.length))
-    ) {
-      return { artist: nextArtist, album: null };
-    }
-    if (delta === 1) {
-      return { artist: activeArtist, album: activeArtistAlbums[0] };
-    }
-    return { artist: nextArtist, album: nextArtistAlbums[nextArtistAlbums.length - 1] };
-  }
+  const activeAlbumIndex = activeAlbum === null ? -1 : activeArtistAlbums.indexOf(activeAlbum);
 
-  const nextAlbum = scrollThroughItems(
-    activeArtistAlbums,
-    (compare) => compare === activeAlbum,
-    delta,
-  );
+  const reverse = delta < 0;
+  const requiredDelta = Math.abs(delta);
 
-  if (delta === -1 && nextAlbum === activeAlbum) {
-    return { artist: activeArtist, album: null };
+  const activeArtistAlbumsSlice =
+    delta > 0
+      ? activeArtistAlbums.slice(activeAlbumIndex + 1)
+      : activeArtistAlbums.slice(0, Math.max(0, activeAlbumIndex)).reverse();
+
+  if (activeArtistAlbumsSlice.length >= requiredDelta) {
+    const album = activeArtistAlbumsSlice[requiredDelta - 1];
+    return { artist: activeArtist, album };
   }
-  if (delta === 1 && nextArtist !== activeArtist && nextAlbum === activeAlbum) {
-    return { artist: nextArtist, album: null };
+  if (reverse && activeAlbum !== null && activeArtistAlbumsSlice.length === requiredDelta - 1) {
+    return { artist: activeArtist, album: null };
   }
 
-  return { artist: activeArtist, album: nextAlbum };
+  const nextArtistsSlice = reverse
+    ? artists.slice(0, activeArtistIndex).reverse()
+    : artists.slice(activeArtistIndex + 1);
+
+  const initialDelta =
+    reverse && activeAlbum !== null
+      ? activeArtistAlbumsSlice.length + 1
+      : activeArtistAlbumsSlice.length;
+
+  const reduction = nextArtistsSlice.reduce<{
+    delta: number;
+    artist: string | null;
+    album: string | null;
+  }>(
+    (last, artist, index) => {
+      const remainingDelta = requiredDelta - last.delta;
+      if (remainingDelta < 1) {
+        return last;
+      }
+      const thisArtistAlbums = getArtistAlbums(artist, artistAlbums, expandedArtists);
+      if (reverse) {
+        if (thisArtistAlbums.length >= remainingDelta) {
+          return {
+            artist,
+            album: thisArtistAlbums[thisArtistAlbums.length - remainingDelta],
+            delta: requiredDelta,
+          };
+        }
+        if (
+          thisArtistAlbums.length === remainingDelta - 1 ||
+          index === nextArtistsSlice.length - 1
+        ) {
+          return { artist, album: null, delta: requiredDelta };
+        }
+        return { ...last, delta: last.delta + thisArtistAlbums.length + 1 };
+      }
+      if (remainingDelta === 1) {
+        return { artist, album: null, delta: requiredDelta };
+      }
+      if (thisArtistAlbums.length >= remainingDelta - 1 || index === nextArtistsSlice.length - 1) {
+        return {
+          artist,
+          album:
+            thisArtistAlbums[Math.min(thisArtistAlbums.length - 1, remainingDelta - 2)] ?? null,
+          delta: requiredDelta,
+        };
+      }
+      return { ...last, delta: last.delta + thisArtistAlbums.length + 1 };
+    },
+    { delta: initialDelta, artist: activeArtist, album: activeAlbum },
+  );
+
+  return { artist: reduction.artist, album: reduction.album };
 }
 
 export function getArtistAlbumScrollIndex(

+ 1 - 1
gmus/src/components/ui/cmus/views/artists.tsx

@@ -163,7 +163,7 @@ export const Artists: React.FC<Props> = ({ active: parentActive, currentArtist }
             outerRef={windowRef}
             height={height}
             width={width}
-            itemCount={artists.length}
+            itemCount={itemData.length}
             itemSize={lineHeight}
             itemKey={itemKey}
             itemData={itemData}

+ 3 - 0
gmus/src/components/ui/cmus/views/help.tsx

@@ -9,12 +9,15 @@ const commandsGeneral: Command[] = [
   { command: 'c', description: 'play / pause' },
   { command: 'j', description: 'select next list item' },
   { command: 'k', description: 'select previous list item' },
+  { command: '<PageDown>', description: 'select next page of list items' },
+  { command: '<PageUp>', description: 'select pervious page of list items' },
   { command: ':q', description: 'log out' },
   { command: '<Esc>', description: 'close this dialog' },
 ];
 
 const commandsLibrary: Command[] = [
   { command: '<Tab>', description: 'switch between artists and albums' },
+  { command: '<Space>', description: 'toggle albums for selected artist' },
 ];
 
 type CommandGroup = {

+ 7 - 4
gmus/src/components/ui/cmus/views/songs.tsx

@@ -6,6 +6,7 @@ import { StateContext } from '../../../../context/state';
 import { Song } from '../../../../types';
 import { namedMemo } from '../../../../utils/component';
 import { CmusUIStateContext } from '../reducer';
+import { NoWrapFill } from '../styled/layout';
 import { AsciiSpinner } from '../styled/spinner';
 import { getSongScrollIndex, lineHeight, useAutoJumpyScroll } from '../utils/scroll';
 
@@ -32,7 +33,9 @@ const Row = namedMemo<{ index: number; data: SongData[]; style: CSSProperties }>
     const { song, active, parentActive, highlight } = data[index];
     return (
       <Styled.Song active={active} parentActive={parentActive} style={style} highlight={highlight}>
-        {song.track} - {song.title || 'Untitled Track'}
+        <NoWrapFill>
+          {song.track} - {song.title || 'Untitled Track'}
+        </NoWrapFill>
       </Styled.Song>
     );
   },
@@ -52,9 +55,9 @@ export const Songs: React.FC<Props> = ({ active: parentActive }) => {
 
   const filteredSongs = useMemo<Song[]>(
     () =>
-      activeAlbum
-        ? activeArtistSongs.filter(({ album }) => album === activeAlbum)
-        : activeArtistSongs,
+      activeAlbum === null
+        ? activeArtistSongs
+        : activeArtistSongs.filter(({ album }) => album === activeAlbum),
     [activeAlbum, activeArtistSongs],
   );
 

+ 2 - 0
gmus/src/hooks/vim.ts

@@ -8,6 +8,8 @@ export const Keys = {
   space: ' ',
   colon: ':',
   question: '?',
+  pageDown: 'PageDown',
+  pageUp: 'PageUp',
   '1': '1',
   C: 'c',
   J: 'j',