Address self-review checklist findings

- Remove dead export useMusicTracksByGenre from useMusicData
- Sanitize metadata?.picture through sanitizeUrl() in ProfileCard,
  MusicHeroCard, MusicTrackCard, and MusicTrackRow
- Fix MusicDiscoverTab featured section where skeleton and loaded
  content could render simultaneously (make mutually exclusive)
- Add error state handling to all 4 music tab components
- Move genre filtering from client-side to relay-level #t filtering
  in MusicTracksTab via new genre param on useMusicFeed
This commit is contained in:
Mary Kate Fain
2026-04-17 19:09:08 -05:00
parent 61d3c261fe
commit 800e0bbe47
10 changed files with 59 additions and 74 deletions
+2 -1
View File
@@ -5,6 +5,7 @@ import { getAvatarShape } from '@/lib/avatarShape';
import { useAuthor } from '@/hooks/useAuthor';
import { useProfileUrl } from '@/hooks/useProfileUrl';
import { getDisplayName } from '@/lib/getDisplayName';
import { sanitizeUrl } from '@/lib/sanitizeUrl';
import { cn } from '@/lib/utils';
interface ProfileCardProps {
@@ -38,7 +39,7 @@ export function ProfileCard({ pubkey, subtitle, className }: ProfileCardProps) {
className={cn('w-[110px] shrink-0 flex flex-col items-center cursor-pointer group', className)}
>
<Avatar shape={avatarShape} className="size-20 border-2 border-border group-hover:border-primary/40 transition-colors">
<AvatarImage src={metadata?.picture} alt={displayName} />
<AvatarImage src={sanitizeUrl(metadata?.picture)} alt={displayName} />
<AvatarFallback className="bg-primary/20 text-primary text-lg font-semibold">
{displayName[0]?.toUpperCase()}
</AvatarFallback>
+9 -1
View File
@@ -16,7 +16,7 @@ import { ProfileCard, ProfileCardSkeleton } from '@/components/discovery/Profile
*/
export function MusicArtistsTab() {
const { data: curatedPubkeys } = useCuratedMusicArtists();
const { artists, isLoading } = useMusicData();
const { artists, isLoading, isError } = useMusicData();
// Merge curated artists (first) with discovered artists
const allArtists = useMemo(() => {
@@ -55,6 +55,14 @@ export function MusicArtistsTab() {
);
}
if (isError) {
return (
<p className="px-4 py-12 text-sm text-muted-foreground text-center">
Failed to load artists. Check your relay connections and try again.
</p>
);
}
if (allArtists.length === 0) {
return (
<p className="px-4 py-12 text-sm text-muted-foreground text-center">
+17 -16
View File
@@ -84,7 +84,7 @@ export function MusicDiscoverTab({ onSwitchToTracks, onSwitchToPlaylists, onSwit
} = useMusicData({ authors: curatedPubkeys });
// New Tracks: sorted query for hot/top via Ditto relay, or chronological for new
const { data: sortedNewTracks, isLoading: isSortedLoading } = useQuery<NostrEvent[]>({
const { data: sortedNewTracks, isLoading: isSortedLoading, isError: isSortedError } = useQuery<NostrEvent[]>({
queryKey: ['discover-new-tracks', newTracksSort, newTracksScope, newTracksAuthors?.slice().sort().join(',') ?? '', selectedGenre ?? ''],
queryFn: async ({ signal }) => {
if (!newTracksAuthors || newTracksAuthors.length === 0) return [];
@@ -177,19 +177,7 @@ export function MusicDiscoverTab({ onSwitchToTracks, onSwitchToPlaylists, onSwit
) : null}
{/* Featured tracks horizontal scroll (hot, one per artist) */}
{featuredTracks && featuredTracks.length > 1 && (
<>
<SectionHeader title="Featured" onSeeAll={onSwitchToTracks} />
<HorizontalScroll>
{featuredTracks.slice(1, 8).map((ev) => (
<MusicTrackCard key={ev.id} event={ev} />
))}
</HorizontalScroll>
</>
)}
{/* Loading state for featured */}
{isFeaturedLoading && (
{isFeaturedLoading ? (
<>
<SectionHeader title="Featured" />
<HorizontalScroll>
@@ -198,7 +186,16 @@ export function MusicDiscoverTab({ onSwitchToTracks, onSwitchToPlaylists, onSwit
))}
</HorizontalScroll>
</>
)}
) : featuredTracks && featuredTracks.length > 1 ? (
<>
<SectionHeader title="Featured" onSeeAll={onSwitchToTracks} />
<HorizontalScroll>
{featuredTracks.slice(1, 8).map((ev) => (
<MusicTrackCard key={ev.id} event={ev} />
))}
</HorizontalScroll>
</>
) : null}
{/* Artists horizontal scroll */}
{(isTracksLoading || featuredArtists.length > 0) && (
@@ -262,7 +259,11 @@ export function MusicDiscoverTab({ onSwitchToTracks, onSwitchToPlaylists, onSwit
onSelect={setSelectedGenre}
/>
)}
{isNewTracksLoading ? (
{isSortedError ? (
<p className="px-4 py-6 text-sm text-muted-foreground text-center">
Failed to load tracks. Check your relay connections and try again.
</p>
) : isNewTracksLoading ? (
<div>
{Array.from({ length: 5 }).map((_, i) => (
<MusicTrackRowSkeleton key={i} />
+2 -1
View File
@@ -4,6 +4,7 @@ import type { NostrEvent } from '@nostrify/nostrify';
import { useAudioPlayer } from '@/contexts/audioPlayerContextDef';
import { useAuthor } from '@/hooks/useAuthor';
import { parseMusicTrack, toAudioTrack } from '@/lib/musicHelpers';
import { sanitizeUrl } from '@/lib/sanitizeUrl';
import { formatTime } from '@/lib/formatTime';
import { Skeleton } from '@/components/ui/skeleton';
import { cn } from '@/lib/utils';
@@ -44,7 +45,7 @@ export function MusicHeroCard({ event }: MusicHeroCardProps) {
player.resume();
} else {
const track = toAudioTrack(event, parsed);
track.artwork ??= author.data?.metadata?.picture;
track.artwork ??= sanitizeUrl(author.data?.metadata?.picture);
player.playTrack(track);
}
};
+6 -1
View File
@@ -28,6 +28,7 @@ export function MusicPlaylistsTab() {
data: rawData,
isPending,
isLoading,
isError,
fetchNextPage,
hasNextPage,
isFetchingNextPage,
@@ -113,7 +114,11 @@ export function MusicPlaylistsTab() {
)}
{/* Playlist grid */}
{showSkeleton ? (
{isError ? (
<p className="px-4 py-12 text-sm text-muted-foreground text-center">
Failed to load playlists. Check your relay connections and try again.
</p>
) : showSkeleton ? (
<div className="px-4 pt-2">
<div className="grid grid-cols-2 md:grid-cols-3 gap-3">
{Array.from({ length: 6 }).map((_, i) => (
+2 -1
View File
@@ -6,6 +6,7 @@ import type { NostrEvent } from '@nostrify/nostrify';
import { useAudioPlayer } from '@/contexts/audioPlayerContextDef';
import { useAuthor } from '@/hooks/useAuthor';
import { parseMusicTrack, toAudioTrack } from '@/lib/musicHelpers';
import { sanitizeUrl } from '@/lib/sanitizeUrl';
import { Skeleton } from '@/components/ui/skeleton';
import { cn } from '@/lib/utils';
@@ -50,7 +51,7 @@ export function MusicTrackCard({ event }: MusicTrackCardProps) {
player.resume();
} else {
const track = toAudioTrack(event, parsed);
track.artwork ??= author.data?.metadata?.picture;
track.artwork ??= sanitizeUrl(author.data?.metadata?.picture);
player.playTrack(track);
}
};
+2 -1
View File
@@ -6,6 +6,7 @@ import type { NostrEvent } from '@nostrify/nostrify';
import { useAudioPlayer } from '@/contexts/audioPlayerContextDef';
import { useAuthor } from '@/hooks/useAuthor';
import { parseMusicTrack, toAudioTrack } from '@/lib/musicHelpers';
import { sanitizeUrl } from '@/lib/sanitizeUrl';
import { formatTime } from '@/lib/formatTime';
import { Skeleton } from '@/components/ui/skeleton';
import { cn } from '@/lib/utils';
@@ -53,7 +54,7 @@ export function MusicTrackRow({ event, index }: MusicTrackRowProps) {
player.resume();
} else {
const track = toAudioTrack(event, parsed);
track.artwork ??= author.data?.metadata?.picture;
track.artwork ??= sanitizeUrl(author.data?.metadata?.picture);
player.playTrack(track);
}
};
+10 -12
View File
@@ -17,7 +17,7 @@ import { MusicTrackRow, MusicTrackRowSkeleton } from './MusicTrackRow';
* Features:
* - **Sort**: Hot (engagement + decay), Top (total engagement), New (chronological)
* - **Scope**: Global (all artists) or Following (user's follow list)
* - **Genre filter**: Client-side genre filtering via TagChips
* - **Genre filter**: Relay-level `#t` tag filtering via TagChips
*/
export function MusicTracksTab() {
const [selectedGenre, setSelectedGenre] = useState<string | null>(null);
@@ -29,12 +29,13 @@ export function MusicTracksTab() {
const { genres } = useMusicData();
const genreNames = useMemo(() => genres.slice(0, 12).map((g) => g.genre), [genres]);
// Infinite-scroll feed with sort + scope
const feedQuery = useMusicFeed({ kind: 36787, sort, scope });
// Infinite-scroll feed with sort + scope + genre
const feedQuery = useMusicFeed({ kind: 36787, sort, scope, genre: selectedGenre });
const {
data: rawData,
isPending,
isLoading,
isError,
fetchNextPage,
hasNextPage,
isFetchingNextPage,
@@ -72,16 +73,9 @@ export function MusicTracksTab() {
if (event.kind !== 36787) return false;
if (parseMusicTrack(event) === null) return false;
if (muteItems.length > 0 && isEventMuted(event, muteItems)) return false;
// Genre filter: check t tags
if (selectedGenre) {
const hasGenre = event.tags.some(
([n, v]) => n === 't' && v?.toLowerCase() === selectedGenre,
);
if (!hasGenre) return false;
}
return true;
});
}, [rawData?.pages, muteItems, selectedGenre]);
}, [rawData?.pages, muteItems]);
const showSkeleton = isPending || (isLoading && !rawData);
@@ -105,7 +99,11 @@ export function MusicTracksTab() {
)}
{/* Track list */}
{showSkeleton ? (
{isError ? (
<p className="px-4 py-12 text-sm text-muted-foreground text-center">
Failed to load tracks. Check your relay connections and try again.
</p>
) : showSkeleton ? (
<div>
{Array.from({ length: 10 }).map((_, i) => (
<MusicTrackRowSkeleton key={i} />
-38
View File
@@ -111,42 +111,4 @@ export function useMusicData(options: UseMusicDataOptions = {}) {
};
}
/**
* Query music tracks filtered by genre tag.
*
* Uses relay-level `#t` filtering for efficiency. Separate from the
* base useMusicData query because genre-filtered results need their
* own cache entry and may have different pagination.
*
* When `authors` is provided, only tracks by those pubkeys are returned.
*/
export function useMusicTracksByGenre(
genre: string | null,
options: { authors?: string[]; enabled?: boolean } = {},
) {
const { nostr } = useNostr();
const { authors, enabled = true } = options;
const authorsKey = authors ? authors.slice().sort().join(',') : 'all';
return useQuery<NostrEvent[]>({
queryKey: ['music-tracks-genre', genre, authorsKey],
queryFn: async ({ signal }) => {
if (!genre) return [];
const filter: Record<string, unknown> = { kinds: [36787], '#t': [genre], limit: 50 };
if (authors && authors.length > 0) {
filter.authors = authors;
}
return nostr.query(
[filter as { kinds: number[]; '#t': string[]; limit: number; authors?: string[] }],
{ signal: AbortSignal.any([signal, AbortSignal.timeout(10000)]) },
);
},
staleTime: 5 * 60 * 1000,
gcTime: 15 * 60 * 1000,
enabled: enabled && !!genre,
placeholderData: (prev) => prev,
});
}
+9 -2
View File
@@ -14,6 +14,8 @@ interface UseMusicFeedOptions {
sort: MusicSort;
/** Scope: global or following. */
scope: MusicScope;
/** Optional genre tag for relay-level `#t` filtering. */
genre?: string | null;
/** Whether the query should run (default: true). */
enabled?: boolean;
}
@@ -31,7 +33,7 @@ interface UseMusicFeedOptions {
*
* Pagination uses `until`-based cursor on `created_at`.
*/
export function useMusicFeed({ kind, sort, scope, enabled = true }: UseMusicFeedOptions) {
export function useMusicFeed({ kind, sort, scope, genre, enabled = true }: UseMusicFeedOptions) {
const { nostr } = useNostr();
const { data: followData } = useFollowList();
const followPubkeys = followData?.pubkeys;
@@ -39,7 +41,7 @@ export function useMusicFeed({ kind, sort, scope, enabled = true }: UseMusicFeed
// Following scope requires a loaded follow list
const isFollowsReady = scope !== 'following' || (followPubkeys && followPubkeys.length > 0);
const queryKey = ['music-feed', kind, sort, scope, scope === 'following' ? followPubkeys?.join(',') ?? '' : ''] as const;
const queryKey = ['music-feed', kind, sort, scope, genre ?? '', scope === 'following' ? followPubkeys?.join(',') ?? '' : ''] as const;
return useInfiniteQuery({
queryKey,
@@ -58,6 +60,11 @@ export function useMusicFeed({ kind, sort, scope, enabled = true }: UseMusicFeed
filter.authors = followPubkeys;
}
// Genre: relay-level tag filtering
if (genre) {
filter['#t'] = [genre];
}
// Sort: add NIP-50 search extension for hot/top
if (sort === 'hot') {
filter.search = 'sort:hot';