Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/i18n/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@
"resetToDefault": "Reset to default",
"restartRequired": "Restart required",
"right": "Right",
"role": "Role",
"sampleRate": "Sample rate",
"save": "Save",
"saveAndReplace": "Save and replace",
Expand Down
4 changes: 3 additions & 1 deletion src/renderer/api/navidrome/navidrome-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -428,16 +428,18 @@ export const NavidromeController: InternalControllerEndpoint = {
? query.artistIds
: query.artistIds?.[0];

const key = query.role ? `role_${query.role}_id` : 'artist_id';

const res = await ndApiClient(apiClientProps).getAlbumList({
query: {
_end: query.startIndex + (query.limit || 0),
_order: sortOrderMap.navidrome[query.sortOrder],
_sort: albumListSortMap.navidrome[query.sortBy],
_start: query.startIndex,
artist_id: artistIds,
compilation: query.compilation,
genre_id: genres,
has_rating: query.hasRating,
[key]: artistIds,
library_id: getLibraryId(query.musicFolderId),
name: query.searchTerm,
recently_played: query.isRecentlyPlayed,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
useQuery,
useQueryClient,
UseQueryResult,
useSuspenseQuery,
UseSuspenseQueryResult,
} from '@tanstack/react-query';
Expand Down Expand Up @@ -1061,6 +1062,7 @@ const AlbumArtistMetadataSimilarArtists = ({
mbz: null,
name: relatedArtist.name,
playCount: null,
roles: null,
similarArtists: null,
songCount: null,
userFavorite: relatedArtist.userFavorite,
Expand Down Expand Up @@ -1101,13 +1103,17 @@ const AlbumArtistMetadataSimilarArtists = ({
};

interface AlbumArtistDetailContentProps {
albumsQuery: UseSuspenseQueryResult<AlbumListResponse, Error>;
albumsQuery: UseQueryResult<AlbumListResponse, Error>;
detailQuery: UseSuspenseQueryResult<AlbumArtistDetailResponse, Error>;
role: null | string;
setRole: (role: null | string) => void;
}

export const AlbumArtistDetailContent = ({
albumsQuery,
detailQuery,
role,
setRole,
}: AlbumArtistDetailContentProps) => {
const artistItems = useArtistItems();
const artistRadioCount = useArtistRadioCount();
Expand Down Expand Up @@ -1220,7 +1226,13 @@ export const AlbumArtistDetailContent = ({
routeId={routeId}
/>
)}
<ArtistAlbums albumsQuery={albumsQuery} order={itemOrder.recentAlbums} />
<ArtistAlbums
albumsQuery={albumsQuery}
order={itemOrder.recentAlbums}
role={role}
roles={detailQuery.data?.roles}
setRole={setRole}
/>
{enabledItem.similarArtists && (
<AlbumArtistMetadataSimilarArtists
order={itemOrder.similarArtists}
Expand Down Expand Up @@ -1420,13 +1432,17 @@ const AlbumSection = memo(function AlbumSection({
});

import { useArtistAlbumsGrouped } from '/@/renderer/features/artists/hooks/use-artist-albums-grouped';
import { Select } from '/@/shared/components/select/select';

interface ArtistAlbumsProps {
albumsQuery: UseSuspenseQueryResult<AlbumListResponse, Error>;
albumsQuery: UseQueryResult<AlbumListResponse, Error>;
order?: number;
role: null | string;
roles?: null | string[];
setRole: (role: null | string) => void;
}

const ArtistAlbums = ({ albumsQuery, order }: ArtistAlbumsProps) => {
const ArtistAlbums = ({ albumsQuery, order, role, roles, setRole }: ArtistAlbumsProps) => {
const { t } = useTranslation();
const [searchTerm, setSearchTerm] = useState('');
const [debouncedSearchTerm] = useDebouncedValue(searchTerm, 300);
Expand Down Expand Up @@ -1521,6 +1537,17 @@ const ArtistAlbums = ({ albumsQuery, order }: ArtistAlbumsProps) => {
}}
value={searchTerm}
/>
{roles?.length && (
<Select
aria-label="role"
clearable
data={roles}
onChange={setRole}
placeholder={t('common.role')}
value={role}
w={200}
/>
)}
Comment on lines +1540 to +1550

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Select input clashes with the rest of the design on the page.

It would be better to incorporate this into the existing settings dropdown with the toggle for "all release types" or "primary release types". It should probably use the Submenus on the DropdownMenu component to begin separating these out.

<ListSortByDropdownControlled
filters={CLIENT_SIDE_ALBUM_FILTERS}
itemType={LibraryItem.ALBUM}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useSuspenseQuery, UseSuspenseQueryResult } from '@tanstack/react-query';
import { UseQueryResult, useSuspenseQuery } from '@tanstack/react-query';
import { forwardRef, Fragment, useCallback } from 'react';
import { useTranslation } from 'react-i18next';
import { useParams } from 'react-router';
Expand Down Expand Up @@ -38,7 +38,7 @@ import { ServerFeature } from '/@/shared/types/features-types';
import { Play } from '/@/shared/types/types';

interface AlbumArtistDetailHeaderProps {
albumsQuery: UseSuspenseQueryResult<AlbumListResponse, Error>;
albumsQuery: UseQueryResult<AlbumListResponse, Error>;
}

function ArtistImageUploadOverlay({
Expand Down
43 changes: 25 additions & 18 deletions src/renderer/features/artists/routes/album-artist-detail-route.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useSuspenseQueries } from '@tanstack/react-query';
import { Suspense, useRef } from 'react';
import { useQuery, useSuspenseQuery } from '@tanstack/react-query';
import { Suspense, useRef, useState } from 'react';
import { useParams } from 'react-router';

import { useItemImageUrl } from '/@/renderer/components/item-image/item-image';
Expand Down Expand Up @@ -34,22 +34,24 @@ const AlbumArtistDetailRouteContent = () => {
};

const routeId = (artistId || albumArtistId) as string;
const [role, setRole] = useState<null | string>(null);

const [detailQuery, albumsQuery] = useSuspenseQueries({
queries: [
artistsQueries.albumArtistDetail({ query: { id: routeId }, serverId: server?.id }),
albumQueries.list({
query: {
artistIds: [routeId],
limit: -1,
sortBy: AlbumListSort.RELEASE_DATE,
sortOrder: SortOrder.DESC,
startIndex: 0,
},
serverId,
}),
],
});
const detailQuery = useSuspenseQuery(
artistsQueries.albumArtistDetail({ query: { id: routeId }, serverId: server?.id }),
);
const albumsQuery = useQuery(
albumQueries.list({
query: {
artistIds: [routeId],
limit: -1,
role: role || undefined,
sortBy: AlbumListSort.RELEASE_DATE,
sortOrder: SortOrder.DESC,
startIndex: 0,
},
serverId,
}),
);
Comment on lines +42 to +54

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is no longer a suspense query, you need to properly handle cases when data is loading.

The handlePlay in the header will just silently fail if you try to click the play buttons while the album query is loading. Similarly, there is no loading UI for the album list. So if the request takes a while, the user might think that it is bugged if no albums are showing up.

Instead, you may just want to reorganize the page composition so that you're no longer fetching the list endpoint in the root component, and instead:

  1. Revert to suspense query and localize the albumsQuery directly in the ArtistAlbums and wrap it in a Suspense container. No longer prop drill the albumsQuery down to header/content components
  2. Lift up the search/filter row for the ArtistAlbums so that it is not affected by the new Suspense container
  3. Pass down the role state to the header (or better add it as a route param so that we can just use useParams to reference it). In the header, just use queryClient.ensureQueryData or similar so that we can fetch from the cache or refetch if not available for the play instead of just silently failing


const imageUrl = useItemImageUrl({
id: detailQuery.data?.imageId || undefined,
Expand Down Expand Up @@ -117,7 +119,12 @@ const AlbumArtistDetailRouteContent = () => {
albumsQuery={albumsQuery}
ref={headerRef as React.Ref<HTMLDivElement>}
/>
<AlbumArtistDetailContent albumsQuery={albumsQuery} detailQuery={detailQuery} />
<AlbumArtistDetailContent
albumsQuery={albumsQuery}
detailQuery={detailQuery}
role={role}
setRole={setRole}
/>
</LibraryContainer>
</NativeScrollArea>
</AnimatedPage>
Expand Down
1 change: 1 addition & 0 deletions src/shared/api/jellyfin/jellyfin-normalize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,7 @@ const normalizeAlbumArtist = (
mbz: item.ProviderIds?.MusicBrainzArtist || null,
name: item.Name,
playCount: item.UserData?.PlayCount || 0,
roles: null,
similarArtists,
songCount: item.SongCount ?? null,
uploadedImage: item.ImageTags?.Primary ?? undefined,
Expand Down
4 changes: 4 additions & 0 deletions src/shared/api/navidrome/navidrome-normalize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -410,10 +410,12 @@ const normalizeAlbumArtist = (

if (item.stats) {
albumCount = Math.max(
item.stats.maincredit?.albumCount ?? 0,
item.stats.albumartist?.albumCount ?? 0,
item.stats.artist?.albumCount ?? 0,
);
songCount = Math.max(
item.stats.maincredit?.songCount ?? 0,
item.stats.albumartist?.songCount ?? 0,
item.stats.artist?.songCount ?? 0,
);
Expand Down Expand Up @@ -453,6 +455,8 @@ const normalizeAlbumArtist = (
mbz: item.mbzArtistId || null,
name: item.name,
playCount: item.playCount || 0,
// filter out specifically maincredit. This is not filterable properly
roles: item.stats ? Object.keys(item.stats).filter((key) => key !== 'maincredit') : null,
similarArtists:
item.similarArtists?.map((artist) => ({
id: artist.id,
Expand Down
1 change: 1 addition & 0 deletions src/shared/api/subsonic/subsonic-normalize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ const normalizeAlbumArtist = (
mbz: null,
name: item.name,
playCount: null,
roles: null,
similarArtists:
item.similarArtists?.map((artist) => ({
id: artist.id,
Expand Down
2 changes: 2 additions & 0 deletions src/shared/types/domain-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ export type AlbumArtist = {
mbz: null | string;
name: string;
playCount: null | number;
roles: null | string[];
similarArtists: null | RelatedArtist[];
songCount: null | number;
uploadedImage?: string;
Expand Down Expand Up @@ -500,6 +501,7 @@ export interface AlbumListQuery extends AlbumListNavidromeQuery, BaseQuery<Album
maxYear?: number;
minYear?: number;
musicFolderId?: string | string[];
role?: string;
searchTerm?: string;
startIndex: number;
}
Expand Down
Loading