From 9dfc073f485177e1199834672cb62c9a9ed13545 Mon Sep 17 00:00:00 2001 From: highperfocused Date: Sun, 6 Sep 2026 18:03:01 +0200 Subject: [PATCH] fix: address review feedback and add a Bookmarked tab to the Reader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review: - useToggleBookmark now fetches the bookmark list fresh from relays right before writing instead of trusting the query cache (60s staleTime), which could otherwise clobber concurrent edits from another tab or device. - The article BookmarkButton only renders when the article actually has a `d` tag, instead of falling back to an unresolvable "kind:pubkey:" address. - Bookmarked note/article ids are filtered for a non-empty tag value before use, and article addresses are parsed properly (kind, author, `d`) instead of a naive split(':')[2] — the relay query is now also constrained by kind and author, not just `d`, and identifiers containing ':' round-trip correctly. - BookmarkButton sets type="button" so it can't misbehave as a form submit button. Per a reviewer comment: added a "Recent" / "Bookmarked" tab to the Reader's sidebar (src/apps/articles/index.tsx) so bookmarked articles are reachable without leaving the app — the dedicated Bookmarks app stays as-is. Both now share useMyBookmarkedArticles from src/hooks/useBookmarks.ts rather than duplicating the address-parsing logic. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01BYiUtZMQeA5RHggQw73wto --- src/apps/articles/index.tsx | 119 ++++++++++++++++++++---- src/apps/bookmarks/index.tsx | 45 ++------- src/components/nostr/BookmarkButton.tsx | 1 + src/hooks/useBookmarks.ts | 99 +++++++++++++++++--- 4 files changed, 193 insertions(+), 71 deletions(-) diff --git a/src/apps/articles/index.tsx b/src/apps/articles/index.tsx index 1098bad..afe8f14 100644 --- a/src/apps/articles/index.tsx +++ b/src/apps/articles/index.tsx @@ -1,7 +1,7 @@ -import { useCallback, useEffect } from 'react'; +import { useCallback, useEffect, useState } from 'react'; import { useNostr } from '@nostrify/react'; import { useQuery } from '@tanstack/react-query'; -import { ChevronLeft, Link2 } from 'lucide-react'; +import { Bookmark, ChevronLeft, Link2, Rss } from 'lucide-react'; import type { NostrEvent } from '@nostrify/nostrify'; import { AppBody, @@ -19,6 +19,8 @@ import { Button } from '@/components/ui/button'; import { Skeleton } from '@/components/ui/skeleton'; import { nip19 } from 'nostr-tools'; import { useAuthor } from '@/hooks/useAuthor'; +import { useCurrentUser } from '@/hooks/useCurrentUser'; +import { useMyBookmarkedArticles } from '@/hooks/useBookmarks'; import { absoluteTime, decodeRelayHints, @@ -33,6 +35,8 @@ import { useToast } from '@/hooks/useToast'; import { cn } from '@/lib/utils'; import type { AppParams, AppProps } from '@/os/types'; +type ListScope = 'recent' | 'bookmarked'; + const ARTICLE_KIND = 30023; /** A long-form event is only useful with a body and a `d` identifier (NIP-23). */ @@ -85,6 +89,13 @@ function useArticle( export default function ArticlesApp({ params, setTitle, setParams }: AppProps) { const isMobile = useIsMobile(); + const { user } = useCurrentUser(); + + // Signing out mid-session must not strand the reader on a "Bookmarked" tab + // it can no longer see anything in, so the effective scope is derived + // rather than corrected after render — same reasoning as the Feed. + const [requestedScope, setRequestedScope] = useState('recent'); + const scope: ListScope = user ? requestedScope : 'recent'; // The selection lives in the window's params rather than local state, so the // URL, a reload and the switch between the desktop and mobile shells all @@ -103,7 +114,9 @@ export default function ArticlesApp({ params, setTitle, setParams }: AppProps) { [setParams], ); - const list = useRecentArticles(); + const recent = useRecentArticles(); + const bookmarked = useMyBookmarkedArticles(); + const list = scope === 'recent' ? recent : bookmarked; const article = useArticle( selected?.pubkey, selected?.identifier, @@ -117,13 +130,19 @@ export default function ArticlesApp({ params, setTitle, setParams }: AppProps) { }, [title, setTitle]); const listPane = ( - - select({ pubkey: event.pubkey, identifier, kind: String(event.kind) }) - } - /> +
+ +
+ + select({ pubkey: event.pubkey, identifier, kind: String(event.kind) }) + } + /> +
+
); const readerPane = !selected ? ( @@ -176,12 +195,14 @@ export default function ArticlesApp({ params, setTitle, setParams }: AppProps) { {article.data && (
- + {tagValue(article.data, 'd') && ( + + )}
)} @@ -225,12 +246,74 @@ function CopyArticleLink({ event }: { event: NostrEvent }) { ); } +function ListScopeTabs({ + scope, + disabled, + onChange, +}: { + scope: ListScope; + disabled: boolean; + onChange: (scope: ListScope) => void; +}) { + return ( +
+ onChange('recent')} + icon={} + label="Recent" + /> + onChange('bookmarked')} + icon={} + label="Bookmarked" + /> +
+ ); +} + +function ScopeTab({ + active, + disabled, + onClick, + icon, + label, +}: { + active: boolean; + disabled?: boolean; + onClick: () => void; + icon: React.ReactNode; + label: string; +}) { + return ( + + ); +} + function ArticleList({ query, + scope, selected, onSelect, }: { query: ReturnType; + scope: ListScope; selected: { pubkey: string; identifier: string } | null; onSelect: (event: NostrEvent, identifier: string) => void; }) { @@ -247,14 +330,14 @@ function ArticleList({ if (!query.data || query.data.length === 0) { return (

- No articles on your relays. + {scope === 'bookmarked' ? 'You haven’t bookmarked any articles yet.' : 'No articles on your relays.'}

); } return ( <> - Recent + {scope === 'bookmarked' ? 'Bookmarked' : 'Recent'}
    {query.data.map((event) => { const identifier = tagValue(event, 'd')!; diff --git a/src/apps/bookmarks/index.tsx b/src/apps/bookmarks/index.tsx index 9d36918..f8252d4 100644 --- a/src/apps/bookmarks/index.tsx +++ b/src/apps/bookmarks/index.tsx @@ -1,4 +1,4 @@ -import { useEffect, useMemo } from 'react'; +import { useEffect } from 'react'; import { useNostr } from '@nostrify/react'; import { useQuery } from '@tanstack/react-query'; import type { NostrEvent } from '@nostrify/nostrify'; @@ -7,7 +7,7 @@ import { LoginRequired } from '@/components/nostr/LoginRequired'; import { NoteCard } from '@/components/nostr/NoteCard'; import { Skeleton } from '@/components/ui/skeleton'; import { useAuthor } from '@/hooks/useAuthor'; -import { useBookmarkList } from '@/hooks/useBookmarks'; +import { useBookmarkedNoteIds, useMyBookmarkedArticles } from '@/hooks/useBookmarks'; import { useCurrentUser } from '@/hooks/useCurrentUser'; import { useWindowManager } from '@/os/useWindowManager'; import { displayName, relativeTime, tagValue } from '@/lib/nostrUtils'; @@ -30,55 +30,22 @@ function useBookmarkedNotes(ids: string[]) { }); } -/** Addresses are `kind:pubkey:d-identifier`; only the `d` half is queryable, so matches are narrowed back down locally. */ -function useBookmarkedArticles(addresses: string[]) { - const { nostr } = useNostr(); - const dTags = useMemo( - () => [...new Set(addresses.map((address) => address.split(':')[2]).filter(Boolean))], - [addresses], - ); - - return useQuery({ - queryKey: ['nostr', 'bookmarked-articles', addresses.join(',')], - enabled: dTags.length > 0, - queryFn: async ({ signal }) => { - const events = await nostr.query( - [{ kinds: [30023], '#d': dTags, limit: dTags.length * 4 }], - { signal: AbortSignal.any([signal, AbortSignal.timeout(6000)]) }, - ); - const wanted = new Set(addresses); - return events.filter((event) => wanted.has(`${event.kind}:${event.pubkey}:${tagValue(event, 'd')}`)); - }, - staleTime: 60_000, - }); -} - export default function BookmarksApp({ setTitle }: AppProps) { const { user } = useCurrentUser(); const { openApp } = useWindowManager(); useEffect(() => setTitle('Bookmarks'), [setTitle]); - const list = useBookmarkList(); - const noteIds = useMemo( - () => (list.data?.tags ?? []).filter(([name]) => name === 'e').map(([, id]) => id), - [list.data], - ); - const addresses = useMemo( - () => (list.data?.tags ?? []).filter(([name]) => name === 'a').map(([, address]) => address), - [list.data], - ); - + const noteIds = useBookmarkedNoteIds(); const notes = useBookmarkedNotes(noteIds); - const articles = useBookmarkedArticles(addresses); + const articles = useMyBookmarkedArticles(); if (!user) { return ; } - const isLoading = - list.isLoading || (noteIds.length > 0 && notes.isLoading) || (addresses.length > 0 && articles.isLoading); - const isEmpty = !isLoading && noteIds.length === 0 && addresses.length === 0; + const isLoading = (noteIds.length > 0 && notes.isLoading) || articles.isLoading; + const isEmpty = !isLoading && (notes.data?.length ?? 0) === 0 && (articles.data?.length ?? 0) === 0; return ( diff --git a/src/components/nostr/BookmarkButton.tsx b/src/components/nostr/BookmarkButton.tsx index 309ba98..e005553 100644 --- a/src/components/nostr/BookmarkButton.tsx +++ b/src/components/nostr/BookmarkButton.tsx @@ -30,6 +30,7 @@ export function BookmarkButton({ target, className }: { target: BookmarkTarget; return (