Skip to content

Fix Multiple Artists #94

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MHRGA merged 2 commits into from
May 22, 2026
Merged

Fix Multiple Artists #94

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e9283f5
fix multiple artists
May 22, 2026
e8b586e
Merge pull request #93 from TrueNorthIT/fix/mhr/multiple-artists
MHRGA May 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion .claude/settings.local.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@
"Bash(npm ci *)",
"Bash(git commit -m ' *)",
"Bash(git reset *)",
"Bash(git restore *)"
"Bash(git restore *)",
"Bash(gh issue *)",
"PowerShell(gh issue *)",
"WebFetch(domain:github.com)",
"Bash(az account show *)",
"Bash(az account *)"
]
}
}
54 changes: 42 additions & 12 deletions renderer/src/components/CardRow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,34 @@
import { getName, browseSub, getItemArt, isAlbum, isArtist, isPlaylist, isContainer, isProgram } from '../lib/itemHelpers';
import { useArtistImage } from '../hooks/useArtistBrowse';
import { MediaCard } from './common/MediaCard';
import type { SonosItem } from '../types/sonos';
import type { SonosItem, SonosItemId } from '../types/sonos';
import styles from '../styles/CardRow.module.css';

// Artist cards used the item's imageUrl, which for leaderboard-derived artists
// is undefined (we no longer seed the artist map with track album-art). Fetch
// the real artist image from the Sonos artist endpoint instead.
function ArtistMediaCard({
item,
onOpen,
}: {
item: SonosItem;
onOpen: (item: SonosItem) => void;
}) {
const rid = (item.resource?.id ?? (typeof item.id === 'object' ? item.id : undefined)) as
| SonosItemId
| undefined;
const { data: artistImg } = useArtistImage(rid?.objectId, rid?.serviceId, rid?.accountId);
return (
<MediaCard
name={getName(item)}
sub={browseSub(item)}
artUrl={artistImg ?? getItemArt(item)}
circular
onOpen={() => onOpen(item)}
/>
);
}

