-
Notifications
You must be signed in to change notification settings - Fork 42
feat(admin-ui): use virtual datatable in admin app #1373
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.
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughA systematic update across multiple admin list pages to replace standard table rendering with virtualized content, alongside CSS column sizing adjustments and a dependency version bump for Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~18 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Pull Request Test Coverage Report for Build 21819281287Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@web/apps/admin/src/pages/audit-logs/list/list.module.css`:
- Around line 53-59: The .table-content-container CSS currently has two height
declarations; remove the redundant height: 100% so only height: calc(100vh -
90px) remains in the .table-content-container rule, leaving overflow, width and
position untouched to ensure the intended viewport-based height is applied.
🧹 Nitpick comments (2)
web/apps/admin/src/pages/organizations/list/list.module.css (1)
40-44: Consider avoiding the hard-coded 90px height offset.A fixed offset can drift if the navbar/toolbar height changes (wrapping, responsive layouts), leading to double scroll or clipped space. A CSS variable keeps it in sync.
Optional tweak (keeps current fallback)
.table-wrapper { /* Navbar Height + Toolbar height */ - height: calc(100vh - 90px); + height: calc(100vh - var(--admin-list-header-height, 90px)); overflow: auto; }web/apps/admin/src/pages/organizations/list/columns.tsx (1)
56-63: Avoid duplicated magic widths for column sizing.The 300px width is repeated across multiple columns; consider centralizing it in constants to reduce drift during future tweaks.
Example consolidation
+const NAME_COL_WIDTH = 300; +const CREATOR_COL_WIDTH = 300; +const PLAN_COL_WIDTH = 250; ... styles: { - cell: { flex: '0 0 300px' }, - header: { flex: '0 0 300px' } + cell: { flex: `0 0 ${NAME_COL_WIDTH}px` }, + header: { flex: `0 0 ${NAME_COL_WIDTH}px` } },
Summary
This PR updates the following tables to use Virtualized DataTable.
Technical Details
Test Plan
Summary by CodeRabbit
New Features
Style
Chores