feat: tanstack replacement for virtual-table in root keys#5066
Merged
MichaelUnkey merged 110 commits intomainfrom Mar 17, 2026
Merged
feat: tanstack replacement for virtual-table in root keys#5066MichaelUnkey merged 110 commits intomainfrom
MichaelUnkey merged 110 commits intomainfrom
Conversation
Contributor
|
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:
📝 WalkthroughWalkthroughReplaces a cursor-based root-keys list with an offset-paginated DataTable system: adds a reusable data-table in the ui package (components, hooks, types, constants), migrates dashboard root-keys UI to the new paginated flow, updates TRPC schema/router for page/limit/sort, and adjusts imports. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Dashboard UI
participant Hook as useRootKeysListPaginated
participant Filters as useFilters
participant TRPC as TRPC Client
participant Router as Backend Router
participant DB as Database
Client->>Hook: mount (page, pageSize, sort)
Hook->>Filters: read active filters
Filters-->>Hook: filter params, filterKey
Hook->>TRPC: query listRootKeys(page, limit, sort, filters)
TRPC->>Router: listRootKeys(params)
Router->>DB: count with filters
Router->>DB: fetch keys with offset & order
DB-->>Router: totalCount, keys
Router-->>TRPC: { keys, totalCount, hasMore }
TRPC-->>Hook: page data
Hook-->>Client: rootKeys, pagination meta
sequenceDiagram
participant Page as App Page
participant DataTable as DataTable Component
participant useData as useDataTable Hook
participant TanStack as `@tanstack/react-table`
participant Cells as Column Renderers
Page->>DataTable: provide data, columns, config
DataTable->>useData: init table instance
useData->>TanStack: build table (core, sorting)
TanStack-->>useData: table instance
useData-->>DataTable: table API/state
DataTable->>Cells: render per-row/per-column cell()
Cells-->>DataTable: JSX cell nodes
DataTable-->>Page: rendered table + footer/controls
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can validate your CodeRabbit configuration file in your editor.If your editor has YAML language server, you can enable auto-completion and validation by adding |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
* fix: cleanup project side nav * feat: simplify deployment overview page only show build logs until it's built, then show domains and network * chore: clean up nav
…0M +) (#4959) * fix(clickhouse): improve clickhouse query for key logs and add new table and mv for latest keys used * fix valid/error count = 0 scenario * remove identity_id from order by * wrap identity_id with aggregating function since its removed from the order key --------- Co-authored-by: Flo <53355483+Flo4604@users.noreply.github.com>
* fix: domain refetch and promotion disable rule * fix: regression --------- Co-authored-by: Andreas Thomas <dev@chronark.com>
* refactor: move custom domains to tanstack db * fix: comment * fix: delete mutation * remove: unnecessary query
* remove agent * remove agent
* remove agent * remove agent * use vault in dashboard * remove
* fix: cleanup project side nav * feat: simplify deployment overview page only show build logs until it's built, then show domains and network * chore: clean up nav * feat: add per-project sticky domain and only display that
* chore: use vault in api * chore: use vault in api * fix harness * use memory test * vault container go start * [autofix.ci] apply automated fixes --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
* dunno * nextjs should allow a setting that says dynamic * [autofix.ci] apply automated fixes --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Add references to real-time performance benchmarks in: - introduction.mdx: new 'Performance at scale' accordion - modes.mdx: link after latency claim Presents benchmarks as capability demonstration rather than comparison.
Add missing SEO description to frontmatter Generated-By: mintlify-agent Co-authored-by: mintlify[bot] <109931778+mintlify[bot]@users.noreply.github.com> Co-authored-by: Andreas Thomas <dev@chronark.com>
Remove Spring Boot Java, Rust, and Elixir SDK docs that are not linked in navigation and appear to be outdated/unmaintained. Generated-By: mintlify-agent Co-authored-by: mintlify[bot] <109931778+mintlify[bot]@users.noreply.github.com> Co-authored-by: Andreas Thomas <dev@chronark.com>
* rework release * rework release
* feat: generate rpc wrappers * bazel happyier * more changes * more changes * move path * delete old files (#5043) * fix: rabbit comments --------- Co-authored-by: Oz <21091016+ogzhanolguncu@users.noreply.github.com>
* add a gossip implementation * add gossip to sentinel/frontline * add message muxing * sentinel fun * cleansings * cleansings * cleansings * cleansings * use oneof * fix bazel happiness * do some changies * exportoneof * more cool fancy thingx * change gateway choosing * add label * adjjust some more * adjjust some more * fixa test * goodbye kafka * fix: bazel * rename gateway -> ambassador * add docs * fix: rabbit comments * [autofix.ci] apply automated fixes * idfk * more changes * more changes * fix ordering * fix missing files * fix test --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
* fix: retry cillium policy until CRDs are ready * fix: blocks until all system pods are ready
* fix: cleanup project side nav * feat: simplify deployment overview page only show build logs until it's built, then show domains and network * feat: new build screen for ongoing deployments * fix: table column typo
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
web/apps/dashboard/components/root-keys-table/hooks/use-root-keys-list-query.ts (1)
64-67:⚠️ Potential issue | 🟠 MajorEnforce the schema minimum when normalizing
pageSize.
normalizedPageSizecurrently allows1..19, but the query schema requireslimit >= 20. That can send invalid payloads and break pagination on small custom page sizes.🔧 Proposed fix
const MAX_PAGE_SIZE = 200; +const MIN_PAGE_SIZE = 20; export function useRootKeysListPaginated(pageSize = DEFAULT_PAGE_SIZE) { - const normalizedPageSize = - Number.isFinite(pageSize) && pageSize > 0 - ? Math.min(Math.floor(pageSize), MAX_PAGE_SIZE) - : DEFAULT_PAGE_SIZE; + const normalizedPageSize = Number.isFinite(pageSize) + ? Math.min(Math.max(Math.floor(pageSize), MIN_PAGE_SIZE), MAX_PAGE_SIZE) + : DEFAULT_PAGE_SIZE;As per coding guidelines: "Make illegal states unrepresentable by modeling domain with discriminated unions and parsing inputs at boundaries into typed structures".
Also applies to: 129-129, 148-148
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/apps/dashboard/components/root-keys-table/hooks/use-root-keys-list-query.ts` around lines 64 - 67, The normalizedPageSize logic currently lets values below the query schema minimum through; update the normalization of pageSize (the normalizedPageSize calculation that references pageSize, MAX_PAGE_SIZE and DEFAULT_PAGE_SIZE) to clamp values to a minimum of 20 (schema limit) before applying Math.floor and MAX_PAGE_SIZE, so the computed limit never falls below 20; apply the same minimum-enforcing change to the other occurrences of this normalization in the file (the other normalizedPageSize usages referenced in the diff) to keep all payloads valid.
🧹 Nitpick comments (1)
web/internal/ui/src/components/data-table/components/footer/load-more-footer.tsx (1)
91-94: Consider usingcn()consistently for className construction.The inner div uses a template literal with a ternary for conditional classes, while other elements in this file use the
cn()utility. Usingcn()here would improve consistency and readability.♻️ Proposed refactor for consistency
<div - className={`w-[740px] border bg-gray-1 dark:bg-black border-gray-6 min-h-[60px] flex items-center justify-center rounded-[10px] drop-shadow-lg transform-gpu shadow-sm mb-5 transition-all duration-200 hover:shadow-lg ${ - shouldShow ? "pointer-events-auto" : "pointer-events-none" - }`} + className={cn( + "w-[740px] border bg-gray-1 dark:bg-black border-gray-6 min-h-[60px] flex items-center justify-center rounded-[10px] drop-shadow-lg transform-gpu shadow-sm mb-5 transition-all duration-200 hover:shadow-lg", + shouldShow ? "pointer-events-auto" : "pointer-events-none" + )} aria-hidden={!shouldShow} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/internal/ui/src/components/data-table/components/footer/load-more-footer.tsx` around lines 91 - 94, Replace the template-literal className on the inner div with the project's cn() utility to match the rest of the file (in load-more-footer component), keeping all static classes and moving the conditional pointer-events class into cn() using shouldShow to toggle "pointer-events-auto" vs "pointer-events-none"; update the className passed to that div (where the current template literal is used) to call cn(...) so readability and consistency are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@web/apps/dashboard/components/virtual-table/components/loading-indicator.tsx`:
- Line 51: The minimized footer branch still mounts interactive controls when
hidden because the top-level wrapper only animates the expanded branch; update
the LoadingIndicator component to short-circuit and return null when hide is
true or shouldShow is false (i.e., when onLoadMore is missing) so the footer and
any focusable descendants are not mounted or focusable; locate the render in
loading-indicator.tsx and add the early return using hide || !shouldShow to
prevent mounting the interactive controls.
- Around line 96-97: The delayed fade-slide-in animations in the
LoadingIndicator JSX (the elements with className "transition-all duration-200
animate-fade-slide-in") should include animationFillMode set to "backwards" to
prevent jank during the animationDelay; update the inline style objects
(currently containing animationDelay: "0.2s" and similar) to also include
animationFillMode: "backwards" for each occurrence (the same change should be
applied to the other two instances of the same className in this file).
In
`@web/internal/ui/src/components/data-table/components/footer/load-more-footer.tsx`:
- Around line 63-70: The minimized view renders countInfoText without a
fallback, causing an empty span when undefined; update the minimized block in
the LoadMoreFooter component to render countInfoText || the same fallback used
in the expanded view (the "Viewing X of Y items" expression) so the span always
shows meaningful text, i.e., replace the direct {countInfoText} usage with the
fallback-or-value expression used in the expanded rendering and keep the
existing classes and structure.
---
Duplicate comments:
In
`@web/apps/dashboard/components/root-keys-table/hooks/use-root-keys-list-query.ts`:
- Around line 64-67: The normalizedPageSize logic currently lets values below
the query schema minimum through; update the normalization of pageSize (the
normalizedPageSize calculation that references pageSize, MAX_PAGE_SIZE and
DEFAULT_PAGE_SIZE) to clamp values to a minimum of 20 (schema limit) before
applying Math.floor and MAX_PAGE_SIZE, so the computed limit never falls below
20; apply the same minimum-enforcing change to the other occurrences of this
normalization in the file (the other normalizedPageSize usages referenced in the
diff) to keep all payloads valid.
---
Nitpick comments:
In
`@web/internal/ui/src/components/data-table/components/footer/load-more-footer.tsx`:
- Around line 91-94: Replace the template-literal className on the inner div
with the project's cn() utility to match the rest of the file (in
load-more-footer component), keeping all static classes and moving the
conditional pointer-events class into cn() using shouldShow to toggle
"pointer-events-auto" vs "pointer-events-none"; update the className passed to
that div (where the current template literal is used) to call cn(...) so
readability and consistency are preserved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f8151586-6255-42a6-8f51-d1f0ab407afa
📒 Files selected for processing (4)
web/apps/dashboard/components/root-keys-table/hooks/use-root-keys-list-query.tsweb/apps/dashboard/components/virtual-table/components/loading-indicator.tsxweb/internal/ui/src/components/data-table/components/footer/load-more-footer.tsxweb/internal/ui/src/components/data-table/types.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- web/internal/ui/src/components/data-table/types.ts
web/apps/dashboard/components/virtual-table/components/loading-indicator.tsx
Show resolved
Hide resolved
web/apps/dashboard/components/virtual-table/components/loading-indicator.tsx
Outdated
Show resolved
Hide resolved
web/internal/ui/src/components/data-table/components/footer/load-more-footer.tsx
Show resolved
Hide resolved
Member
|
Can I request we don't show page numbers unless we have more than 1 page of keys 
Also this coloring is weird both dark and light 
|
perkinsjr
approved these changes
Mar 17, 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
8 participants
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.
What does this PR do?
Changes
@unkey/ui (new exports)
Dashboard — components/root-keys-table/ (new)
Dashboard — consumer updates
Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtmake fmton/godirectoryconsole.logsgit pull origin mainAppreciated