export function CardRow({
items,
isLoading,
Expand All @@ -29,17 +55,21 @@ export function CardRow({
if (!items.length) return null;
return (
<div className={styles.cardRow} style={sizeStyle}>
{items.map((item, i) => (
<MediaCard
key={i}
name={getName(item)}
sub={browseSub(item)}
artUrl={getItemArt(item)}
circular={isArtist(item)}
onAdd={(isContainer(item) || isArtist(item)) ? undefined : () => onAdd(item)}
onOpen={(isAlbum(item) || isPlaylist(item) || isContainer(item) || isProgram(item) || isArtist(item)) ? () => onOpen(item) : undefined}
/>
))}
{items.map((item, i) =>
isArtist(item) ? (
<ArtistMediaCard key={i} item={item} onOpen={onOpen} />
) : (
<MediaCard
key={i}
name={getName(item)}
sub={browseSub(item)}
artUrl={getItemArt(item)}
circular={false}
onAdd={isContainer(item) ? undefined : () => onAdd(item)}
onOpen={(isAlbum(item) || isPlaylist(item) || isContainer(item) || isProgram(item)) ? () => onOpen(item) : undefined}
/>
),
)}
</div>
);
}
164 changes: 121 additions & 43 deletions renderer/src/components/LeaderboardPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { useState } from 'react';
import { useState, Fragment } from 'react';
import { useNavigate } from 'react-router-dom';
import { Info, X } from 'lucide-react';
import type { GameRankTierKey } from '../hooks/useDailyGame';
import { useStats, StatsPeriod } from '../hooks/useStats';
import { useGameRankings } from '../hooks/useDailyGame';
import { useImage } from '../hooks/useImage';
import { useArtistImage } from '../hooks/useArtistBrowse';
import { getGameRankIcon, getGameRankInfoImage } from '../lib/gameRankAssets';
import { useResolveAndOpen } from '../hooks/useResolveAndOpen';
import styles from '../styles/LeaderboardPanel.module.css';
Expand All @@ -15,6 +16,75 @@ function CachedArt({ url, className }: { url: string | undefined; className: str
return <img className={className} src={cached} alt="" loading="lazy" />;
}

// Tries to fetch the real artist image (so YT Music shows the artist's photo)
// and falls back to whatever the event sent us — typically the track's album
// art. The Sonos artist endpoint needs `defaults` we don't store in events, so
// the fetch quietly returns null on many services; the fallback keeps the row
// looking populated instead of empty.
function ArtistAvatar({
artistId,
serviceId,
accountId,
fallbackUrl,
className,
placeholderClassName,
}: {
artistId: string | undefined;
serviceId: string | undefined;
accountId: string | undefined;
fallbackUrl?: string;
className: string;
placeholderClassName: string;
}) {
const { data: imageUrl } = useArtistImage(artistId, serviceId, accountId);
const cached = useImage(imageUrl ?? fallbackUrl ?? null);
if (!cached) return <div className={placeholderClassName} />;
return <img className={className} src={cached} alt="" loading="lazy" />;
}

// Multi-artist subtitles arrive as a single joined string (e.g. "Sonny Stitt, Kenny Garrett").
// Rendering them as one button means resolveAndOpen searches for the whole string and never
// matches an individual artist; for album rows the button also swallows the row click via
// stopPropagation. Splitting on commas gives each artist its own link.
function ArtistLinks({
artist,
artistId,
serviceId,
accountId,
onNavigateArtist,
onResolveArtist,
}: {
artist: string;
artistId?: string;
serviceId?: string;
accountId?: string;
onNavigateArtist: (name: string, artistId: string, serviceId: string, accountId: string) => void;
onResolveArtist: (name: string) => void;
}) {
const parts = artist.split(/,\s*/).map((s) => s.trim()).filter(Boolean);
if (parts.length === 0) return null;
const canUseDirect = parts.length === 1 && !!artistId && !!serviceId && !!accountId;
return (
<>
{parts.map((name, i) => (
<Fragment key={i}>
{i > 0 && ', '}
<button
className={styles.artistLink}
onClick={(e) => {
e.stopPropagation();
if (canUseDirect) onNavigateArtist(name, artistId!, serviceId!, accountId!);
else onResolveArtist(name);
}}
>
{name}
</button>
</Fragment>
))}
</>
);
}

const PERIODS: { value: StatsPeriod; label: string }[] = [
{ value: 'today', label: 'Today' },
{ value: 'week', label: 'This week' },
Expand Down Expand Up @@ -215,27 +285,28 @@ export function LeaderboardPanel() {
<span className={styles.trackName}>{t.trackName}</span>
<span className={styles.trackSub}>
{t.artist ? (
<button
className={styles.artistLink}
onClick={(e) => {
e.stopPropagation();
if (t.artistId && t.serviceId && t.accountId)
navigate(`/artist/${encodeURIComponent(t.artistId)}`, {
state: {
item: {
<ArtistLinks
artist={t.artist}
artistId={t.artistId}
serviceId={t.serviceId}
accountId={t.accountId}
onNavigateArtist={(name, aid, sid, acc) =>
navigate(`/artist/${encodeURIComponent(aid)}`, {
state: {
item: {
type: 'ARTIST',
title: name,
name,
resource: {
type: 'ARTIST',
resource: {
type: 'ARTIST',
id: { objectId: t.artistId, serviceId: t.serviceId, accountId: t.accountId },
},
id: { objectId: aid, serviceId: sid, accountId: acc },
},
},
});
else resolveAndOpen(t.artist, 'artist');
}}
>
{t.artist}
</button>
},
})
}
onResolveArtist={(name) => resolveAndOpen(name, 'artist', { serviceId: t.serviceId, accountId: t.accountId })}
/>
) : null}
{t.artist && t.album ? ' · ' : null}
{t.album ? (
Expand Down Expand Up @@ -275,11 +346,14 @@ export function LeaderboardPanel() {
data.topArtists.map((a, i) => (
<div key={i} className={styles.artistRow}>
<span className={styles.rankNum}>{i + 1}</span>
{a.imageUrl ? (
<CachedArt url={a.imageUrl} className={styles.artistArt} />
) : (
<div className={styles.artistArtPh} />
)}
<ArtistAvatar
artistId={a.artistId}
serviceId={a.serviceId}
accountId={a.accountId}
fallbackUrl={a.imageUrl}
className={styles.artistArt}
placeholderClassName={styles.artistArtPh}
/>
<button
className={styles.artistLink}
onClick={(e) => {
Expand All @@ -289,14 +363,17 @@ export function LeaderboardPanel() {
state: {
item: {
type: 'ARTIST',
title: a.artist,
name: a.artist,
imageUrl: a.imageUrl,
resource: {
type: 'ARTIST',
id: { objectId: a.artistId, serviceId: a.serviceId, accountId: a.accountId },
},
},
},
});
else resolveAndOpen(a.artist, 'artist');
else resolveAndOpen(a.artist, 'artist', { serviceId: a.serviceId, accountId: a.accountId });
}}
>
{a.artist}
Expand Down Expand Up @@ -340,27 +417,28 @@ export function LeaderboardPanel() {
<span className={styles.trackName}>{a.album}</span>
<span className={styles.trackSub}>
{a.artist ? (
<button
className={styles.artistLink}
onClick={(e) => {
e.stopPropagation();
if (a.artistId && a.serviceId && a.accountId)
navigate(`/artist/${encodeURIComponent(a.artistId)}`, {
state: {
item: {
<ArtistLinks
artist={a.artist}
artistId={a.artistId}
serviceId={a.serviceId}
accountId={a.accountId}
onNavigateArtist={(name, aid, sid, acc) =>
navigate(`/artist/${encodeURIComponent(aid)}`, {
state: {
item: {
type: 'ARTIST',
title: name,
name,
resource: {
type: 'ARTIST',
resource: {
type: 'ARTIST',
id: { objectId: a.artistId, serviceId: a.serviceId, accountId: a.accountId },
},
id: { objectId: aid, serviceId: sid, accountId: acc },
},
},
});
else resolveAndOpen(a.artist, 'artist');
}}
>
{a.artist}
</button>
},
})
}
onResolveArtist={(name) => resolveAndOpen(name, 'artist', { serviceId: a.serviceId, accountId: a.accountId })}
/>
) : null}
</span>
</div>
Expand Down
54 changes: 54 additions & 0 deletions renderer/src/components/__tests__/LeaderboardPanel.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,14 @@ vi.mock('../../hooks/useImage', () => ({
useImage: (url: string | null | undefined) => url ?? null,
}));

// LeaderboardPanel renders artist avatars via useArtistImage; the tests don't
// wrap with a QueryClientProvider, so stub it to return no image.
vi.mock('../../hooks/useArtistBrowse', () => ({
useArtistImage: () => ({ data: null }),
useArtistBrowse: () => ({ data: undefined, isLoading: false, error: null }),
artistQueryOptions: vi.fn(() => ({ queryKey: [], queryFn: async () => null })),
}));

vi.mock('../../hooks/useDailyGame', () => ({
useGameLeaderboard: () => ({ data: { gameId: 'today', scores: [] }, isLoading: false }),
useGameRankings: (userName: string | null | undefined, enabled: boolean) => mockUseGameRankings(userName, enabled),
Expand Down Expand Up @@ -513,6 +521,52 @@ describe('LeaderboardPanel', () => {
});
});

it('renders each artist as a separate link for a multi-artist album', () => {
mockLoaded({
...mockData,
topAlbums: [
{
album: 'Diz N Bird',
artist: 'Sonny Stitt, Kenny Garrett',
albumId: 'alb-multi',
serviceId: 'svc1',
accountId: 'acc1',
count: 2,
},
],
});
render(<LeaderboardPanel />);
// Each artist should render as its own clickable button, not as one joined string.
expect(screen.getByRole('button', { name: 'Sonny Stitt' })).toBeInTheDocument();
expect(screen.getByRole('button', { name: 'Kenny Garrett' })).toBeInTheDocument();
expect(screen.queryByRole('button', { name: 'Sonny Stitt, Kenny Garrett' })).toBeNull();
});

it('clicking a single artist in a multi-artist album resolves that individual artist', async () => {
mockLoaded({
...mockData,
topAlbums: [
{
album: 'Diz N Bird',
artist: 'Sonny Stitt, Kenny Garrett',
albumId: 'alb-multi',
serviceId: 'svc1',
accountId: 'acc1',
count: 2,
},
],
});
// The search returns the individual artist matching the clicked name.
mockSearchResolves({
artist: { name: 'Kenny Garrett', objectId: 'kg-id', serviceId: 'svc1', accountId: 'acc1' },
});
render(<LeaderboardPanel />);
fireEvent.click(screen.getByRole('button', { name: 'Kenny Garrett' }));
await waitFor(() => {
expect(mockNavigate).toHaveBeenCalledWith(expect.stringMatching(/^\/artist\//), expect.anything());
});
});

it('clicking album row without albumId picks the candidate that matches the displayed artist', async () => {
mockLoaded({
...mockData,
Expand Down
Loading
Loading