Skip to content

Enhance book clippings UI with sorting, view modes, and pagination #179

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AnnatarHe merged 1 commit into from
Apr 14, 2026
Merged

Enhance book clippings UI with sorting, view modes, and pagination #179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
179 changes: 142 additions & 37 deletions src/app/dash/[userid]/book/[bookid]/content.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,26 @@
'use client'
import { useQuery } from '@apollo/client/react'
import { BookOpen } from 'lucide-react'
import { Masonry, useInfiniteLoader } from 'masonic'
import { useRef } from 'react'
import { useMemo, useRef, useState } from 'react'

import ClippingItem from '@/components/clipping-item/clipping-item'
import BookClippingCard from '@/components/clipping-item/book-clipping-card'
import BookClippingsToolbar, {
type ClippingsSortOrder,
type ClippingsViewMode,
} from '@/components/clipping-item/book-clippings-toolbar'
import InfiniteScrollFooter from '@/components/clipping-item/infinite-scroll-footer'
import { usePageTrack } from '@/hooks/tracke'
Comment on lines +12 to 13
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 14, 2026

Choose a reason for hiding this comment

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

medium

The BookPageSkeleton import was removed, but it is still needed to provide a proper loading state while clippingsData is being fetched on the client. Re-adding this import allows for a better user experience than returning null.

Suggested change
import InfiniteScrollFooter from '@/components/clipping-item/infinite-scroll-footer'
import { usePageTrack } from '@/hooks/tracke'
import InfiniteScrollFooter from '@/components/clipping-item/infinite-scroll-footer'
import BookPageSkeleton from './skeleton'
import { usePageTrack } from '@/hooks/tracke'

import { useMasonaryColumnCount } from '@/hooks/use-screen-size'
import { BookDocument, type BookQuery, type Clipping } from '@/schema/generated'
import { useTranslation } from '@/i18n/client'
import { BookDocument, type BookQuery } from '@/schema/generated'
import { IN_APP_CHANNEL } from '@/services/channel'
import type { WenquBook } from '@/services/wenqu'
import { uniqueById } from '@/utils/array'

import BookPageSkeleton from './skeleton'
export const BOOK_CLIPPINGS_PAGE_SIZE = 12

type BookClippingsItem = NonNullable<BookQuery['book']['clippings']>[number]

