refactor(webui): route artist-detail handoff
- add canonical /artist-detail/:source/:id TanStack route - hand the legacy page off through the shell bridge - remove artist-detail branching from generic shell helpers
This commit is contained in:
parent
19307630d1
commit
728481db31
14 changed files with 172 additions and 68 deletions
|
|
@ -60,6 +60,7 @@ function createShellBridge(overrides: Partial<ShellBridge> = {}): ShellBridge {
|
|||
resolveLegacyPath: vi.fn<(pathname: string) => ShellPageId | null>(() => 'search'),
|
||||
setActivePageChrome: vi.fn(),
|
||||
activateLegacyPath: vi.fn(),
|
||||
navigateToArtistDetail: vi.fn(),
|
||||
showReactHost: vi.fn(),
|
||||
...overrides,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -88,4 +88,13 @@ describe('bindWindowWebRouter', () => {
|
|||
replace: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('refuses artist detail navigation without an artist id', async () => {
|
||||
const navigate = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
bindWindowWebRouter({ navigate } as never);
|
||||
|
||||
await expect(window.SoulSyncWebRouter?.navigateToPage('artist-detail', {} as never)).resolves.toBe(false);
|
||||
expect(navigate).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -88,6 +88,9 @@ export function bindWindowWebRouter(router: AnyRouter) {
|
|||
async navigateToPage(pageId, options) {
|
||||
const route = getShellRouteByPageId(pageId);
|
||||
if (!route) return false;
|
||||
if (pageId === 'artist-detail' && !options?.artistId) {
|
||||
return false;
|
||||
}
|
||||
|
||||
let href: `/${string}` = route.path;
|
||||
if (pageId === 'artist-detail' && options?.artistId) {
|
||||
|
|
|
|||
9
webui/src/platform/shell/globals.d.ts
vendored
9
webui/src/platform/shell/globals.d.ts
vendored
|
|
@ -35,6 +35,15 @@ declare global {
|
|||
resolveLegacyPath: (pathname: string) => ShellPageId | null;
|
||||
setActivePageChrome: (pageId: ShellPageId) => void;
|
||||
activateLegacyPath: (pathname: string) => void;
|
||||
navigateToArtistDetail: (
|
||||
artistId: string | number,
|
||||
artistName: string,
|
||||
sourceOverride?: string | null,
|
||||
options?: {
|
||||
skipOriginPush?: boolean;
|
||||
skipRouteChange?: boolean;
|
||||
},
|
||||
) => void;
|
||||
showReactHost: (pageId: ShellPageId) => void;
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -16,7 +16,7 @@ describe('shellRouteManifest', () => {
|
|||
expect(resolveShellPageFromPath('/discover')).toBe('discover');
|
||||
expect(resolveShellPageFromPath('/watchlist')).toBe('watchlist');
|
||||
expect(resolveShellPageFromPath('/active-downloads')).toBe('active-downloads');
|
||||
expect(resolveShellPageFromPath('/artist-detail')).toBe('artist-detail');
|
||||
expect(resolveShellPageFromPath('/artist-detail')).toBeNull();
|
||||
expect(resolveShellPageFromPath('/artist-detail/spotify/2YZyLoL8N0Wb9xBt1NhZWg')).toBe('artist-detail');
|
||||
expect(resolveShellPageFromPath('/artists')).toBeNull();
|
||||
});
|
||||
|
|
@ -50,6 +50,7 @@ describe('shellRouteManifest', () => {
|
|||
expect(resolveLegacyShellPageFromPath('/search')).toBe('search');
|
||||
expect(resolveLegacyShellPageFromPath('/active-downloads')).toBe('active-downloads');
|
||||
expect(resolveLegacyShellPageFromPath('/tools')).toBe('tools');
|
||||
expect(resolveLegacyShellPageFromPath('/artist-detail')).toBeNull();
|
||||
expect(resolveLegacyShellPageFromPath('/artist-detail/deezer/12345')).toBe('artist-detail');
|
||||
expect(resolveLegacyShellPageFromPath('/issues')).toBeNull();
|
||||
expect(resolveLegacyShellPageFromPath('/does-not-exist')).toBeNull();
|
||||
|
|
|
|||
|
|
@ -72,7 +72,10 @@ export function getShellRouteByPath(pathname: string): ShellRouteDefinition | un
|
|||
|
||||
export function resolveShellPageFromPath(pathname: string): ShellPageId | null {
|
||||
const normalized = normalizeShellPath(pathname);
|
||||
if (normalized === '/artist-detail' || normalized.startsWith('/artist-detail/')) {
|
||||
if (normalized === '/artist-detail') {
|
||||
return null;
|
||||
}
|
||||
if (normalized.startsWith('/artist-detail/')) {
|
||||
return 'artist-detail';
|
||||
}
|
||||
return getShellRouteByPath(pathname)?.pageId ?? null;
|
||||
|
|
@ -80,7 +83,10 @@ export function resolveShellPageFromPath(pathname: string): ShellPageId | null {
|
|||
|
||||
export function resolveLegacyShellPageFromPath(pathname: string): ShellPageId | null {
|
||||
const normalized = normalizeShellPath(pathname);
|
||||
if (normalized === '/artist-detail' || normalized.startsWith('/artist-detail/')) {
|
||||
if (normalized === '/artist-detail') {
|
||||
return null;
|
||||
}
|
||||
if (normalized.startsWith('/artist-detail/')) {
|
||||
return 'artist-detail';
|
||||
}
|
||||
const route = getShellRouteByPath(pathname);
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ import { Route as rootRouteImport } from './routes/__root'
|
|||
import { Route as SplatRouteImport } from './routes/$'
|
||||
import { Route as IssuesRouteRouteImport } from './routes/issues/route'
|
||||
import { Route as IndexRouteImport } from './routes/index'
|
||||
import { Route as ArtistDetailSourceIdRouteImport } from './routes/artist-detail/$source/$id'
|
||||
|
||||
const SplatRoute = SplatRouteImport.update({
|
||||
id: '/$',
|
||||
|
|
@ -28,35 +29,44 @@ const IndexRoute = IndexRouteImport.update({
|
|||
path: '/',
|
||||
getParentRoute: () => rootRouteImport,
|
||||
} as any)
|
||||
const ArtistDetailSourceIdRoute = ArtistDetailSourceIdRouteImport.update({
|
||||
id: '/artist-detail/$source/$id',
|
||||
path: '/artist-detail/$source/$id',
|
||||
getParentRoute: () => rootRouteImport,
|
||||
} as any)
|
||||
|
||||
export interface FileRoutesByFullPath {
|
||||
'/': typeof IndexRoute
|
||||
'/issues': typeof IssuesRouteRoute
|
||||
'/$': typeof SplatRoute
|
||||
'/artist-detail/$source/$id': typeof ArtistDetailSourceIdRoute
|
||||
}
|
||||
export interface FileRoutesByTo {
|
||||
'/': typeof IndexRoute
|
||||
'/issues': typeof IssuesRouteRoute
|
||||
'/$': typeof SplatRoute
|
||||
'/artist-detail/$source/$id': typeof ArtistDetailSourceIdRoute
|
||||
}
|
||||
export interface FileRoutesById {
|
||||
__root__: typeof rootRouteImport
|
||||
'/': typeof IndexRoute
|
||||
'/issues': typeof IssuesRouteRoute
|
||||
'/$': typeof SplatRoute
|
||||
'/artist-detail/$source/$id': typeof ArtistDetailSourceIdRoute
|
||||
}
|
||||
export interface FileRouteTypes {
|
||||
fileRoutesByFullPath: FileRoutesByFullPath
|
||||
fullPaths: '/' | '/issues' | '/$'
|
||||
fullPaths: '/' | '/issues' | '/$' | '/artist-detail/$source/$id'
|
||||
fileRoutesByTo: FileRoutesByTo
|
||||
to: '/' | '/issues' | '/$'
|
||||
id: '__root__' | '/' | '/issues' | '/$'
|
||||
to: '/' | '/issues' | '/$' | '/artist-detail/$source/$id'
|
||||
id: '__root__' | '/' | '/issues' | '/$' | '/artist-detail/$source/$id'
|
||||
fileRoutesById: FileRoutesById
|
||||
}
|
||||
export interface RootRouteChildren {
|
||||
IndexRoute: typeof IndexRoute
|
||||
IssuesRouteRoute: typeof IssuesRouteRoute
|
||||
SplatRoute: typeof SplatRoute
|
||||
ArtistDetailSourceIdRoute: typeof ArtistDetailSourceIdRoute
|
||||
}
|
||||
|
||||
declare module '@tanstack/react-router' {
|
||||
|
|
@ -82,6 +92,13 @@ declare module '@tanstack/react-router' {
|
|||
preLoaderRoute: typeof IndexRouteImport
|
||||
parentRoute: typeof rootRouteImport
|
||||
}
|
||||
'/artist-detail/$source/$id': {
|
||||
id: '/artist-detail/$source/$id'
|
||||
path: '/artist-detail/$source/$id'
|
||||
fullPath: '/artist-detail/$source/$id'
|
||||
preLoaderRoute: typeof ArtistDetailSourceIdRouteImport
|
||||
parentRoute: typeof rootRouteImport
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -89,6 +106,7 @@ const rootRouteChildren: RootRouteChildren = {
|
|||
IndexRoute: IndexRoute,
|
||||
IssuesRouteRoute: IssuesRouteRoute,
|
||||
SplatRoute: SplatRoute,
|
||||
ArtistDetailSourceIdRoute: ArtistDetailSourceIdRoute,
|
||||
}
|
||||
export const routeTree = rootRouteImport
|
||||
._addFileChildren(rootRouteChildren)
|
||||
|
|
|
|||
27
webui/src/routes/artist-detail/$source/$id.tsx
Normal file
27
webui/src/routes/artist-detail/$source/$id.tsx
Normal file
|
|
@ -0,0 +1,27 @@
|
|||
import { createFileRoute } from '@tanstack/react-router';
|
||||
import { useLayoutEffect } from 'react';
|
||||
|
||||
import { useShellBridge } from '@/platform/shell/route-controllers';
|
||||
|
||||
export const Route = createFileRoute('/artist-detail/$source/$id')({
|
||||
component: ArtistDetailPage,
|
||||
});
|
||||
|
||||
// Thin legacy handoff: TanStack owns the URL shape here, but the vanilla JS
|
||||
// artist-detail page still renders the actual experience for now.
|
||||
function ArtistDetailPage() {
|
||||
const bridge = useShellBridge();
|
||||
const { source, id } = Route.useParams();
|
||||
|
||||
useLayoutEffect(() => {
|
||||
if (!bridge) return;
|
||||
|
||||
const normalizedSource = source.toLowerCase() === 'library' ? null : source.toLowerCase();
|
||||
bridge.navigateToArtistDetail(id, '', normalizedSource, {
|
||||
skipOriginPush: true,
|
||||
skipRouteChange: true,
|
||||
});
|
||||
}, [bridge, id, source]);
|
||||
|
||||
return null;
|
||||
}
|
||||
76
webui/src/routes/artist-detail/-route.test.tsx
Normal file
76
webui/src/routes/artist-detail/-route.test.tsx
Normal file
|
|
@ -0,0 +1,76 @@
|
|||
import { createMemoryHistory } from '@tanstack/react-router';
|
||||
import { render, waitFor } from '@testing-library/react';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import type { ShellBridge, ShellPageId } from '@/platform/shell/bridge';
|
||||
|
||||
import { createAppQueryClient } from '@/app/query-client';
|
||||
import { AppRouterProvider, createAppRouter } from '@/app/router';
|
||||
|
||||
function createShellBridge(overrides: Partial<ShellBridge> = {}): ShellBridge {
|
||||
return {
|
||||
getCurrentProfileContext: vi.fn(() => ({ profileId: 2, isAdmin: false })),
|
||||
isPageAllowed: vi.fn(() => true),
|
||||
getProfileHomePage: vi.fn<() => ShellPageId>(() => 'discover'),
|
||||
resolveLegacyPath: vi.fn<(pathname: string) => ShellPageId | null>(() => 'artist-detail'),
|
||||
setActivePageChrome: vi.fn(),
|
||||
activateLegacyPath: vi.fn(),
|
||||
navigateToArtistDetail: vi.fn(),
|
||||
showReactHost: vi.fn(),
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function renderArtistDetailRoute(initialEntries = ['/artist-detail/library/42']) {
|
||||
const queryClient = createAppQueryClient();
|
||||
const history = createMemoryHistory({ initialEntries });
|
||||
const router = createAppRouter({ history, queryClient });
|
||||
|
||||
return {
|
||||
history,
|
||||
router,
|
||||
...render(<AppRouterProvider router={router} queryClient={queryClient} />),
|
||||
};
|
||||
}
|
||||
|
||||
describe('artist-detail route', () => {
|
||||
beforeEach(() => {
|
||||
window.SoulSyncWebShellBridge = createShellBridge();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
window.SoulSyncWebShellBridge = undefined;
|
||||
});
|
||||
|
||||
it('hands off canonical artist-detail URLs to the legacy shell', async () => {
|
||||
renderArtistDetailRoute(['/artist-detail/spotify/2YZyLoL8N0Wb9xBt1NhZWg']);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(window.SoulSyncWebShellBridge?.navigateToArtistDetail).toHaveBeenCalledWith(
|
||||
'2YZyLoL8N0Wb9xBt1NhZWg',
|
||||
'',
|
||||
'spotify',
|
||||
{
|
||||
skipOriginPush: true,
|
||||
skipRouteChange: true,
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
it('normalizes library sources before handing off', async () => {
|
||||
renderArtistDetailRoute(['/artist-detail/library/42']);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(window.SoulSyncWebShellBridge?.navigateToArtistDetail).toHaveBeenCalledWith(
|
||||
'42',
|
||||
'',
|
||||
null,
|
||||
{
|
||||
skipOriginPush: true,
|
||||
skipRouteChange: true,
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -22,6 +22,7 @@ function createShellBridge(overrides: Partial<ShellBridge> = {}): ShellBridge {
|
|||
resolveLegacyPath: vi.fn<(pathname: string) => ShellPageId | null>(() => 'search'),
|
||||
setActivePageChrome: vi.fn(),
|
||||
activateLegacyPath: vi.fn(),
|
||||
navigateToArtistDetail: vi.fn(),
|
||||
showReactHost: vi.fn(),
|
||||
...overrides,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -2154,10 +2154,6 @@ function _getPageFromPath() {
|
|||
const basePage = segs[0];
|
||||
if (!_DEEPLINK_VALID_PAGES.has(basePage)) return 'dashboard';
|
||||
// Context-dependent pages fall back to a sensible parent
|
||||
if (basePage === 'artist-detail') {
|
||||
// /artist-detail/:id deep-link — keep on artist-detail; bare /artist-detail falls back to library
|
||||
return (segs.length >= 2 && segs[1]) ? 'artist-detail' : 'library';
|
||||
}
|
||||
if (basePage === 'playlist-explorer') return 'library';
|
||||
return basePage;
|
||||
}
|
||||
|
|
@ -2168,35 +2164,13 @@ function _normalizeArtistDetailSource(source) {
|
|||
}
|
||||
|
||||
function buildArtistDetailPath(artistId, source = null) {
|
||||
if (!artistId) return '/artist-detail';
|
||||
if (!artistId) {
|
||||
throw new Error('artistId is required for artist-detail navigation');
|
||||
}
|
||||
const normalizedSource = _normalizeArtistDetailSource(source);
|
||||
return '/artist-detail/' + encodeURIComponent(normalizedSource) + '/' + encodeURIComponent(String(artistId));
|
||||
}
|
||||
|
||||
/** Extract artist source + ID from /artist-detail/:source/:id or legacy /artist-detail/:id URLs. */
|
||||
function _getDeepLinkArtistDetail(pathname = window.location.pathname) {
|
||||
const path = (pathname || '').replace(/^\/+|\/+$/g, '');
|
||||
const segs = path.split('/');
|
||||
if (segs[0] !== 'artist-detail' || !segs[1]) return null;
|
||||
|
||||
if (segs[2]) {
|
||||
return {
|
||||
source: _normalizeArtistDetailSource(decodeURIComponent(segs[1])),
|
||||
artistId: decodeURIComponent(segs.slice(2).join('/')),
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
source: 'library',
|
||||
artistId: decodeURIComponent(segs[1]),
|
||||
};
|
||||
}
|
||||
|
||||
/** Legacy convenience wrapper for callers that only need the artist ID. */
|
||||
function _getDeepLinkArtistId() {
|
||||
return _getDeepLinkArtistDetail()?.artistId || null;
|
||||
}
|
||||
|
||||
// ===============================
|
||||
// MOBILE NAVIGATION
|
||||
// ===============================
|
||||
|
|
@ -2310,6 +2284,10 @@ function navigateToPage(pageId, options = {}) {
|
|||
return;
|
||||
}
|
||||
|
||||
if (pageId === 'artist-detail' && !options.artistId) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const router = getWebRouter();
|
||||
if (router && !options.skipRouteChange) {
|
||||
notifyPageWillChange(pageId);
|
||||
|
|
@ -2386,7 +2364,10 @@ async function loadPageData(pageId) {
|
|||
case 'library':
|
||||
// Check if we should return to artist detail view instead of list
|
||||
if (artistDetailPageState.currentArtistId && artistDetailPageState.currentArtistName) {
|
||||
navigateToPage('artist-detail');
|
||||
navigateToPage('artist-detail', {
|
||||
artistId: artistDetailPageState.currentArtistId,
|
||||
artistSource: artistDetailPageState.currentArtistSource,
|
||||
});
|
||||
if (!artistDetailPageState.isInitialized) {
|
||||
initializeArtistDetailPage();
|
||||
loadArtistDetailData(artistDetailPageState.currentArtistId, artistDetailPageState.currentArtistName);
|
||||
|
|
@ -2400,7 +2381,7 @@ async function loadPageData(pageId) {
|
|||
}
|
||||
break;
|
||||
case 'artist-detail':
|
||||
// Artist detail page is handled separately by navigateToArtistDetail()
|
||||
// Artist detail page is entered through the route handoff and legacy navigator.
|
||||
break;
|
||||
case 'discover':
|
||||
if (!discoverPageInitialized) {
|
||||
|
|
|
|||
|
|
@ -815,7 +815,7 @@ function navigateToArtistDetail(artistId, artistName, sourceOverride = null, opt
|
|||
navigateToPage('artist-detail', {
|
||||
artistId,
|
||||
artistSource: normalizedSource,
|
||||
skipRouteChange: true
|
||||
skipRouteChange: options.skipRouteChange === true
|
||||
});
|
||||
_updateArtistDetailBackButtonLabel();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1197,16 +1197,7 @@ async function loadInitialData() {
|
|||
return;
|
||||
}
|
||||
|
||||
if (targetPage === 'artist-detail') {
|
||||
const deepArtist = _getDeepLinkArtistDetail();
|
||||
if (deepArtist?.artistId && typeof navigateToArtistDetail === 'function') {
|
||||
navigateToArtistDetail(deepArtist.artistId, '', deepArtist.source === 'library' ? null : deepArtist.source);
|
||||
} else {
|
||||
navigateToPage('library', { skipRouteChange: true, forceReload: true });
|
||||
}
|
||||
} else {
|
||||
navigateToPage(targetPage, { skipRouteChange: true, forceReload: true });
|
||||
}
|
||||
navigateToPage(targetPage, { forceReload: true });
|
||||
} catch (error) {
|
||||
console.error('Error loading initial data:', error);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -80,16 +80,6 @@ function activateLegacyPath(pathname) {
|
|||
return;
|
||||
}
|
||||
|
||||
if (targetPage === 'artist-detail') {
|
||||
const deepArtist = _getDeepLinkArtistDetail(pathname);
|
||||
if (deepArtist?.artistId && typeof navigateToArtistDetail === 'function') {
|
||||
navigateToArtistDetail(deepArtist.artistId, '', deepArtist.source === 'library' ? null : deepArtist.source, { skipOriginPush: true, skipRouteChange: true });
|
||||
return;
|
||||
}
|
||||
navigateToPage('library', { replace: true });
|
||||
return;
|
||||
}
|
||||
|
||||
notifyPageWillChange(targetPage);
|
||||
activatePage(targetPage, { forceReload: true });
|
||||
}
|
||||
|
|
@ -107,16 +97,6 @@ function syncActivePageFromLocation() {
|
|||
return;
|
||||
}
|
||||
|
||||
if (targetPage === 'artist-detail') {
|
||||
const deepArtist = _getDeepLinkArtistDetail();
|
||||
if (deepArtist?.artistId && typeof navigateToArtistDetail === 'function') {
|
||||
navigateToArtistDetail(deepArtist.artistId, '', deepArtist.source === 'library' ? null : deepArtist.source, { skipOriginPush: true, skipRouteChange: true });
|
||||
return;
|
||||
}
|
||||
navigateToPage('library', { replace: true });
|
||||
return;
|
||||
}
|
||||
|
||||
notifyPageWillChange(targetPage);
|
||||
const route = router?.routeManifest?.find((entry) => entry.pageId === targetPage);
|
||||
if (route?.kind === 'react') {
|
||||
|
|
@ -185,6 +165,7 @@ window.SoulSyncWebShellBridge = {
|
|||
activateLegacyPath(pathname) {
|
||||
activateLegacyPath(pathname);
|
||||
},
|
||||
navigateToArtistDetail,
|
||||
showReactHost(pageId) {
|
||||
showReactHost(pageId);
|
||||
},
|
||||
|
|
|
|||
Loading…
Reference in a new issue