feat: project-wide bookmark manager with a -/bookmarks tab and home page section - #9863
Open
nishantmonu51 wants to merge 4 commits into
Open
nishantmonu51 wants to merge 4 commits into
nishantmonu51 wants to merge 4 commits into
Conversation
Bookmarks were only reachable from a single dashboard's dropdown. This adds a Bookmarks section on the project home page and a `-/bookmarks` tab that list every bookmark the user can see across dashboards, with search, sorting and open, edit and delete actions. Backend: - `ListBookmarks` treats resource kind and name as optional filters, so an empty request returns all bookmarks in the project visible to the caller. - The handler now requires read access to the project and rejects a resource name without a kind. - Rows are returned ordered by name, and `UpdateBookmark` bumps `updated_on`. Frontend: - Home page section mirrors the Dashboards section: 5-row preview, sort dropdown in the heading on its own `bookmarks_sort` URL param, and a "See all bookmarks" link. Hidden for anonymous viewers. - New `-/bookmarks` page and tab with the shared table toolbar. - Sort by last used (tracked per browser like dashboards), last updated, name or dashboard. Rows show dashboard, category chips, update and usage times. - Owners and bookmark managers get hover edit (metadata dialog) and delete (with confirmation) actions. - All bookmark mutations invalidate every bookmark query so the dropdown and the manager stay in sync. - `UrlParamsState.createStringParam` is typed as non-nullable, which also removes pre-existing type errors in the dashboards listing.
AdityaHegde
requested changes
Sep 9, 2026
AdityaHegde
left a comment
AdityaHegde
left a comment
Collaborator
There was a problem hiding this comment.
- Lets align the bookmark icons here to the top. Looks weird being in the middle. Same would apply to bookmarks page.
2. Creating a dashboard doesn't invalidate the project wide query. Needs a refresh to start showing up.
| description: values.description, | ||
| // Home bookmarks are always shared. | ||
| shared: bookmark.default || values.shared === "true", | ||
| urlSearch: bookmark.urlSearch ?? "", |
Collaborator
There was a problem hiding this comment.
Since data is not changing this needs to be backwards compatible unlike the form in dashboard. Add data: bookmark.data ?? "" as well.
Collaborator
Author
There was a problem hiding this comment.
UpdateBookmarkRequest has no data field, and the SQL update only sets display_name, description, url_search and shared, so a legacy bookmark keeps its data after a metadata edit. urlSearch is passed through unchanged (empty for legacy), so the dropdown still falls back to data. Left as is.
Collaborator
There was a problem hiding this comment.
Hmmm this is probably an unintended issue with the UpdateBookmarkRequest API and the query. Not in scope of this PR
- mark `ListBookmarksRequest` filters as optional - move the web-admin list to `components/list-table/ListTable` with a required `getRowId` - open legacy explore bookmarks via the `state` param instead of a Legacy chip - refetch inactive bookmark list queries on invalidation - top-align the bookmark row icon - create the e2e fixture bookmark in `beforeAll` and navigate in-app for the dropdown check Claude-Session: https://claude.ai/code/session_01Ft2MEXK1nWvFWrubUyE82W
Collaborator
Author
|
nishantmonu51
requested review from
AdityaHegde
and
a lite review from Copilot
September 11, 2026 08:24
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate findings affect permission-aware editing, accessibility, legacy labeling, and usage tracking.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds a project-wide bookmark manager with a home-page preview, -/bookmarks tab, search, sorting, metadata management, usage tracking, localization, API updates, and tests.
Changes:
- Adds bookmark navigation, listing, editing, deletion, and usage tracking.
- Extends bookmark APIs, persistence, invalidation, and permissions.
- Adds shared list-table behavior, translations, and automated coverage.
Review findings:
- Moderate (3 votes):
manage_bookmarksusers cannot access the category selector. - Nit (1 vote): the
"Required"validation message is not localized. - Moderate (3 votes): hover-only actions are inaccessible to keyboard and touch users.
- Moderate (2 votes): legacy bookmarks do not display the Legacy chip.
- Moderate (1 vote): unsupported rows incorrectly update last-used tracking.
File summaries
| File | Description |
|---|---|
web-common/src/proto/gen/rill/admin/v1/api_pb.ts |
Updates generated bookmark filters. |
web-common/src/lib/store-utils/url-params-state.svelte.ts |
Makes string URL parameters non-nullable. |
web-common/src/lib/store-utils/url-params-state.svelte.spec.ts |
Updates URL parameter tests. |
web-common/src/lib/i18n/messages/es.json |
Adds Spanish bookmark translations. |
web-common/src/lib/i18n/messages/en.json |
Adds English bookmark translations. |
web-admin/tests/bookmarks-manager.spec.ts |
Adds end-to-end manager coverage. |
web-admin/src/routes/[organization]/[project]/+page.svelte |
Adds the home-page bookmark preview. |
web-admin/src/routes/[organization]/[project]/+layout.svelte |
Propagates authentication state. |
web-admin/src/routes/[organization]/[project]/-/bookmarks/+page.svelte |
Adds the bookmark manager route. |
web-admin/src/features/projects/ProjectTabs.svelte |
Adds the bookmarks navigation tab. |
web-admin/src/features/personal-files/canvas/PersonalCanvasesList.svelte |
Migrates the list to ListTable. |
web-admin/src/features/dashboards/listing/DashboardsTable.svelte |
Migrates the dashboard list to ListTable. |
web-admin/src/features/bookmarks/utils.ts |
Adds bookmark filter classification helpers. |
web-admin/src/features/bookmarks/selectors.ts |
Adds project queries and invalidation. |
web-admin/src/features/bookmarks/recently-used-bookmarks.ts |
Tracks local bookmark usage. |
web-admin/src/features/bookmarks/listing/BookmarksTableCompositeCell.svelte |
Renders bookmark rows and actions. |
web-admin/src/features/bookmarks/listing/BookmarksTable.svelte |
Implements manager table behavior. |
web-admin/src/features/bookmarks/listing/bookmark-listing-utils.ts |
Builds, filters, and sorts rows. |
web-admin/src/features/bookmarks/listing/bookmark-listing-utils.spec.ts |
Tests listing utilities. |
web-admin/src/features/bookmarks/HomeBookmarkButton.svelte |
Tracks home bookmark usage. |
web-admin/src/features/bookmarks/BookmarksFormDialog.svelte |
Uses shared bookmark invalidation. |
web-admin/src/features/bookmarks/Bookmarks.svelte |
Syncs mutations and usage tracking. |
web-admin/src/features/bookmarks/BookmarkMetadataDialog.svelte |
Adds bookmark metadata editing. |
web-admin/src/components/list-table/ListTableToolbar.svelte |
Adds reusable list search controls. |
web-admin/src/components/list-table/ListTable.svelte |
Generalizes list rows and identities. |
web-admin/src/client/gen/index.schemas.ts |
Updates generated API schemas. |
proto/rill/admin/v1/api.proto |
Makes bookmark filters optional. |
proto/gen/rill/admin/v1/openapi.yaml |
Regenerates OpenAPI documentation. |
proto/gen/rill/admin/v1/api.pb.validate.go |
Regenerates validation code. |
proto/gen/rill/admin/v1/admin.swagger.yaml |
Regenerates Swagger documentation. |
admin/server/bookmarks.go |
Adds access checks and flexible listing. |
admin/server/bookmarks_test.go |
Adds backend bookmark coverage. |
admin/database/postgres/postgres.go |
Updates filtering, ordering, and timestamps. |
admin/database/database.go |
Documents the expanded bookmark query contract. |
Review details
Suppressed comments (2)
web-admin/src/features/bookmarks/BookmarkMetadataDialog.svelte:38
"Required"is a new user-facing validation message in a component that already uses Paraglide. Use the existing localizedm.common_required()message instead; otherwise Spanish users will see English validation copy.
displayName: string().required("Required"),
web-admin/src/features/bookmarks/listing/BookmarksTableCompositeCell.svelte:54
- When
row.hrefis undefined for an unsupported resource kind, this anchor still invokesonOpen(row). That records a last-used timestamp even though no dashboard was opened, which can incorrectly reorder the list underLast used. Only record usage for rows with a navigable href.
<a
class="flex flex-row items-start gap-x-3 min-w-0 grow h-full py-2.5"
href={row.href}
aria-label={m.bookmark_entry_aria_label({ name: displayName })}
onclick={() => onOpen(row)}
- Files reviewed: 33/35 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+112
to
+116
| <ProjectAccessControls {organization} {project}> | ||
| <Select | ||
| bind:value={$form["shared"]} | ||
| id="shared" | ||
| label={m.bookmark_category()} |
Collaborator
Author
There was a problem hiding this comment.
Every seeded role sets manage_bookmarks equal to manage_project and there are no custom roles, so no user has one without the other. This mirrors the existing BookmarksFormDialog, so leaving it as is.
Comment on lines
+65
to
+69
| {#if row.category === "home"} | ||
| <Tag color="blue">{m.bookmark_tag_home()}</Tag> | ||
| {:else if row.category === "managed"} | ||
| <Tag color="gray">{m.bookmark_tag_managed()}</Tag> | ||
| {/if} |
Collaborator
Author
There was a problem hiding this comment.
The Legacy chip was intentionally removed in the previous revision; legacy explore bookmarks now open through the state URL param instead. The PR description was stale and has been updated.
Comment on lines
+117
to
+134
| {#if hovered} | ||
| <Button | ||
| square | ||
| type="tertiary" | ||
| label={m.bookmark_edit()} | ||
| onClick={() => onEdit(row)} | ||
| > | ||
| <Pencil size="16px" /> | ||
| </Button> | ||
| <Button | ||
| square | ||
| type="tertiary" | ||
| label={m.bookmark_delete_bookmark()} | ||
| onClick={() => onDelete(row)} | ||
| > | ||
| <Trash size="16px" /> | ||
| </Button> | ||
| {/if} |
Collaborator
Author
There was a problem hiding this comment.
Fixed in 1c950c0: the actions stay mounted and are revealed with group-hover and group-focus-within, so keyboard and touch users can reach them.
AdityaHegde
approved these changes
Sep 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bookmarks were only reachable from a single dashboard's dropdown, so there was no way to find a bookmark across dashboards or manage a long list of them. This adds a bookmark manager that follows the Dashboards listing UX.
bookmarks_sortURL param so it does not collide with the dashboards sort), and a "See all bookmarks" link. It is hidden for anonymous viewers of public projects.-/bookmarkstab and page lists every bookmark the user can see across dashboards, with the shared table toolbar for search and sort.stateURL param.BookmarkMetadataDialog) and to delete with a confirmation.ListBookmarksnow treats resource kind and name as optional filters, requires read access to the project, and returns rows ordered by name;UpdateBookmarkbumpsupdated_on. Same response shape, so the existing dropdown callers are unchanged. Covered byadmin/server/bookmarks_test.go.UrlParamsState.createStringParamis typed as non-nullable, which also removes pre-existing type errors in the dashboards listing.web-admin/tests/bookmarks-manager.spec.ts; unit tests inbookmark-listing-utils.spec.ts.Tags on bookmarks are a follow-up; the slot next to the sort dropdown is left for the tag filter.
Checklist: