Merge pull request #689 from kettui/refactor/more-native-links

Replace programmatic navigation with anchor links on issues & stats pages
This commit is contained in:
BoulderBadgeDad 2026-05-24 14:41:25 -07:00 committed by GitHub
commit 29ba4e0049
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 158 additions and 124 deletions

View file

@ -136,7 +136,11 @@ describe('issues route', () => {
it('renders stats and list items through the app router', async () => { it('renders stats and list items through the app router', async () => {
renderIssuesRoute(); renderIssuesRoute();
await waitFor(() => expect(screen.getByTestId('issue-counts')).toHaveTextContent('2')); await waitFor(() => expect(screen.getByTestId('issue-counts')).toHaveTextContent('2'));
expect(await screen.findByTestId('issue-card-7')).toHaveTextContent('Bad tags'); const issueCard = await screen.findByRole('link', { name: /Bad tags/i });
expect(issueCard).toHaveAttribute('href', expect.stringContaining('/issues?'));
expect(issueCard).toHaveAttribute('href', expect.stringContaining('status=open'));
expect(issueCard).toHaveAttribute('href', expect.stringContaining('category=all'));
expect(issueCard).toHaveAttribute('href', expect.stringContaining('issueId=7'));
}); });
it('loads the detail modal from the route search state', async () => { it('loads the detail modal from the route search state', async () => {
@ -157,17 +161,22 @@ describe('issues route', () => {
it('opens and closes the detail modal', async () => { it('opens and closes the detail modal', async () => {
const { history } = renderIssuesRoute(); const { history } = renderIssuesRoute();
fireEvent.click(await screen.findByTestId('issue-card-7')); fireEvent.click(await screen.findByRole('link', { name: /Bad tags/i }));
await waitFor(() => expect(screen.getByRole('dialog')).toHaveTextContent('Issue #7')); await waitFor(() => expect(screen.getByRole('dialog')).toHaveTextContent('Issue #7'));
await waitFor(() => expect(history.location.search).toContain('issueId=7')); await waitFor(() => expect(history.location.search).toContain('issueId=7'));
fireEvent.click(screen.getByRole('button', { name: /close issue detail/i })); const closeLink = screen.getByRole('link', { name: /^close$/i });
expect(closeLink).toHaveAttribute(
'href',
expect.stringContaining('/issues?status=open&category=all'),
);
fireEvent.click(closeLink);
await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument());
await waitFor(() => expect(history.location.search).toBe('?status=open&category=all')); await waitFor(() => expect(history.location.search).toBe('?status=open&category=all'));
}); });
it('closes the detail modal with Escape', async () => { it('closes the detail modal with Escape', async () => {
const { history } = renderIssuesRoute(); const { history } = renderIssuesRoute();
fireEvent.click(await screen.findByTestId('issue-card-7')); fireEvent.click(await screen.findByRole('link', { name: /Bad tags/i }));
await waitFor(() => expect(screen.getByRole('dialog')).toHaveTextContent('Issue #7')); await waitFor(() => expect(screen.getByRole('dialog')).toHaveTextContent('Issue #7'));
await waitFor(() => expect(history.location.search).toContain('issueId=7')); await waitFor(() => expect(history.location.search).toContain('issueId=7'));
@ -179,7 +188,7 @@ describe('issues route', () => {
it('focuses the detail modal close button on open', async () => { it('focuses the detail modal close button on open', async () => {
renderIssuesRoute(); renderIssuesRoute();
fireEvent.click(await screen.findByTestId('issue-card-7')); fireEvent.click(await screen.findByRole('link', { name: /Bad tags/i }));
const closeButton = await screen.findByRole('button', { const closeButton = await screen.findByRole('button', {
name: /close issue detail/i, name: /close issue detail/i,
@ -190,7 +199,7 @@ describe('issues route', () => {
it('invokes the shared workflow adapter for admin downloads', async () => { it('invokes the shared workflow adapter for admin downloads', async () => {
renderIssuesRoute(); renderIssuesRoute();
fireEvent.click(await screen.findByTestId('issue-card-7')); fireEvent.click(await screen.findByRole('link', { name: /Bad tags/i }));
fireEvent.click(await screen.findByRole('button', { name: /download album/i })); fireEvent.click(await screen.findByRole('button', { name: /download album/i }));
await waitFor(() => expect(workflowActions.openDownloadMissingAlbum).toHaveBeenCalled()); await waitFor(() => expect(workflowActions.openDownloadMissingAlbum).toHaveBeenCalled());
expect(workflowActions.openDownloadMissingAlbum).toHaveBeenCalledWith( expect(workflowActions.openDownloadMissingAlbum).toHaveBeenCalledWith(

View file

@ -878,6 +878,43 @@
font-size: 13px; font-size: 13px;
} }
.modalLinkButton {
display: inline-flex;
align-items: center;
justify-content: center;
gap: 8px;
border: 1px solid rgba(255, 255, 255, 0.1);
border-radius: 10px;
background: rgba(255, 255, 255, 0.05);
color: #fff;
cursor: pointer;
font: inherit;
font-size: 13px;
font-weight: 600;
line-height: 1.2;
min-height: 36px;
padding: 8px 12px;
text-decoration: none;
transition:
transform 0.18s ease,
border-color 0.18s ease,
box-shadow 0.18s ease,
background 0.18s ease,
color 0.18s ease;
}
.modalLinkButton:hover {
transform: translateY(-1px);
border-color: rgba(255, 255, 255, 0.18);
background: rgba(255, 255, 255, 0.08);
}
.modalLinkButton:focus-visible {
outline: none;
border-color: rgba(var(--accent-light-rgb), 0.55);
box-shadow: 0 0 0 3px rgba(var(--accent-light-rgb), 0.12);
}
.modalButtonSecondary { .modalButtonSecondary {
background: rgba(255, 255, 255, 0.1); background: rgba(255, 255, 255, 0.1);
color: rgba(255, 255, 255, 0.8); color: rgba(255, 255, 255, 0.8);

View file

@ -1,4 +1,5 @@
import { useMutation, useQuery } from '@tanstack/react-query'; import { useMutation, useQuery } from '@tanstack/react-query';
import { Link } from '@tanstack/react-router';
import { useEffect, useMemo, useState, type ReactNode } from 'react'; import { useEffect, useMemo, useState, type ReactNode } from 'react';
import { DialogBody, DialogFooter, DialogFrame, DialogHeader } from '@/components/dialog'; import { DialogBody, DialogFooter, DialogFrame, DialogHeader } from '@/components/dialog';
@ -22,6 +23,7 @@ import {
ISSUE_CATEGORY_META, ISSUE_CATEGORY_META,
parseSnapshot, parseSnapshot,
} from '../-issues.helpers'; } from '../-issues.helpers';
import { Route } from '../route';
import styles from './issue-detail-modal.module.css'; import styles from './issue-detail-modal.module.css';
export function IssueDetailModal({ export function IssueDetailModal({
@ -213,9 +215,14 @@ export function IssueDetailModal({
/> />
<DialogBody>{renderIssueDetailContent()}</DialogBody> <DialogBody>{renderIssueDetailContent()}</DialogBody>
<DialogFooter> <DialogFooter>
<Button className={styles.modalButtonSecondary} type="button" onClick={onClose}> <Link
className={`${styles.modalLinkButton} ${styles.modalButtonSecondary}`}
replace
search={(prev) => ({ ...prev, issueId: undefined })}
to={Route.fullPath}
>
Close Close
</Button> </Link>
{issue && ( {issue && (
<> <>
{statusButtons} {statusButtons}

View file

@ -159,6 +159,7 @@
width: 100%; width: 100%;
padding: 14px 16px; padding: 14px 16px;
text-align: left; text-align: left;
text-decoration: none;
border-radius: 12px; border-radius: 12px;
border: 1px solid rgba(255, 255, 255, 0.06); border: 1px solid rgba(255, 255, 255, 0.06);
background: rgba(255, 255, 255, 0.03); background: rgba(255, 255, 255, 0.03);

View file

@ -1,5 +1,5 @@
import { useQuery, useQueryClient } from '@tanstack/react-query'; import { useQuery, useQueryClient } from '@tanstack/react-query';
import { useNavigate } from '@tanstack/react-router'; import { Link, useNavigate } from '@tanstack/react-router';
import { Select } from '@/components/form'; import { Select } from '@/components/form';
import { Show } from '@/components/primitives'; import { Show } from '@/components/primitives';
@ -73,13 +73,6 @@ function IssueBoard() {
...issueListQueryOptions(profileId, params), ...issueListQueryOptions(profileId, params),
}); });
const openIssue = (issueId: number) => {
void navigate({
to: Route.fullPath,
search: (prev) => ({ ...prev, issueId }),
});
};
const onCategoryChange = (category: IssuesSearch['category']) => { const onCategoryChange = (category: IssuesSearch['category']) => {
void navigate({ void navigate({
to: Route.fullPath, to: Route.fullPath,
@ -112,7 +105,6 @@ function IssueBoard() {
issuesError={issuesQuery.error} issuesError={issuesQuery.error}
issuesLoading={issuesQuery.isLoading} issuesLoading={issuesQuery.isLoading}
showReporterName={isAdmin} showReporterName={isAdmin}
onIssueSelect={openIssue}
statusFilter={params.status} statusFilter={params.status}
/> />
</div> </div>
@ -225,7 +217,6 @@ function IssueBoardList({
issues, issues,
issuesError, issuesError,
issuesLoading, issuesLoading,
onIssueSelect,
showReporterName, showReporterName,
statusFilter, statusFilter,
}: { }: {
@ -233,7 +224,6 @@ function IssueBoardList({
issues: IssueRecord[]; issues: IssueRecord[];
issuesError: unknown; issuesError: unknown;
issuesLoading: boolean; issuesLoading: boolean;
onIssueSelect: (issueId: number) => void;
showReporterName: boolean; showReporterName: boolean;
statusFilter: IssuesSearch['status']; statusFilter: IssuesSearch['status'];
}) { }) {
@ -285,7 +275,6 @@ function IssueBoardList({
key={issue.id} key={issue.id}
issue={issue} issue={issue}
showReporterName={showReporterName} showReporterName={showReporterName}
onIssueSelect={onIssueSelect}
/> />
)); ));
} }
@ -294,11 +283,9 @@ function IssueBoardList({
function IssueBoardCard({ function IssueBoardCard({
issue, issue,
showReporterName, showReporterName,
onIssueSelect,
}: { }: {
issue: IssueRecord; issue: IssueRecord;
showReporterName: boolean; showReporterName: boolean;
onIssueSelect: (issueId: number) => void;
}) { }) {
const snapshot = parseSnapshot(issue.snapshot_data); const snapshot = parseSnapshot(issue.snapshot_data);
const artwork = getIssueArtwork(snapshot); const artwork = getIssueArtwork(snapshot);
@ -311,11 +298,11 @@ function IssueBoardCard({
const createdDate = formatIssueDate(issue.created_at); const createdDate = formatIssueDate(issue.created_at);
return ( return (
<button <Link
className={styles.issueCard} className={styles.issueCard}
type="button"
data-testid={`issue-card-${issue.id}`} data-testid={`issue-card-${issue.id}`}
onClick={() => onIssueSelect(issue.id)} to={Route.fullPath}
search={(prev) => ({ ...prev, issueId: issue.id })}
> >
<div className={styles.issueCardLeft}> <div className={styles.issueCardLeft}>
{artwork ? ( {artwork ? (
@ -360,7 +347,7 @@ function IssueBoardCard({
title={`${issue.priority} priority`} title={`${issue.priority} priority`}
/> />
</div> </div>
</button> </Link>
); );
} }

View file

@ -117,14 +117,29 @@ describe('stats route', () => {
await waitFor(() => expect(history.location.search).toContain('range=30d')); await waitFor(() => expect(history.location.search).toContain('range=30d'));
}); });
it('hands artist detail navigation directly to the shell bridge', async () => { it('links artist names to the artist-detail route', async () => {
renderStatsRoute(); const { history } = renderStatsRoute();
fireEvent.click(await screen.findByRole('button', { name: 'Artist A' })); const bubbleLink = await screen.findByRole('link', {
name: 'Open artist detail for Artist A',
});
expect(bubbleLink).toHaveAttribute('href', '/artist-detail/library/7');
expect(window.SoulSyncWebShellBridge?.navigateToArtistDetail).toHaveBeenCalledWith( const rankedLink = screen.getByRole('link', { name: 'Artist A' });
7, expect(rankedLink).toHaveAttribute('href', '/artist-detail/library/7');
'Artist A',
fireEvent.click(bubbleLink);
await waitFor(() => expect(history.location.pathname).toBe('/artist-detail/library/7'));
await waitFor(() =>
expect(window.SoulSyncWebShellBridge?.navigateToArtistDetail).toHaveBeenCalledWith(
'7',
'',
null,
{
skipRouteChange: true,
},
),
); );
}); });

View file

@ -364,10 +364,13 @@
border: none; border: none;
padding: 0; padding: 0;
cursor: pointer; cursor: pointer;
color: inherit;
text-decoration: none;
font: inherit;
transition: transform 0.2s ease; transition: transform 0.2s ease;
} }
.statsArtistBubble:disabled { .statsArtistBubbleDisabled {
cursor: default; cursor: default;
} }
@ -375,6 +378,10 @@
transform: translateY(-3px); transform: translateY(-3px);
} }
.statsArtistBubbleDisabled:hover {
transform: none;
}
.statsBubbleImage { .statsBubbleImage {
border-radius: 50%; border-radius: 50%;
background-size: cover; background-size: cover;

View file

@ -1,6 +1,6 @@
import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query'; import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query';
import { useNavigate } from '@tanstack/react-router'; import { Link, useNavigate } from '@tanstack/react-router';
import { type ReactNode, useEffect, useRef, useState } from 'react'; import { type ComponentPropsWithoutRef, type ReactNode, useEffect, useRef, useState } from 'react';
import { import {
Bar, Bar,
BarChart, BarChart,
@ -72,6 +72,8 @@ const STATS_CHART_CURSOR = {
fill: 'rgba(var(--accent-rgb), 0.12)', fill: 'rgba(var(--accent-rgb), 0.12)',
} as const; } as const;
const ARTIST_DETAIL_SOURCE = 'library' as const;
export function StatsPage() { export function StatsPage() {
const bridge = useReactPageShell('stats'); const bridge = useReactPageShell('stats');
@ -136,10 +138,6 @@ export function StatsPage() {
}); });
}; };
const openArtistDetail = (artistId: string | number, artistName: string) => {
bridge.navigateToArtistDetail(artistId, artistName);
};
return ( return (
<div id="stats-container" className={styles.statsContainer} data-testid="stats-page"> <div id="stats-container" className={styles.statsContainer} data-testid="stats-page">
<header className={styles.statsHeader}> <header className={styles.statsHeader}>
@ -229,25 +227,15 @@ export function StatsPage() {
</div> </div>
<div className={styles.statsRightCol}> <div className={styles.statsRightCol}>
<StatsSectionCard title="Top Artists"> <StatsSectionCard title="Top Artists">
<TopArtistsVisual <TopArtistsVisual artists={cachedStats?.top_artists ?? []} />
artists={cachedStats?.top_artists ?? []} <StatsRankedArtists artists={cachedStats?.top_artists ?? []} />
onArtistSelect={(artistId, artistName) => openArtistDetail(artistId, artistName)}
/>
<StatsRankedArtists
artists={cachedStats?.top_artists ?? []}
onArtistSelect={(artistId, artistName) => openArtistDetail(artistId, artistName)}
/>
</StatsSectionCard> </StatsSectionCard>
<StatsSectionCard title="Top Albums"> <StatsSectionCard title="Top Albums">
<StatsRankedAlbums <StatsRankedAlbums albums={cachedStats?.top_albums ?? []} />
albums={cachedStats?.top_albums ?? []}
onArtistSelect={(artistId, artistName) => openArtistDetail(artistId, artistName)}
/>
</StatsSectionCard> </StatsSectionCard>
<StatsSectionCard title="Top Tracks"> <StatsSectionCard title="Top Tracks">
<StatsRankedTracks <StatsRankedTracks
tracks={cachedStats?.top_tracks ?? []} tracks={cachedStats?.top_tracks ?? []}
onArtistSelect={(artistId, artistName) => openArtistDetail(artistId, artistName)}
onPlay={(track) => playStatsTrack(bridge, track)} onPlay={(track) => playStatsTrack(bridge, track)}
/> />
</StatsSectionCard> </StatsSectionCard>
@ -405,13 +393,7 @@ function StatsGenreLegend({
); );
} }
function TopArtistsVisual({ function TopArtistsVisual({ artists }: { artists: StatsArtistRow[] }) {
artists,
onArtistSelect,
}: {
artists: StatsArtistRow[];
onArtistSelect: (artistId: string | number, artistName: string) => void;
}) {
const topArtists = getTopArtistBubbles(artists); const topArtists = getTopArtistBubbles(artists);
if (topArtists.length === 0) return null; if (topArtists.length === 0) return null;
@ -420,18 +402,8 @@ function TopArtistsVisual({
<div className={styles.statsArtistBubbles}> <div className={styles.statsArtistBubbles}>
{topArtists.map(({ artist, percent, size }) => { {topArtists.map(({ artist, percent, size }) => {
const isClickable = artist.id !== null && artist.id !== undefined; const isClickable = artist.id !== null && artist.id !== undefined;
return ( const bubbleContent = (
<button <>
key={`${artist.name}-${artist.id ?? 'unknown'}`}
type="button"
className={styles.statsArtistBubble}
onClick={() => {
if (isClickable) {
onArtistSelect(artist.id as string | number, artist.name);
}
}}
disabled={!isClickable}
>
<div <div
className={styles.statsBubbleImage} className={styles.statsBubbleImage}
style={{ style={{
@ -451,7 +423,25 @@ function TopArtistsVisual({
<div className={styles.statsBubbleCount}> <div className={styles.statsBubbleCount}>
{formatCompactNumber(artist.play_count)} {formatCompactNumber(artist.play_count)}
</div> </div>
</button> </>
);
return isClickable ? (
<ArtistDetailLink
key={`${artist.name}-${artist.id ?? 'unknown'}`}
artistId={artist.id}
className={styles.statsArtistBubble}
aria-label={`Open artist detail for ${artist.name}`}
>
{bubbleContent}
</ArtistDetailLink>
) : (
<div
key={`${artist.name}-${artist.id ?? 'unknown'}`}
className={`${styles.statsArtistBubble} ${styles.statsArtistBubbleDisabled}`}
aria-disabled="true"
>
{bubbleContent}
</div>
); );
})} })}
</div> </div>
@ -459,13 +449,30 @@ function TopArtistsVisual({
); );
} }
function StatsRankedArtists({ function ArtistDetailLink({
artists, artistId,
onArtistSelect, children,
...linkProps
}: { }: {
artists: StatsArtistRow[]; artistId: string | number | null | undefined;
onArtistSelect: (artistId: string | number, artistName: string) => void; children: ReactNode;
}) { } & Omit<ComponentPropsWithoutRef<'a'>, 'children' | 'href'>) {
if (artistId == null) {
return <>{children}</>;
}
return (
<Link
to="/artist-detail/$source/$id"
params={{ source: ARTIST_DETAIL_SOURCE, id: String(artistId) }}
{...linkProps}
>
{children}
</Link>
);
}
function StatsRankedArtists({ artists }: { artists: StatsArtistRow[] }) {
return ( return (
<div id="stats-top-artists" className={styles.statsRankedList}> <div id="stats-top-artists" className={styles.statsRankedList}>
{artists.length === 0 ? <EmptyListState message="No data yet" /> : null} {artists.length === 0 ? <EmptyListState message="No data yet" /> : null}
@ -479,17 +486,9 @@ function StatsRankedArtists({
)} )}
<div className={styles.statsRankedInfo}> <div className={styles.statsRankedInfo}>
<div className={styles.statsRankedName}> <div className={styles.statsRankedName}>
{artist.id ? ( <ArtistDetailLink artistId={artist.id} className={styles.statsArtistLink}>
<button {artist.name}
type="button" </ArtistDetailLink>
className={styles.statsArtistLink}
onClick={() => onArtistSelect(artist.id as string | number, artist.name)}
>
{artist.name}
</button>
) : (
artist.name
)}
{artist.soul_id && !String(artist.soul_id).startsWith('soul_unnamed_') ? ( {artist.soul_id && !String(artist.soul_id).startsWith('soul_unnamed_') ? (
<img src="/static/trans2.png" className={styles.statsSoulIdBadge} alt="SoulID" /> <img src="/static/trans2.png" className={styles.statsSoulIdBadge} alt="SoulID" />
) : null} ) : null}
@ -509,13 +508,7 @@ function StatsRankedArtists({
); );
} }
function StatsRankedAlbums({ function StatsRankedAlbums({ albums }: { albums: StatsAlbumRow[] }) {
albums,
onArtistSelect,
}: {
albums: StatsAlbumRow[];
onArtistSelect: (artistId: string | number, artistName: string) => void;
}) {
return ( return (
<div id="stats-top-albums" className={styles.statsRankedList}> <div id="stats-top-albums" className={styles.statsRankedList}>
{albums.length === 0 ? <EmptyListState message="No data yet" /> : null} {albums.length === 0 ? <EmptyListState message="No data yet" /> : null}
@ -530,19 +523,9 @@ function StatsRankedAlbums({
<div className={styles.statsRankedInfo}> <div className={styles.statsRankedInfo}>
<div className={styles.statsRankedName}>{album.name}</div> <div className={styles.statsRankedName}>{album.name}</div>
<div className={styles.statsRankedMeta}> <div className={styles.statsRankedMeta}>
{album.artist_id ? ( <ArtistDetailLink artistId={album.artist_id} className={styles.statsArtistLink}>
<button {album.artist || ''}
type="button" </ArtistDetailLink>
className={styles.statsArtistLink}
onClick={() =>
onArtistSelect(album.artist_id as string | number, album.artist || '')
}
>
{album.artist || ''}
</button>
) : (
album.artist || ''
)}
</div> </div>
</div> </div>
<span className={styles.statsRankedCount}> <span className={styles.statsRankedCount}>
@ -556,11 +539,9 @@ function StatsRankedAlbums({
function StatsRankedTracks({ function StatsRankedTracks({
tracks, tracks,
onArtistSelect,
onPlay, onPlay,
}: { }: {
tracks: StatsTrackRow[]; tracks: StatsTrackRow[];
onArtistSelect: (artistId: string | number, artistName: string) => void;
onPlay: (track: { title: string; artist: string; album: string }) => Promise<void>; onPlay: (track: { title: string; artist: string; album: string }) => Promise<void>;
}) { }) {
return ( return (
@ -577,19 +558,9 @@ function StatsRankedTracks({
<div className={styles.statsRankedInfo}> <div className={styles.statsRankedInfo}>
<div className={styles.statsRankedName}>{track.name}</div> <div className={styles.statsRankedName}>{track.name}</div>
<div className={styles.statsRankedMeta}> <div className={styles.statsRankedMeta}>
{track.artist_id ? ( <ArtistDetailLink artistId={track.artist_id} className={styles.statsArtistLink}>
<button {track.artist || ''}
type="button" </ArtistDetailLink>
className={styles.statsArtistLink}
onClick={() =>
onArtistSelect(track.artist_id as string | number, track.artist || '')
}
>
{track.artist || ''}
</button>
) : (
track.artist || ''
)}
{track.album ? ` · ${track.album}` : ''} {track.album ? ` · ${track.album}` : ''}
</div> </div>
</div> </div>