mirror of
https://github.com/Psychotoxical/psysonic.git
synced 2026-07-21 14:55:43 +00:00
fix(artists): per-artist links on song rails and shared OpenSubsonic refs (#1023)
* fix(artists): per-artist links on song rails and shared OpenSubsonic refs Song cards in Random Picks and Discover Songs showed joined artist credits but navigated to a single artistId. Route track surfaces through resolveTrackArtistRefs and coerce single-object Subsonic JSON payloads. * docs(changelog): note song-rail multi-artist link fix (PR #1023)
This commit is contained in:
@@ -144,6 +144,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
|
||||
|
||||
### Song rails — multi-artist credits link to each artist
|
||||
|
||||
**By [@cucadmuh](https://github.com/cucadmuh), reported by zunoz on Discord, PR [#1023](https://github.com/Psychotoxical/psysonic/pull/1023)**
|
||||
|
||||
* **Random Picks**, **Discover Songs**, and other song cards now split OpenSubsonic `artists[]` into individually clickable names — the same behaviour as album track rows and the player bar, instead of one link for the whole joined credit string.
|
||||
* Album cards and the rest of the app share the same artist-ref helper, including when Subsonic returns a single ref object instead of a one-element array.
|
||||
|
||||
|
||||
|
||||
## [1.47.0]
|
||||
|
||||
> **🙏 Thank you to our amazing Discord community.** This release would not have been possible without your tireless support, quality checks, bug reports and all-round collaboration. Every report, every repro and every bit of feedback shaped what shipped here — thank you. Come join us: [discord.gg/AMnDRErm4u](https://discord.gg/AMnDRErm4u)
|
||||
|
||||
@@ -28,6 +28,8 @@ import PlaybackScheduleBadge from './PlaybackScheduleBadge';
|
||||
import { usePlaybackScheduleRemaining } from '../utils/format/playbackScheduleFormat';
|
||||
import { usePreviewStore } from '../store/previewStore';
|
||||
import { usePerfProbeFlags } from '../utils/perf/perfFlags';
|
||||
import { coerceOpenArtistRefs } from '../utils/openArtistRefs';
|
||||
import { resolveTrackArtistRefs } from '../utils/playback/trackArtistRefs';
|
||||
import { formatTrackTime } from '../utils/format/formatDuration';
|
||||
import { PlayerTrackInfo } from './playerBar/PlayerTrackInfo';
|
||||
import { PlayerTransportControls } from './playerBar/PlayerTransportControls';
|
||||
@@ -146,8 +148,8 @@ export default function PlayerBar() {
|
||||
const showPreviewMeta = isPreviewing && !isRadio && previewingTrack !== null;
|
||||
const displayTitle = showPreviewMeta ? previewingTrack!.title : (currentTrack?.title ?? t('player.noTitle'));
|
||||
const displayArtist = showPreviewMeta ? previewingTrack!.artist : (currentTrack?.artist ?? '—');
|
||||
const displayArtistRefs = !showPreviewMeta && currentTrack?.artists && currentTrack.artists.length > 0
|
||||
? currentTrack.artists
|
||||
const displayArtistRefs = !showPreviewMeta && currentTrack && coerceOpenArtistRefs(currentTrack.artists).length > 0
|
||||
? resolveTrackArtistRefs(currentTrack)
|
||||
: undefined;
|
||||
|
||||
const coverArtId = showPreviewMeta
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
import { screen } from '@testing-library/react';
|
||||
import userEvent from '@testing-library/user-event';
|
||||
import { renderWithProviders } from '../test/helpers/renderWithProviders';
|
||||
import SongCard from './SongCard';
|
||||
import type { SubsonicSong } from '../api/subsonicTypes';
|
||||
|
||||
const navigateToArtist = vi.fn();
|
||||
|
||||
vi.mock('../hooks/useNavigateToArtist', () => ({
|
||||
useNavigateToArtist: () => navigateToArtist,
|
||||
}));
|
||||
|
||||
vi.mock('../cover/useLibraryCoverRef', () => ({
|
||||
useTrackCoverRef: () => undefined,
|
||||
}));
|
||||
|
||||
function song(overrides: Partial<SubsonicSong>): SubsonicSong {
|
||||
return {
|
||||
id: 's1', title: 'Track', artist: 'A', album: 'Alb', albumId: 'al1', duration: 100,
|
||||
...overrides,
|
||||
} as SubsonicSong;
|
||||
}
|
||||
|
||||
describe('SongCard', () => {
|
||||
it('splits OpenSubsonic artists into individual links', async () => {
|
||||
navigateToArtist.mockClear();
|
||||
const user = userEvent.setup();
|
||||
renderWithProviders(
|
||||
<SongCard
|
||||
disableArtwork
|
||||
song={song({
|
||||
artist: 'Apocalyptica', artistId: 'a1',
|
||||
artists: [{ id: 'a1', name: 'Apocalyptica' }, { id: 'a2', name: 'Joe Duplantier' }],
|
||||
})}
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByText('Apocalyptica')).toHaveClass('track-artist-link');
|
||||
expect(screen.getByText('Joe Duplantier')).toHaveClass('track-artist-link');
|
||||
await user.click(screen.getByText('Joe Duplantier'));
|
||||
expect(navigateToArtist).toHaveBeenCalledWith('a2');
|
||||
});
|
||||
});
|
||||
+16
-15
@@ -1,7 +1,6 @@
|
||||
import type { SubsonicSong } from '../api/subsonicTypes';
|
||||
import { songToTrack } from '../utils/playback/songToTrack';
|
||||
import React, { memo } from 'react';
|
||||
import { useNavigate } from 'react-router-dom';
|
||||
import React, { memo, useMemo } from 'react';
|
||||
import { Play, ListPlus, Star, Disc3 } from 'lucide-react';
|
||||
import { useTranslation } from 'react-i18next';
|
||||
import { usePlayerStore } from '../store/playerStore';
|
||||
@@ -13,6 +12,9 @@ import { enqueueAndPlay } from '../utils/playback/playSong';
|
||||
import { useDragDrop } from '../contexts/DragDropContext';
|
||||
import { useOrbitSongRowBehavior } from '../hooks/useOrbitSongRowBehavior';
|
||||
import { useNavigateToAlbum } from '../hooks/useNavigateToAlbum';
|
||||
import { useNavigateToArtist } from '../hooks/useNavigateToArtist';
|
||||
import { OpenArtistRefInline } from './OpenArtistRefInline';
|
||||
import { resolveTrackArtistRefs } from '../utils/playback/trackArtistRefs';
|
||||
|
||||
interface SongCardProps {
|
||||
song: SubsonicSong;
|
||||
@@ -31,7 +33,7 @@ function SongCard({
|
||||
}: SongCardProps) {
|
||||
const layoutPx = artworkSize ?? displayCssPx;
|
||||
const { t } = useTranslation();
|
||||
const navigate = useNavigate();
|
||||
const navigateToArtist = useNavigateToArtist();
|
||||
const openContextMenu = usePlayerStore(s => s.openContextMenu);
|
||||
const enqueue = usePlayerStore(s => s.enqueue);
|
||||
const coverRef = useTrackCoverRef(song, undefined, { libraryResolve: false });
|
||||
@@ -55,12 +57,7 @@ function SongCard({
|
||||
};
|
||||
|
||||
const handleClick = handlePlay;
|
||||
|
||||
const handleArtistClick = (e: React.MouseEvent) => {
|
||||
if (!song.artistId) return;
|
||||
e.stopPropagation();
|
||||
navigate(`/artist/${song.artistId}`);
|
||||
};
|
||||
const artistRefs = useMemo(() => resolveTrackArtistRefs(song), [song]);
|
||||
|
||||
const handleAlbumClick = (e: React.MouseEvent) => {
|
||||
if (!song.albumId) return;
|
||||
@@ -142,12 +139,16 @@ function SongCard({
|
||||
</div>
|
||||
<div className="song-card-info">
|
||||
<p className="song-card-title truncate" title={song.title}>{song.title}</p>
|
||||
<p
|
||||
className={`song-card-artist truncate${song.artistId ? ' track-artist-link' : ''}`}
|
||||
style={{ cursor: song.artistId ? 'pointer' : 'default' }}
|
||||
onClick={handleArtistClick}
|
||||
title={song.artist}
|
||||
>{song.artist}</p>
|
||||
<p className="song-card-artist truncate" title={song.artist}>
|
||||
<OpenArtistRefInline
|
||||
refs={artistRefs}
|
||||
fallbackName={song.artist}
|
||||
onGoArtist={id => navigateToArtist(id)}
|
||||
as="none"
|
||||
linkTag="span"
|
||||
linkClassName="track-artist-link"
|
||||
/>
|
||||
</p>
|
||||
{song.albumId && (
|
||||
<button
|
||||
type="button"
|
||||
|
||||
@@ -10,6 +10,7 @@ import { enqueueAndPlay } from '../utils/playback/playSong';
|
||||
import { useDragDrop } from '../contexts/DragDropContext';
|
||||
import { useOrbitSongRowBehavior } from '../hooks/useOrbitSongRowBehavior';
|
||||
import { formatTrackTime } from '../utils/format/formatDuration';
|
||||
import { resolveTrackArtistRefs } from '../utils/playback/trackArtistRefs';
|
||||
import { tooltipAttrs } from './tooltipAttrs';
|
||||
|
||||
interface Props {
|
||||
@@ -39,12 +40,7 @@ function SongRow({ song, showBpm }: Props) {
|
||||
enqueue([songToTrack(song)]);
|
||||
};
|
||||
|
||||
// Split multi-artist tracks into individually clickable links (OpenSubsonic
|
||||
// `artists[]`), falling back to the single flat artist when absent — mirrors
|
||||
// the album tracklist so a "A · B" credit isn't one link to a single artist.
|
||||
const artistRefs = song.artists && song.artists.length > 0
|
||||
? song.artists
|
||||
: [{ id: song.artistId, name: song.artist }];
|
||||
const artistRefs = resolveTrackArtistRefs(song);
|
||||
|
||||
const bpmTooltip =
|
||||
song.localBpmSource === 'analysis'
|
||||
|
||||
@@ -15,6 +15,7 @@ import { formatLongDuration } from '../../utils/format/formatDuration';
|
||||
import { formatLastSeen } from '../../utils/componentHelpers/userMgmtHelpers';
|
||||
import i18n from '../../i18n';
|
||||
import { offlineActionPolicy, type OfflineActionPolicy } from '../../utils/offline/offlineActionPolicy';
|
||||
import { resolveTrackArtistRefs } from '../../utils/playback/trackArtistRefs';
|
||||
|
||||
type ContextMenuFn = (
|
||||
x: number,
|
||||
@@ -137,9 +138,7 @@ export const TrackRow = React.memo(function TrackRow({
|
||||
</div>
|
||||
);
|
||||
case 'artist': {
|
||||
const artistRefs = song.artists && song.artists.length > 0
|
||||
? song.artists
|
||||
: [{ id: song.artistId, name: song.artist }];
|
||||
const artistRefs = resolveTrackArtistRefs(song);
|
||||
return (
|
||||
<div key="artist" className="track-artist-cell">
|
||||
{artistRefs.map((a, i) => (
|
||||
|
||||
@@ -8,6 +8,8 @@ import { formatLastSeen } from '../../utils/componentHelpers/userMgmtHelpers';
|
||||
import i18n from '../../i18n';
|
||||
import { formatTrackTime } from '../../utils/format/formatDuration';
|
||||
import StarRating from '../StarRating';
|
||||
import { OpenArtistRefInline } from '../OpenArtistRefInline';
|
||||
import { resolveTrackArtistRefs } from '../../utils/playback/trackArtistRefs';
|
||||
|
||||
export interface FavoriteSongRowCallbacks {
|
||||
activate: (song: SubsonicSong, index: number, e: React.MouseEvent) => void;
|
||||
@@ -100,7 +102,15 @@ function FavoriteSongRow({
|
||||
);
|
||||
case 'artist': return (
|
||||
<div key="artist" className="track-artist-cell">
|
||||
<span className={`track-artist${song.artistId ? ' track-artist-link' : ''}`} style={{ cursor: song.artistId ? 'pointer' : 'default' }} onClick={e => { if (song.artistId) { e.stopPropagation(); cb.navArtist(song.artistId, song.serverId); } }}>{song.artist}</span>
|
||||
<OpenArtistRefInline
|
||||
refs={resolveTrackArtistRefs(song)}
|
||||
fallbackName={song.artist}
|
||||
onGoArtist={id => cb.navArtist(id, song.serverId)}
|
||||
as="none"
|
||||
linkTag="span"
|
||||
linkClassName="track-artist track-artist-link"
|
||||
separatorClassName="track-artist-sep"
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
case 'album': return (
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import React from 'react';
|
||||
import React, { useMemo } from 'react';
|
||||
import { useNavigate } from 'react-router-dom';
|
||||
import type { SubsonicSong } from '../../api/subsonicTypes';
|
||||
import { resolveTrackArtistRefs } from '../../utils/playback/trackArtistRefs';
|
||||
|
||||
/**
|
||||
* Multi-artist credit for playlist track rows (main list + suggestions).
|
||||
@@ -11,9 +12,7 @@ import type { SubsonicSong } from '../../api/subsonicTypes';
|
||||
*/
|
||||
export function PlaylistArtistCell({ song }: { song: SubsonicSong }) {
|
||||
const navigate = useNavigate();
|
||||
const artistRefs = song.artists && song.artists.length > 0
|
||||
? song.artists
|
||||
: [{ id: song.artistId, name: song.artist }];
|
||||
const artistRefs = useMemo(() => resolveTrackArtistRefs(song), [song]);
|
||||
return (
|
||||
<div className="track-artist-cell">
|
||||
{artistRefs.map((a, i) => (
|
||||
|
||||
@@ -13,6 +13,8 @@ import { ndListSongs, ndInvalidateSongsCache } from '../../api/navidromeBrowse';
|
||||
import { usePerfProbeFlags } from '../../utils/perf/perfFlags';
|
||||
import { useNavigateToAlbum } from '../../hooks/useNavigateToAlbum';
|
||||
import { useNavigateToArtist } from '../../hooks/useNavigateToArtist';
|
||||
import { OpenArtistRefInline } from '../OpenArtistRefInline';
|
||||
import { resolveTrackArtistRefs } from '../../utils/playback/trackArtistRefs';
|
||||
|
||||
const RANDOM_RAIL_SIZE = 18;
|
||||
const RATED_RAIL_FETCH = 60;
|
||||
@@ -105,13 +107,7 @@ export default function TracksPageChrome({
|
||||
[random, hero],
|
||||
);
|
||||
|
||||
// Split a multi-artist feature into individually clickable links
|
||||
// (OpenSubsonic `artists[]`), falling back to the single flat artist.
|
||||
const heroArtistRefs = hero
|
||||
? (hero.artists && hero.artists.length > 0
|
||||
? hero.artists
|
||||
: [{ id: hero.artistId, name: hero.artist }])
|
||||
: [];
|
||||
const heroArtistRefs = hero ? resolveTrackArtistRefs(hero) : [];
|
||||
|
||||
return (
|
||||
<>
|
||||
@@ -148,16 +144,15 @@ export default function TracksPageChrome({
|
||||
</span>
|
||||
<h2 className="tracks-hero-title" title={hero.title}>{hero.title}</h2>
|
||||
<p className="tracks-hero-meta">
|
||||
{heroArtistRefs.map((a, i) => (
|
||||
<React.Fragment key={a.id ?? a.name ?? i}>
|
||||
{i > 0 && <span className="track-artist-sep"> · </span>}
|
||||
<span
|
||||
className={a.id ? 'track-artist-link' : ''}
|
||||
style={{ cursor: a.id ? 'pointer' : 'default' }}
|
||||
onClick={() => a.id && navigateToArtist(a.id)}
|
||||
>{a.name ?? hero.artist}</span>
|
||||
</React.Fragment>
|
||||
))}
|
||||
<OpenArtistRefInline
|
||||
refs={heroArtistRefs}
|
||||
fallbackName={hero.artist}
|
||||
onGoArtist={id => navigateToArtist(id)}
|
||||
as="none"
|
||||
linkTag="span"
|
||||
linkClassName="track-artist-link"
|
||||
separatorClassName="track-artist-sep"
|
||||
/>
|
||||
{hero.album && (
|
||||
<>
|
||||
<span className="tracks-hero-meta-dot">·</span>
|
||||
|
||||
@@ -29,6 +29,14 @@ describe('deriveAlbumArtistRefs', () => {
|
||||
expect(deriveAlbumArtistRefs({ ...baseAlbum(), artistId: ' ', artist: 'Solo' }))
|
||||
.toEqual([{ name: 'Solo' }]);
|
||||
});
|
||||
|
||||
it('coerces a single-object OpenSubsonic artists payload', () => {
|
||||
const album: SubsonicAlbum = {
|
||||
...baseAlbum(),
|
||||
artists: { id: 'a1', name: 'Solo' } as unknown as SubsonicAlbum['artists'],
|
||||
};
|
||||
expect(deriveAlbumArtistRefs(album)).toEqual([{ id: 'a1', name: 'Solo' }]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('deriveAlbumHeaderArtistRefs', () => {
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
import type { SubsonicAlbum, SubsonicOpenArtistRef, SubsonicSong } from '../../api/subsonicTypes';
|
||||
import { coerceOpenArtistRefs } from '../openArtistRefs';
|
||||
|
||||
function nonEmpty(refs: SubsonicOpenArtistRef[] | undefined): refs is SubsonicOpenArtistRef[] {
|
||||
return !!refs && refs.length > 0;
|
||||
function nonEmpty(refs: SubsonicOpenArtistRef[]): refs is SubsonicOpenArtistRef[] {
|
||||
return refs.length > 0;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -10,7 +11,8 @@ function nonEmpty(refs: SubsonicOpenArtistRef[] | undefined): refs is SubsonicOp
|
||||
* OpenSubsonic `artists` array; falls back to legacy `artist` + `artistId`.
|
||||
*/
|
||||
export function deriveAlbumArtistRefs(album: SubsonicAlbum): SubsonicOpenArtistRef[] {
|
||||
if (nonEmpty(album.artists)) return album.artists;
|
||||
const albumArtists = coerceOpenArtistRefs(album.artists);
|
||||
if (nonEmpty(albumArtists)) return albumArtists;
|
||||
const name = album.artist?.trim() || '—';
|
||||
const id = album.artistId?.trim();
|
||||
return id ? [{ id, name }] : [{ name }];
|
||||
@@ -26,9 +28,11 @@ export function deriveAlbumHeaderArtistRefs(
|
||||
album: SubsonicAlbum,
|
||||
songs: SubsonicSong[],
|
||||
): SubsonicOpenArtistRef[] {
|
||||
if (nonEmpty(album.artists)) return album.artists;
|
||||
const albumArtists = coerceOpenArtistRefs(album.artists);
|
||||
if (nonEmpty(albumArtists)) return albumArtists;
|
||||
for (const s of songs) {
|
||||
if (nonEmpty(s.albumArtists)) return s.albumArtists;
|
||||
const songAlbumArtists = coerceOpenArtistRefs(s.albumArtists);
|
||||
if (nonEmpty(songAlbumArtists)) return songAlbumArtists;
|
||||
}
|
||||
return deriveAlbumArtistRefs(album);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,19 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { coerceOpenArtistRefs } from './openArtistRefs';
|
||||
|
||||
describe('coerceOpenArtistRefs', () => {
|
||||
it('returns an empty array for nullish input', () => {
|
||||
expect(coerceOpenArtistRefs(undefined)).toEqual([]);
|
||||
expect(coerceOpenArtistRefs(null)).toEqual([]);
|
||||
});
|
||||
|
||||
it('passes through arrays', () => {
|
||||
const refs = [{ id: 'a1', name: 'One' }, { id: 'a2', name: 'Two' }];
|
||||
expect(coerceOpenArtistRefs(refs)).toBe(refs);
|
||||
});
|
||||
|
||||
it('wraps a single ref object from Subsonic JSON', () => {
|
||||
const ref = { id: 'a1', name: 'Solo' };
|
||||
expect(coerceOpenArtistRefs(ref)).toEqual([ref]);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,11 @@
|
||||
import type { SubsonicOpenArtistRef } from '../api/subsonicTypes';
|
||||
|
||||
/** Subsonic JSON may return one ref object instead of a one-element array. */
|
||||
export function coerceOpenArtistRefs(
|
||||
refs: SubsonicOpenArtistRef[] | SubsonicOpenArtistRef | undefined | null,
|
||||
): SubsonicOpenArtistRef[] {
|
||||
if (refs == null) return [];
|
||||
if (Array.isArray(refs)) return refs;
|
||||
if (typeof refs === 'object') return [refs];
|
||||
return [];
|
||||
}
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { SubsonicSong } from '../../api/subsonicTypes';
|
||||
import type { Track } from '../../store/playerStoreTypes';
|
||||
import { coerceOpenArtistRefs } from '../openArtistRefs';
|
||||
import { activeServerProfileId } from './trackServerScope';
|
||||
|
||||
export function songToTrack(song: SubsonicSong): Track {
|
||||
@@ -10,7 +11,10 @@ export function songToTrack(song: SubsonicSong): Track {
|
||||
album: song.album,
|
||||
albumId: song.albumId,
|
||||
artistId: song.artistId,
|
||||
artists: song.artists && song.artists.length > 0 ? song.artists : undefined,
|
||||
artists: (() => {
|
||||
const artists = coerceOpenArtistRefs(song.artists);
|
||||
return artists.length > 0 ? artists : undefined;
|
||||
})(),
|
||||
duration: song.duration,
|
||||
coverArt: song.coverArt,
|
||||
discNumber: song.discNumber,
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import type { Track } from '../../store/playerStoreTypes';
|
||||
import { primaryTrackArtistRef, resolveTrackArtistRefs } from './trackArtistRefs';
|
||||
|
||||
describe('resolveTrackArtistRefs', () => {
|
||||
@@ -21,6 +22,14 @@ describe('resolveTrackArtistRefs', () => {
|
||||
it('returns name-only ref when no id', () => {
|
||||
expect(resolveTrackArtistRefs({ artist: 'Unknown' })).toEqual([{ name: 'Unknown' }]);
|
||||
});
|
||||
|
||||
it('coerces a single-object OpenSubsonic artists payload', () => {
|
||||
expect(resolveTrackArtistRefs({
|
||||
artist: 'Joined',
|
||||
artistId: 'legacy',
|
||||
artists: { id: 'a1', name: 'Solo' } as unknown as Track['artists'],
|
||||
})).toEqual([{ id: 'a1', name: 'Solo' }]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('primaryTrackArtistRef', () => {
|
||||
|
||||
@@ -1,12 +1,14 @@
|
||||
import type { SubsonicOpenArtistRef } from '../../api/subsonicTypes';
|
||||
import type { Track } from '../../store/playerStoreTypes';
|
||||
import { coerceOpenArtistRefs } from '../openArtistRefs';
|
||||
|
||||
type TrackArtistFields = Pick<Track, 'artist' | 'artistId' | 'artists'>;
|
||||
|
||||
/** OpenSubsonic `artists` when present; else legacy `artistId` + `artist` (album track rows). */
|
||||
export function resolveTrackArtistRefs(track: TrackArtistFields): SubsonicOpenArtistRef[] {
|
||||
if (track.artists && track.artists.length > 0) {
|
||||
return track.artists;
|
||||
const structured = coerceOpenArtistRefs(track.artists);
|
||||
if (structured.length > 0) {
|
||||
return structured;
|
||||
}
|
||||
const id = track.artistId?.trim();
|
||||
if (id) {
|
||||
|
||||
Reference in New Issue
Block a user