feat(webapp): favorite pages and sidebar customization#4375
Conversation
Add a star button to every page header (and Option+F) that favorites the current page, full URL included, into a new Favorites side menu section that appears instantly, with hover Rename (inline edit) and Remove. Add a Customize sidebar modal on section header ellipsis menus: reorder sections with arrows, drag-reorder items, hide items behind a per-section More popover via eye toggles, and rename favorites inline. Changes apply on Confirm; Reset restores the default layout without touching favorites. Preferences are stored per user in dashboard preferences.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds favorite dashboard pages with metadata-based labels and icons, click and Alt+F toggling, optimistic updates, inline rename/remove actions, and authenticated resource endpoints. Refactors sidebar preferences to store section order, item visibility, item order, and favorite arrangements. Adds a customization dialog for reordering, hiding, and renaming items, plus shared sidebar ordering and active-state helpers. Registers page titles so the header favorite control can label detail pages. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Move the favorite star next to the page title, brighten it on hover, and use a favorite's custom name in its tooltip. Favorite links now carry a marker param so only the favorite highlights as active (with its page's icon color), never its identical main menu item. Fix removing a favorite from its hover menu not persisting: the mutation fetcher now lives in the side menu, since the item unmounts optimistically and an unmounting owner's request gets aborted. Side menu labels fade out at the right edge when they overflow instead of hard-clipping. Section headers keep their hover state while hovering the header ellipsis, the More menu gains a Customize sidebar entry, and popover menu items use the larger side menu size. In the Customize sidebar modal: favorite name fields match the row text size, blank names show an error and block Confirm, Reset keeps custom names while restoring layout, the footer divider spans the full modal width, and the scrollbar sits at the modal edge. Built-in metric dashboards now save with their own icons and names when favorited.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
New favorites now land at the top of the Favorites section, and favoriting a detail page whose title is generic uses the short id from the URL (e.g. "Run: 05hrqq9n", matching the id shown in the runs table). The star tooltip previews that label before you save. Fix sidebar customizations reappearing after Reset + Confirm: every preference writer is a read-modify-write over one JSON column, and a concurrent write (a debounced collapse or width save, a favorite toggle, a dashboard reorder) could land after the clear and write the old customization back from its stale read. All preference writes now re-read the row under a lock inside a transaction, so concurrent writers serialize instead of clobbering each other.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/webapp/app/services/dashboardPreferences.server.ts (1)
222-252: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
FavoritePage.urlaccepts arbitrary strings, so restrict it to app-relative paths before persisting or rendering favorites.
addFavoritestoresfavorite.urlexactly as submitted, butFavoritePage.urlis onlyz.string()and the route validation only checks that links start with/and not//. This still allows://javascript:...and other non-app-like URLs; add a zod-safe pattern for paths like/...and keep thenoStartsWith("//")guard.
♻️ Duplicate comments (1)
apps/webapp/app/services/dashboardPreferences.server.ts (1)
278-308: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
renameFavoritestill doesn't trim the label.
updateSideMenuCustomizationtrims incoming favorite labels (see line 357), but this direct rename path storeslabelas-is, and the no-op checkfavorite.label === labelwon't catch a rename to an already-untrimmed-equivalent string. This is the same gap flagged on a previous commit and remains unaddressed here.🐛 Proposed fix
const favorite = favorites.find((f) => f.id === id); - if (!favorite || favorite.label === label) { + const trimmed = label.trim(); + if (!favorite || favorite.label === trimmed) { return undefined; } return { ...prefs, sideMenu: { ...currentSideMenu, - favorites: favorites.map((f) => (f.id === id ? { ...f, label } : f)), + favorites: favorites.map((f) => (f.id === id ? { ...f, label: trimmed } : f)), }, };
🧹 Nitpick comments (1)
apps/webapp/app/services/dashboardPreferences.server.ts (1)
156-165: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winThree mutators skip the no-op guard used elsewhere in this file.
clearCurrentProject,updateSideMenuCustomization, andupdateItemOrderalways return a new value from theirmutatecallback and therefore always trigger a write under theFOR UPDATElock, unlikeupdateCurrentProjectEnvironmentId,addFavorite,removeFavorite,renameFavorite, andupdateSideMenuPreferences, which all returnundefinedwhen nothing actually changed. This is purely a consistency/efficiency gap (the lock still guarantees correctness either way), but it needlessly extends lock hold time and issues no-op UPDATEs on the exact hot paths (debounced reorder/collapse events, repeated Reset confirms) this refactor was built to protect.
apps/webapp/app/services/dashboardPreferences.server.ts#L156-L165: returnundefinedfrom themutatecallback whenprefs.currentProjectIdis alreadyundefined.apps/webapp/app/services/dashboardPreferences.server.ts#L310-L374: compare the finalSideMenuPreferences.parse(next)againstcurrentSideMenuand returnundefinedif unchanged.apps/webapp/app/services/dashboardPreferences.server.ts#L385-L420: compareupdatedSideMenuagainstcurrentSideMenuand returnundefinedif unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 77baaf4d-f889-4c28-953e-cbad68980b6f
📒 Files selected for processing (3)
apps/webapp/app/components/navigation/FavoritePageButton.tsxapps/webapp/app/components/navigation/favoritePages.tsxapps/webapp/app/services/dashboardPreferences.server.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/webapp/app/components/navigation/favoritePages.tsx
- apps/webapp/app/components/navigation/FavoritePageButton.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (19)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 12)
- GitHub Check: runops-guard / runops-guard
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: typecheck / typecheck
- GitHub Check: code-quality / code-quality
- GitHub Check: audit
- GitHub Check: audit
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Prefer static imports over dynamicimport(); use dynamic imports only for unresolvable circular dependencies, genuine performance code splitting, or conditional runtime loading.
Import Trigger.dev tasks from@trigger.dev/sdk; never use@trigger.dev/sdk/v3or deprecatedclient.defineJob.
Add agentcrumbs while writing code using approved namespaces; mark lines with//@Crumbsor blocks with `// `#region` `@crumbs, and strip them before merging.
Files:
apps/webapp/app/services/dashboardPreferences.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/services/dashboardPreferences.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/services/dashboardPreferences.server.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/app/services/dashboardPreferences.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: Access environment variables through theenvexport ofenv.server.tsinstead of directly accessingprocess.env
Use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepathDo not reintroduce the removed v1 execution path;
RunEngineVersion.V1branches may only reject or finalize gracefully so v3 clients receive a clean 4xx, never a 5xx.
Files:
apps/webapp/app/services/dashboardPreferences.server.ts
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
For apps, use
typecheckfor verification and never usebuildas the correctness check.
Files:
apps/webapp/app/services/dashboardPreferences.server.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/app/**/*.{ts,tsx}: For dashboard changes, visually verify the running Remix app with Chrome DevTools MCP, using snapshots, screenshots, interaction, and console-message checks as appropriate.
UseuseCallbackanduseMemoonly for context provider values, expensive derived data used as a dependency, or stable references required by dependency arrays; do not wrap ordinary event handlers or trivial computations.
Use named constants for sentinel or placeholder values instead of scattering raw string literals across comparisons.
Files:
apps/webapp/app/services/dashboardPreferences.server.ts
apps/webapp/app/**/*.ts
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/app/**/*.ts: Never userequest.signalto detect client disconnects. UsegetRequestAbortSignal()fromapp/services/httpAsyncStorage.server.ts, which is wired to Express response close events.
Access environment variables through theenvexport fromapp/env.server.ts; never useprocess.envdirectly.
Always use PrismafindFirstinstead offindUnique.
Always use the$transactionhelper from~/db.server, never callprisma.$transactionor$replica.$transactiondirectly. Pass isolation levels as strings, useSerializablefor correctness-critical read-then-write invariants, and guard possibly undefined helper results when a definite value is required.
Files:
apps/webapp/app/services/dashboardPreferences.server.ts
🧠 Learnings (14)
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).
Applied to files:
apps/webapp/app/services/dashboardPreferences.server.ts
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Applied to files:
apps/webapp/app/services/dashboardPreferences.server.ts
📚 Learning: 2026-05-18T08:21:27.694Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3632
File: apps/webapp/sentry.server.ts:4-21
Timestamp: 2026-05-18T08:21:27.694Z
Learning: When handling Prisma error P1001 ("Can't reach database server") in TypeScript, don’t assume a single error shape. Prisma can surface P1001 via two different error classes/fields: `PrismaClientKnownRequestError` exposes it as `err.code === "P1001"` (common during mid-query connection drops), while `PrismaClientInitializationError` exposes it as `err.errorCode === "P1001"` (common on client startup failure). Therefore, predicates should use `err.code === "P1001" || err.errorCode === "P1001"`. Do not flag `err.code === "P1001"` as “unreachable/never matches,” as it is expected in production.
Applied to files:
apps/webapp/app/services/dashboardPreferences.server.ts
📚 Learning: 2026-05-18T08:21:27.694Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3632
File: apps/webapp/sentry.server.ts:4-21
Timestamp: 2026-05-18T08:21:27.694Z
Learning: When handling Prisma errors for P1001 ("Can't reach database server"), do not assume it only appears under a single property name. Prisma may surface P1001 via either `PrismaClientKnownRequestError` (`err.code === "P1001"`, e.g., mid-query connection drops) or `PrismaClientInitializationError` (`err.errorCode === "P1001"`, e.g., client startup connection failure). To reliably detect the condition, check `err.code === "P1001" || err.errorCode === "P1001"`, and avoid review rules that would incorrectly flag `err.code === "P1001"` as unreachable/never-matching.
Applied to files:
apps/webapp/app/services/dashboardPreferences.server.ts
📚 Learning: 2026-06-13T19:53:13.759Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3937
File: packages/trigger-sdk/skills/realtime-and-frontend/SKILL.md:258-260
Timestamp: 2026-06-13T19:53:13.759Z
Learning: When reviewing code that uses `trigger.dev/react-hooks`’s `useRealtimeRun`, preserve the call signature where the first argument is the full realtime handle object (not `handle.id`). This is intentional to maintain type-safety and is consistent with the official docs; do not suggest changing the first argument from the handle object to `handle.id`.
Applied to files:
apps/webapp/app/services/dashboardPreferences.server.ts
📚 Learning: 2026-06-17T17:13:49.929Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3948
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.bulk-actions.$bulkActionParam/route.tsx:48-62
Timestamp: 2026-06-17T17:13:49.929Z
Learning: In triggerdotdev/trigger.dev, within `dashboardLoader`/`dashboardAction` (or similar context resolver code) whenever you resolve an organization ID from an organization slug for RBAC/enterprise authorization scope, always read from the primary Prisma client (`prisma`), not `$replica`. Using `$replica` can hit replica-lag and cause the RBAC lookup/authorization to run without the correct org scope (bypassing intended role enforcement). Implement the slug→org lookup with `prisma.organization.findFirst(...)` (or equivalent primary-client query) and add an inline comment documenting why the primary client is required (replica lag could lead to unscoped RBAC checks).
Applied to files:
apps/webapp/app/services/dashboardPreferences.server.ts
📚 Learning: 2026-06-23T13:04:21.413Z
Learnt from: carderne
Repo: triggerdotdev/trigger.dev PR: 4023
File: apps/webapp/app/services/upsertBranch.server.ts:14-18
Timestamp: 2026-06-23T13:04:21.413Z
Learning: In TypeScript, it’s valid to `import { type X }` and then use `typeof X` in a type-only position, e.g. `type Alias = z.infer<typeof X>`. The `type` modifier suppresses the runtime import, but the type checker still has the full exported type so `z.infer<typeof X>` can resolve correctly. In code reviews, don’t flag this as a TypeScript compile error as long as `typeof X` is used in a type context (e.g., with `z.infer`, `type` aliases, generics), not as a runtime value.
Applied to files:
apps/webapp/app/services/dashboardPreferences.server.ts
📚 Learning: 2026-03-26T09:02:07.973Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 3274
File: apps/webapp/app/services/runsReplicationService.server.ts:922-924
Timestamp: 2026-03-26T09:02:07.973Z
Learning: When parsing Trigger.dev task run annotations in server-side services, keep `TaskRun.annotations` strictly conforming to the `RunAnnotations` schema from `trigger.dev/core/v3`. If the code already uses `RunAnnotations.safeParse` (e.g., in a `#parseAnnotations` helper), treat that as intentional/necessary for atomic, schema-accurate annotation handling. Do not recommend relaxing the annotation payload schema or using a permissive “passthrough” parse path, since the annotations are expected to be written atomically in one operation and should not contain partial/legacy payloads that would require a looser parser.
Applied to files:
apps/webapp/app/services/dashboardPreferences.server.ts
📚 Learning: 2026-05-05T09:38:02.512Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3523
File: apps/webapp/app/routes/api.v3.batches.ts:178-181
Timestamp: 2026-05-05T09:38:02.512Z
Learning: When reviewing code that catches `ServiceValidationError` in `*.server.ts` files, do not blindly forward `error.status` to HTTP responses, because SVEs may be thrown with non-default statuses (e.g., 400/500) and forwarding them can cause client-visible behavioral regressions (e.g., surfacing 500s to clients). Prefer a safe default response status of `error.status ?? 422`, but only after confirming via the reachable call graph that the caught `ServiceValidationError` instances are expected to carry those non-default statuses; otherwise, normalize to `422` to avoid unexpected client-visible 5xx behavior.
Applied to files:
apps/webapp/app/services/dashboardPreferences.server.ts
📚 Learning: 2026-05-12T21:04:05.815Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3542
File: apps/webapp/app/components/sessions/v1/SessionStatus.tsx:1-3
Timestamp: 2026-05-12T21:04:05.815Z
Learning: In this Remix + TypeScript codebase, do not flag a server/client boundary violation when a file imports only types from a module matching `*.server`.
Specifically, it’s safe to import types using `import type { Foo } from "*.server"` or `import { type Foo } from "*.server"` because TypeScript erases type-only imports at compile time and they emit no JavaScript, so they won’t cross the Remix server/client bundle boundary.
Only raise the boundary concern for value imports (e.g., `import { Foo }` without `type`, or `import Foo`), since those produce JavaScript output.
Applied to files:
apps/webapp/app/services/dashboardPreferences.server.ts
📚 Learning: 2026-06-25T18:21:51.905Z
Learnt from: carderne
Repo: triggerdotdev/trigger.dev PR: 4039
File: apps/webapp/app/routes/invite-revoke.tsx:0-0
Timestamp: 2026-06-25T18:21:51.905Z
Learning: During the Zod v4 migration in the triggerdotdev/trigger.dev webapp, ensure any imports from `conform-to/zod` use the Zod-4 subpath: `conform-to/zod/v4` (e.g., `import { parseWithZod } from "conform-to/zod/v4"`). Do not import from the package root `conform-to/zod`, because it is the Zod 3 implementation and may load Zod-3-only symbols (e.g., `ZodBranded`, `ZodEffects`), which can throw at module load (notably with `zod4.4.3`). This should be enforced across `apps/webapp/**/*` where helpers like `parseWithZod` and `conformZodMessage` are used.
Applied to files:
apps/webapp/app/services/dashboardPreferences.server.ts
📚 Learning: 2026-07-03T17:10:21.498Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 4148
File: apps/webapp/app/models/orgMember.server.ts:149-168
Timestamp: 2026-07-03T17:10:21.498Z
Learning: In triggerdotdev/trigger.dev, `User.email` (Prisma schema: `internal-packages/database/prisma/schema.prisma`) currently does NOT use `citext` and does NOT have a `lower(email)` functional unique index. Therefore, do not introduce Prisma queries like `where: { email: { equals: <value>, mode: "insensitive" } }` (or any case-insensitive lookup) against `User.email`, because it can force sequential scans of the `users` table under load. During review, ensure email is normalized (e.g., lowercased/trimmed) before both writes and subsequent lookups, and if true case-insensitive behavior/uniqueness is required, implement it via a separate app-wide migration (e.g., switch to `citext` and/or add a functional unique index with backfill) rather than bolting it onto individual feature PRs.
Applied to files:
apps/webapp/app/services/dashboardPreferences.server.ts
📚 Learning: 2026-06-04T18:16:35.386Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3836
File: apps/supervisor/src/backpressure/backpressureMonitor.ts:3-5
Timestamp: 2026-06-04T18:16:35.386Z
Learning: When reviewing TypeScript in this repo, apply the rule “prefer type aliases over interfaces” only to data/object shapes and union/intersection type modeling. If an interface is being used as a behavioral contract for collaborators to implement (e.g., method-shape interfaces that define required behavior, such as `BackpressureLogger` / `BackpressureSignalSource` in `apps/supervisor/src/backpressure/backpressureMonitor.ts`), keep it as an `interface` and do not flag it as a type-alias-vs-interface violation.
Applied to files:
apps/webapp/app/services/dashboardPreferences.server.ts
📚 Learning: 2026-06-09T17:58:04.699Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 3879
File: apps/webapp/app/models/vercelIntegration.server.ts:619-630
Timestamp: 2026-06-09T17:58:04.699Z
Learning: In this codebase, outbound raw `fetch` calls should typically rely on Node/undici’s default request timeout (about ~300s) rather than adding a per-call `AbortController` + `setTimeout` wrapper inside individual functions (e.g. in files like `apps/webapp/app/models/vercelIntegration.server.ts`). During code review, do not flag the absence of a per-call timeout on a single `fetch` as an issue; if per-call timeouts are needed, they should be implemented via a codebase-wide convention (e.g., a shared fetch wrapper or documented pattern) rather than ad-hoc per-function changes.
Applied to files:
apps/webapp/app/services/dashboardPreferences.server.ts
🔇 Additional comments (4)
apps/webapp/app/services/dashboardPreferences.server.ts (4)
83-119: Solid fix for the prior lost-update race.
mutateDashboardPreferencesre-reads the row underSELECT ... FOR UPDATEinside the shared$transactionhelper before computing and writing the new value, correctly serializing concurrent preference writers on the same user row. The$queryRawtemplate usage is parameterized (safe from injection), and the docstring clearly documents the rationale.
120-154: LGTM!
184-217: LGTM!
254-276: LGTM!
…u spacing A favorite link's marker param now only affects the person who owns that favorite. Opening someone else's shared link (or a stale link to a removed favorite) quietly cleans the marker from the URL, leaves the star unfilled unless the page is in your own favorites, and highlights the regular menu item as usual. The customize modal's list scrolls flush between the header divider and the footer border, with the scrollbar spanning that full area. The section header ellipsis now sits clear of the header's edges while the header height stays the same.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Deleting a custom dashboard now also removes any of your favorites pointing at it, so the side menu doesn't keep a dead link. The favorite item's ellipsis menu reveals instantly on hover instead of fading in, and the header star sits flush against the page title accessory so the visual gaps between title, help icon, and star are even.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…spacing
Favoriting a filtered view now summarizes its filters in the default
name ("Runs: Canceled, last 30d"), naming at most two filters and
counting the rest, all derived from the URL so labels stay instant and
predictable. The Tasks page filtered to a single task type uses that
type as the whole name ("Agent tasks").
A favorite now only shows as active while the URL exactly matches the
view it saved; changing any filter hands the active state back to the
regular menu item.
Header spacing: title, help icon, and star sit on an even rhythm with
a subtle optical nudge on the accessory. Popovers containing Customize
sidebar no longer show an extra line-box pixel under their last item.
9d4a897 to
c9eb226
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…n icons Favorite rows in the customize sidebar modal gain a remove button next to the visibility toggle; removals stage in the modal and apply on Confirm (Cancel discards them). New custom icons replace the previous ones for Remove, Rename, and Customize sidebar. Slack and Vercel favorites render their brand logos at the same size as the organization menu, and favoriting a custom dashboard uses the dashboard chart icon.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…zation Skip the locked preference transaction on the hot current-environment save when the session snapshot already matches, so plain page loads do no extra database work. Give favorite rename and remove their own fetchers so quick successive mutations can't cancel each other. Close favorite and More popovers when navigation changes only the search string, keep the section options trigger visible while it holds keyboard focus, and dedupe saved order ids defensively.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Preview Deployment
|
Favoriting a task page produced "Page": the resolver keyed the tasks
list under the environment index, so the task detail path fell through
to the unnamed fallback. Task and agent pages now take the task's name
from the URL as the favorite label ("hello-world") and carry their
task-type icon (standard, scheduled, or agent), matching how the tasks
list shows them. Their page headers render the name as composed markup,
so the URL is the only place it can be read from.
Also names the playground route, which reported "Page" the same way.
…ng silently A confirmed customization could silently vanish: the modal closed the moment Confirm was clicked, so a save that failed or hung (dev server hiccups, transaction timeouts) looked identical to a successful one, and a thrown preferences write escalated to the app error boundary. Confirm now spins until the save response and the refreshed side menu data land, the dialog stays open with an inline error on failure, the preference routes report failures as responses instead of throwing, and the preferences transaction retries retriable Prisma errors.
| const [, setSearchParams] = useSearchParams(); | ||
|
|
||
| // The favorite marker param is presentation-only, so it never counts toward URL identity | ||
| const url = location.pathname + stripFavoriteSearchParam(location.search); |
There was a problem hiding this comment.
🔍 Favorited URLs capture pagination/cursor params, which the label logic deliberately ignores
When favoriting a page, the saved URL is location.pathname + stripFavoriteSearchParam(location.search) (apps/webapp/app/components/navigation/FavoritePageButton.tsx:41,79), which retains volatile params like cursor, page, direction, and span. The label builder explicitly treats these as non-descriptive via NON_FILTER_PARAMS (apps/webapp/app/components/navigation/favoritePages.tsx:347), but that list is only used for the label, not for stripping them from the persisted URL. Consequence: favoriting a filtered runs view while on page 2 saves the exact cursor, so clicking the favorite later navigates to a stale/possibly-invalid paginated view, and isFavoriteActive will only match when that exact cursor is in the URL. The PR description says favorites save "the exact view," so this may be intended — but reusing NON_FILTER_PARAMS to also strip pagination from the saved URL would likely match user expectations better. Worth confirming.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Favorite any dashboard page and it appears in a new "Favorites" section at the top of the side menu. The star next to the page title (or Option+F) saves the exact view, filters and tabs included, with a name derived from the URL ("Runs: Completed successfully, last 7d", "Run: 05hrqq9n") that you can rename inline from each item's hover menu.
The sidebar is customizable too: "Customize sidebar" (on section header menus and in each "More" menu) opens a modal where you can reorder sections, drag items into a new order, hide items behind a per-section "More" popover, and rename or remove favorites. Changes apply on Confirm, Reset restores the default layout without touching favorites, and everything is stored per user in dashboard preferences.
Screenshots
Design notes
event.codewith a raw listener because macOS reports Option-modified letters as symbols, which theevent.keybased shortcut hook can't capture.Verified end-to-end in the browser: star toggle and shortcut, instant section appearance, inline rename and staged modal removal, filter-aware labels and unique active states, shared-link normalization, drag reordering, and persistence across reloads.