type BookPageContentProps = {
userid: string
Expand All @@ -19,67 +29,162 @@ type BookPageContentProps = {

function BookPageContent(props: BookPageContentProps) {
const { userid: domain, book: bookData } = props
const { t } = useTranslation(undefined, 'book')
usePageTrack('book', {
bookId: bookData.id,
})

const hasMore = useRef(true)
const [extraItems, setExtraItems] = useState<BookClippingsItem[]>([])
const loadingRef = useRef(false)

const { data: clippingsData, fetchMore } = useQuery<BookQuery>(BookDocument, {
variables: {
id: bookData.doubanId,
pagination: {
limit: 10,
offset: 0,
limit: BOOK_CLIPPINGS_PAGE_SIZE,
},
},
notifyOnNetworkStatusChange: true,
})

const masonaryColumnCount = useMasonaryColumnCount()
const maybeLoadMore = useInfiniteLoader(
(_, __, currentItems) => {
if (!hasMore.current) {
return Promise.reject(1)
}
return fetchMore({
const initialClippings = (clippingsData?.book.clippings ?? []) as
| BookClippingsItem[]
| never[]
const totalCount = clippingsData?.book.clippingsCount ?? 0

// Merge + dedupe server page + loaded pages.
const mergedItems = useMemo(
() => uniqueById<BookClippingsItem>([...initialClippings, ...extraItems]),
[initialClippings, extraItems]
)

const [sort, setSort] = useState<ClippingsSortOrder>('newest')
const [view, setView] = useState<ClippingsViewMode>('masonry')

// Client-side sort of loaded items. A backend `orderBy` arg is a follow-up;
// until then this gives the user instant control over loaded rows.
const renderList = useMemo(() => {
if (mergedItems.length < 2) {
return mergedItems
}
const copy = [...mergedItems]
copy.sort((a, b) => {
const delta = (b.id as number) - (a.id as number)
return sort === 'newest' ? delta : -delta
})
Comment on lines +71 to +74
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 14, 2026

Choose a reason for hiding this comment

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

medium

Sorting by id assumes that IDs are strictly chronological and numeric. It is more reliable and accurate to sort by createdAt, which is explicitly intended for time-based ordering. Additionally, using a direct subtraction on a type-asserted number can be fragile if the ID format is actually a string (as indicated by the uniqueById utility type).

    copy.sort((a, b) => {
      const timeA = a.createdAt ? new Date(a.createdAt).getTime() : 0
      const timeB = b.createdAt ? new Date(b.createdAt).getTime() : 0
      const delta = timeB - timeA
      return sort === 'newest' ? delta : -delta
    })

return copy
}, [mergedItems, sort])

const hasMore = mergedItems.length < totalCount
const [loadingMore, setLoadingMore] = useState(false)

const loadMore = async () => {
if (!hasMore || loadingRef.current || mergedItems.length === 0) {
return
}
loadingRef.current = true
setLoadingMore(true)
try {
const lastId = mergedItems[mergedItems.length - 1].id as number
const resp = await fetchMore({
variables: {
id: bookData.doubanId,
pagination: {
limit: 10,
offset: currentItems.length,
limit: BOOK_CLIPPINGS_PAGE_SIZE,
lastId,
},
},
})
const next = (resp.data?.book.clippings ?? []) as BookClippingsItem[]
if (next.length > 0) {
setExtraItems((prev) => [...prev, ...next])
}
} finally {
loadingRef.current = false
setLoadingMore(false)
}
}

const masonaryColumnCount = useMasonaryColumnCount()
const maybeLoadMore = useInfiniteLoader(
async () => {
if (!hasMore) {
return
}
await loadMore()
},
{
isItemLoaded: (index, items) => !!items[index],
threshold: 3,
totalItems: clippingsData?.book.clippingsCount,
totalItems: totalCount,
}
)

if (!bookData || !clippingsData) {
return <BookPageSkeleton />
// Initial loading state — rendered by the Suspense skeleton instead.
if (!clippingsData) {
return null
}
Comment on lines +124 to 126
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 14, 2026

Choose a reason for hiding this comment

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

medium

Returning null while data is loading results in a blank UI area, which is a regression from the previous implementation that showed a loading skeleton. Since BookPageSkeleton was updated in this PR, it should be used here to improve perceived performance and maintain a consistent layout during the initial client-side fetch.

Suggested change
if (!clippingsData) {
return null
}
// Initial loading state
if (!clippingsData) {
return <BookPageSkeleton />
}


// Empty state
if (totalCount === 0) {
return (
<div className="mt-4 flex flex-col items-center justify-center rounded-2xl border border-slate-200/60 bg-white/60 px-6 py-16 text-center backdrop-blur-xl dark:border-slate-700/50 dark:bg-slate-800/50">
<div className="mb-4 flex h-14 w-14 items-center justify-center rounded-2xl bg-blue-50 text-blue-400 dark:bg-blue-400/10 dark:text-blue-300">
<BookOpen size={28} />
</div>
<h3 className="mb-1 text-lg font-semibold text-slate-800 dark:text-slate-100">
{t('app.book.clippings.empty.title')}
</h3>
<p className="max-w-sm text-sm text-slate-500 dark:text-slate-400">
{t('app.book.clippings.empty.description')}
</p>
</div>
)
}

const footerState: 'loading' | 'hasMore' | 'end' = loadingMore
? 'loading'
: hasMore
? 'hasMore'
: 'end'

const columnCount = view === 'list' ? 1 : masonaryColumnCount
const columnGutter = view === 'list' ? 0 : 24

return (
<Masonry
items={(clippingsData?.book.clippings ?? []) as Clipping[]}
columnCount={masonaryColumnCount}
columnGutter={30}
onRender={maybeLoadMore}
render={(row) => {
const clipping = row.data
return (
<ClippingItem
item={clipping}
domain={domain}
book={bookData}
key={clipping.id}
inAppChannel={IN_APP_CHANNEL.clippingFromBook}
/>
)
}}
/>
<section aria-label="clippings" className="mt-2">
<BookClippingsToolbar
totalCount={totalCount}
loadedCount={mergedItems.length}
sort={sort}
onSortChange={setSort}
view={view}
onViewChange={setView}
/>

<Masonry
// Keying on view/sort forces Masonic to recalc layout cleanly.
key={`${view}-${sort}-${columnCount}`}
Comment on lines +166 to +167
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 14, 2026

Choose a reason for hiding this comment

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

medium

Including sort in the key prop of the Masonry component causes the entire grid to unmount and remount whenever the sort order changes. This is inefficient and can cause a flickering effect as the list grows. Masonic is designed to handle item reordering efficiently via the items prop and itemKey. The key should only include properties that fundamentally change the layout structure, such as view or columnCount.

Suggested change
// Keying on view/sort forces Masonic to recalc layout cleanly.
key={`${view}-${sort}-${columnCount}`}
// Keying on view/columnCount forces Masonic to recalc layout when the grid structure changes.
key={`${view}-${columnCount}`}

items={renderList}
columnCount={columnCount}
columnGutter={columnGutter}
onRender={maybeLoadMore}
itemKey={(item: BookClippingsItem) => item.id}
render={(row) => {
const clipping = row.data as BookClippingsItem
return (
<BookClippingCard
item={clipping}
domain={domain}
density={view === 'list' ? 'compact' : 'default'}
inAppChannel={IN_APP_CHANNEL.clippingFromBook}
/>
)
}}
/>

<InfiniteScrollFooter state={footerState} onLoadMore={loadMore} />
</section>
)
}

Expand Down
5 changes: 2 additions & 3 deletions src/app/dash/[userid]/book/[bookid]/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
wenquRequest,
} from '@/services/wenqu'

import BookPageContent from './content'
import BookPageContent, { BOOK_CLIPPINGS_PAGE_SIZE } from './content'
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 14, 2026

Choose a reason for hiding this comment

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

P1 Badge Move pagination size constant out of the client component

page.tsx is a Server Component, but this import pulls BOOK_CLIPPINGS_PAGE_SIZE from content.tsx, which is marked 'use client'. In App Router, server code can render a client component reference, but it cannot reliably consume runtime values exported by that client module; using this binding in pagination.limit can fail at runtime/build time. Put the constant in a shared non-client module (or duplicate locally) so the server query always receives a plain number.

Useful? React with 👍 / 👎.


type PageProps = {
params: Promise<{ bookid: string; userid: string }>
Expand Down Expand Up @@ -71,8 +71,7 @@ async function Page(props: PageProps) {
variables: {
id: ~~dbId,
pagination: {
limit: 10,
offset: 0,
limit: BOOK_CLIPPINGS_PAGE_SIZE,
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 14, 2026

Choose a reason for hiding this comment

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

high

The clippings data is fetched here on the server but is not passed to the BookPageContent component. Consequently, the client-side component performs the exact same query again upon mounting. This redundant network request increases page load time and server load. Consider passing the server-fetched data as props to BookPageContent to initialize the client-side state or hydrate the Apollo cache.

},
},
context: {
Expand Down
21 changes: 18 additions & 3 deletions src/app/dash/[userid]/book/[bookid]/skeleton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,30 @@ function BookPageSkeleton() {
/>
</div>

{/* Toolbar skeleton */}
<div className="flex flex-col items-start justify-between gap-3 sm:flex-row sm:items-center">
<div className="h-6 w-40 rounded-md bg-gray-200 dark:bg-zinc-700" />
<div className="flex items-center gap-2">
<div className="h-9 w-32 rounded-xl bg-gray-200 dark:bg-zinc-700" />
<div className="h-9 w-20 rounded-xl bg-gray-200 dark:bg-zinc-700" />
</div>
</div>

{/* Clippings grid skeleton */}
<div className="grid grid-cols-1 gap-6 md:grid-cols-2 lg:grid-cols-3">
{new Array(6).fill(1).map((_, i) => (
<div
key={i}
className="space-y-4 rounded-2xl border border-slate-100/50 bg-slate-50/80 p-6 backdrop-blur-sm dark:border-slate-700/50 dark:bg-slate-800/70"
className="relative space-y-4 overflow-hidden rounded-2xl border border-slate-200/60 bg-white/70 p-6 backdrop-blur-sm dark:border-slate-700/50 dark:bg-slate-800/60"
>
<div className="h-6 w-3/4 rounded bg-gray-200 dark:bg-zinc-700" />
<div className="h-px w-full bg-gradient-to-r from-transparent via-gray-200 to-transparent dark:via-zinc-700" />
<span
aria-hidden
className="absolute top-4 bottom-4 left-0 w-[3px] rounded-r-full bg-gradient-to-b from-blue-300/40 to-transparent dark:from-blue-400/40"
/>
<div className="flex items-center justify-between">
<div className="h-5 w-14 rounded-full bg-blue-100 dark:bg-blue-400/20" />
<div className="h-3 w-20 rounded bg-gray-200 dark:bg-zinc-700" />
</div>
<div className="space-y-2">
<div className="h-4 w-full rounded bg-gray-200 dark:bg-zinc-700" />
<div className="h-4 w-5/6 rounded bg-gray-200 dark:bg-zinc-700" />
Expand Down
97 changes: 97 additions & 0 deletions src/components/clipping-item/book-clipping-card.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import dayjs from 'dayjs'
import Link from 'next/link'

import { useTranslation } from '@/i18n/client'
import type { Clipping } from '@/schema/generated'
import type { IN_APP_CHANNEL } from '@/services/channel'

import ClippingContent from '../clipping-content'

type BookClippingCardProps = {
item: Pick<Clipping, 'id' | 'content' | 'pageAt' | 'createdAt'>
domain: string
density?: 'default' | 'compact'
inAppChannel: IN_APP_CHANNEL
className?: string
}

function BookClippingCard(props: BookClippingCardProps) {
const {
item,
domain,
density = 'default',
inAppChannel,
className = '',
} = props
const { t } = useTranslation(undefined, 'book')

const pageLabel = item.pageAt ? String(item.pageAt).trim() : ''
const hasPage = pageLabel.length > 0
const date = item.createdAt ? dayjs(item.createdAt).format('YYYY-MM-DD') : ''

const isCompact = density === 'compact'

return (
<Link
href={`/dash/${domain}/clippings/${item.id}?iac=${inAppChannel}`}
className={`group relative mb-5 block ${className}`}
>
<article
className={[
'relative overflow-hidden rounded-2xl border backdrop-blur-xl transition-all duration-300 ease-out',
'border-slate-200/60 bg-white/70 shadow-[0_4px_24px_rgba(15,23,42,0.04)]',
'hover:-translate-y-0.5 hover:border-blue-300/60 hover:shadow-[0_12px_40px_rgba(59,130,246,0.15)]',
'dark:border-slate-700/50 dark:bg-slate-800/60 dark:shadow-[0_4px_24px_rgba(0,0,0,0.25)]',
'dark:hover:border-blue-400/40 dark:hover:shadow-[0_12px_40px_rgba(59,130,246,0.2)]',
'motion-reduce:transition-none motion-reduce:hover:translate-y-0',
isCompact ? 'p-4 lg:p-5' : 'p-6 lg:p-7',
].join(' ')}
>
{/* Subtle left quote accent bar */}
{!isCompact && (
<span
aria-hidden
className="pointer-events-none absolute top-4 bottom-4 left-0 w-[3px] rounded-r-full bg-gradient-to-b from-blue-400/70 via-blue-400/40 to-transparent opacity-70 transition-opacity duration-300 group-hover:opacity-100 dark:from-blue-400/80 dark:via-blue-400/40"
/>
)}

{/* Top row: page pill + date */}
{(hasPage || date) && (
<header
className={`flex items-center justify-between gap-2 ${isCompact ? 'mb-2' : 'mb-3'}`}
>
{hasPage ? (
<span className="inline-flex items-center rounded-full border border-blue-200/60 bg-blue-50/70 px-2.5 py-0.5 font-mono text-[11px] font-medium tracking-wide text-blue-600 dark:border-blue-400/30 dark:bg-blue-400/10 dark:text-blue-300">
{t('app.book.clippings.meta.page', { page: pageLabel })}
</span>
) : (
<span />
)}
{date && (
<time
dateTime={item.createdAt as unknown as string}
className="text-[11px] font-medium tracking-wide text-slate-400 tabular-nums dark:text-slate-500"
>
{date}
</time>
)}
</header>
)}

{/* Body */}
<ClippingContent
content={item.content}
maxLines={isCompact ? 4 : 0}
className={[
'font-lxgw text-slate-800 dark:text-slate-200',
isCompact
? 'text-base leading-relaxed lg:text-lg'
: 'text-lg leading-relaxed lg:text-xl',
].join(' ')}
/>
</article>
</Link>
)
}

export default BookClippingCard
Loading
Loading