Add document bookmarks#13
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
- persist the view token a doc was bookmarked through and re-check access with it on /bookmarks, so a token-shared doc stays linkable (token re-appended) until the token is rotated or the doc is revoked - make the doc-page bookmark button a real toggle (add/remove) - add a per-row remove control on /bookmarks, working for revoked docs too - revoked bookmarks show "<doc id> — revoked" with no link, never the live title of a doc the viewer can no longer access - label token-only access "link" (was mislabeled "public") - extract pure row rendering to lib/docs/bookmarks-view.ts with unit tests Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add the /bookmarks web surface to the Viewing listing in skill-content.ts (the source for /llms.txt and SKILL.md), alongside its sibling /docs. Regenerated SKILL.md via gen:skill. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the "bookmark"/"bookmarked" text button with a clickable star (☆ unbookmarked, ★ bookmarked), matching the bar's glyph controls. Keeps the toggle behavior and adds an aria-label since it's now icon-only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Snapshot the doc title at bookmark time (bookmarks.doc_title) and render it dimmed and unlinked on revoked rows, falling back to the slug when untitled — replacing the bare "<id> — revoked". The snapshot is the title the viewer saw when bookmarking, so a revoked row stays recognizable without exposing the doc's current (possibly changed) title. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
bookmark feature user interactions what happens when a user revokes access |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
rgarcia
left a comment
rgarcia
left a comment
There was a problem hiding this comment.
can it be the bookmark icon not the star icon?
Swap the ★/☆ glyph for an inline bookmark-ribbon SVG (filled when bookmarked, outline when not), consistent with the app's other SVG icons. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The bookmark button did a native form POST → 303 → full page reload on every click, unlike the comment/reaction controls beside it. Make it an optimistic client toggle: flip the icon immediately, POST to /bookmarks in the background, roll back on failure. /bookmarks now content-negotiates — fetch (Accept: application/json) gets 204/4xx, the zero-JS list forms still get the 303 redirect. Drops the now-unused bookmarkNext prop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6ded664. Configure here.
| me, | ||
| initialSections, | ||
| } = props; | ||
| const [bookmarked, setBookmarked] = useState(props.bookmarked); |
There was a problem hiding this comment.
Bookmark icon stale after navigation
Medium Severity
The doc viewer keeps bookmark UI in useState(props.bookmarked) without resetting when the server sends a new bookmarked value for another document. Client navigations that reuse CommentsShell can show the previous doc’s filled or empty bookmark icon and wrong aria-pressed labels on the current page.
Reviewed by Cursor Bugbot for commit 6ded664. Configure here.
There was a problem hiding this comment.
Valid as a general React pattern, but not a reachable bug in this app, so leaving as-is.
- `bookmarked` is not special: `threads`, `docReactions`, `anchoredReactions`, and `theme` all use the same `useState(props.*)` seeding. The whole viewer already assumes a fresh mount per doc.
- There is no client-side navigation anywhere — no `next/link`, no `useRouter`/`router.push` (the only `next/navigation` import is `notFound`). Every doc link on `/docs`, `/bookmarks`, and the homepage is a plain server-rendered ``, i.e. a hard navigation → full page load → fresh `CommentsShell` mount with the correct `props.bookmarked`.
So the trigger the check describes ("client navigations that reuse CommentsShell") does not exist. If soft routing between docs is ever introduced, the correct fix is key={slug} on <CommentsShell> so all prop-seeded state resets together — a bookmark-only useEffect would be the wrong shape.
On add, canViewSession can pass via ownership, a grant, or the doc being public, so the submitted viewtoken wasn't necessarily the basis for access. Storing it anyway persisted a stale/irrelevant token (and could overwrite a good one on upsert), making the /bookmarks re-check fail after a grant is removed even when a valid share token still exists. Store the token only when it actually matches the doc's view_token; otherwise store null. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2 participants
Summary
Adds document bookmarks. Every doc page gets a bookmark button that saves the doc to the signed-in user's bookmarks, and
/bookmarkslists them — split into "your documents" and "shared with you", mirroring the/docsdesign.Bookmarks re-resolve access on every load rather than trusting save-time access, so they honor the same authorization as the viewer:
revokedwith no link, showing only its doc id — never the live title of a doc the viewer can no longer see.Behavior
/bookmarksroute with OWNED and SHARED-WITH-YOU sections, man-page styled like/docs.removeon/bookmarks, keyed by doc id so revoked/deleted docs can still be dropped.owner/editor/commenter/viewer/public/link(token-only) /revoked.bookmarkstable migration; nav links added from/and/docs.lib/docs/bookmarks-view.ts(unit-tested).Tests
npm test(includeslib/docs/bookmarks-view.test.tscovering revoked / token-link / remove-form rendering)npm run buildNote
Medium Risk
New bookmark flows store optional view tokens and re-use doc access checks; mistakes could leak titles or links, but add/list paths mirror existing viewer authorization.
Overview
Adds document bookmarks for signed-in users: save docs from the viewer chrome and list them at
/bookmarks(man-page layout like/docs, split into YOUR DOCUMENTS and SHARED WITH YOU).Persistence & access: New
bookmarkstable (migration/0015) stores per-email saves with optional view token and title snapshot. Adds are gated withcanViewSession; the list re-resolves access on load (grants, public, stored token). Lost access shows as revoked (snapshot title, no link); token-only private docs stay linkable with?viewtoken=until rotation/delete.POST /bookmarkssupports JSON (204) for the viewer toggle and form redirects for list remove (bydoc_id).UI: Bookmark control in
CommentsShell(optimistic toggle). Links from/and/docs. Row HTML lives inlib/docs/bookmarks-view.ts(unit tests).Dev docs:
DEVELOPMENT.mddocuments local Postgres via Docker, migrations, andscripts/dev-seed.tsfor test users/sessions. Agent skill /llms.txtmention/bookmarks.Reviewed by Cursor Bugbot for commit 775dd8d. Bugbot is set up for automated code reviews on this repo. Configure here.