feat: Support role filters for (album) artist role for Navidrome - #2120
feat: Support role filters for (album) artist role for Navidrome#2120kgarner7 wants to merge 1 commit into
Conversation
- Adds an optional (Navidrome only, currently) field `roles` to type `AlbumArtist` - `roles` is populated using artist.stats (excludign maincredit) - `getAlbumList` supports a role filter; if specified, only filter artists with that role (ND only) - Album list on artist page can filter by role if specified (only one) - Album query is no longer suspend, as it can change multiple times
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| {roles?.length && ( | ||
| <Select | ||
| aria-label="role" | ||
| clearable | ||
| data={roles} | ||
| onChange={setRole} | ||
| placeholder={t('common.role')} | ||
| value={role} | ||
| w={200} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
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.
| const albumsQuery = useQuery( | ||
| albumQueries.list({ | ||
| query: { | ||
| artistIds: [routeId], | ||
| limit: -1, | ||
| role: role || undefined, | ||
| sortBy: AlbumListSort.RELEASE_DATE, | ||
| sortOrder: SortOrder.DESC, | ||
| startIndex: 0, | ||
| }, | ||
| serverId, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
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:
- 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
- Lift up the search/filter row for the ArtistAlbums so that it is not affected by the new Suspense container
- 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
2 participants
rolesto typeAlbumArtistrolesis populated using artist.stats (excludign maincredit)getAlbumListsupports a role filter; if specified, only filter artists with that role (ND only)