style(ui): polish shared console surfaces#2213
Conversation
📝 WalkthroughWalkthroughThis PR introduces configurable pagination to DataTable via a new optional ChangesComponent Behavior & Authentication Updates
Design & Theme Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/dashboard/ChartCard.vue`:
- Line 74: The badge's dynamic class binding using 'bg-cyan-600' /
'bg-amber-600' for the element that shows lastDayEvolution doesn't meet WCAG
contrast; update the class binding in ChartCard.vue (the expression referencing
lastDayEvolution) to use darker background colors (e.g., 'bg-cyan-800' and
'bg-amber-800') or switch to a high-contrast text color (e.g., add 'text-black'
instead of 'text-white') and optionally increase weight/size (e.g.,
'font-semibold') so the evolution value meets accessibility contrast
requirements.
In `@src/components/tables/AppTable.vue`:
- Line 324: The table currently hard-codes :page-size="10" regardless of mode,
causing pagination UI to move while the table shows the full client-side list;
change the binding to a computed value (e.g., pageSizeForMode) and set
:page-size="pageSizeForMode", where pageSizeForMode returns 10 only when the
component is in server-side pagination mode and returns filteredApps.length (or
null/undefined) when using client-side mode; update the computed or prop logic
that determines server-side mode (the same flag you already use to decide
whether filteredApps is pre-sliced) so the paginator and rendered rows stay in
sync.
In `@src/pages/admin/dashboard/replication.vue`:
- Around line 111-113: The current thrown Error when checking "if
(!session?.access_token)" still references a replication secret fallback; update
the message to reflect that replication now relies solely on Supabase session
auth by changing the error text thrown from that check (the `if
(!session?.access_token)` branch) to a clear message like "No Supabase session
available; authentication is required to perform replication" so it no longer
mentions a replication secret.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5c44921a-228a-4abf-b834-b07cf407c0db
⛔ Files ignored due to path filters (12)
docs/pr-screenshots/ui-polish-tweaks/after/app-bundles.jpgis excluded by!**/*.jpgdocs/pr-screenshots/ui-polish-tweaks/after/app-overview.jpgis excluded by!**/*.jpgdocs/pr-screenshots/ui-polish-tweaks/after/apps.jpgis excluded by!**/*.jpgdocs/pr-screenshots/ui-polish-tweaks/after/dashboard.jpgis excluded by!**/*.jpgdocs/pr-screenshots/ui-polish-tweaks/after/login.jpgis excluded by!**/*.jpgdocs/pr-screenshots/ui-polish-tweaks/after/settings-organization.jpgis excluded by!**/*.jpgdocs/pr-screenshots/ui-polish-tweaks/before/app-bundles.jpgis excluded by!**/*.jpgdocs/pr-screenshots/ui-polish-tweaks/before/app-overview.jpgis excluded by!**/*.jpgdocs/pr-screenshots/ui-polish-tweaks/before/apps.jpgis excluded by!**/*.jpgdocs/pr-screenshots/ui-polish-tweaks/before/dashboard.jpgis excluded by!**/*.jpgdocs/pr-screenshots/ui-polish-tweaks/before/login.jpgis excluded by!**/*.jpgdocs/pr-screenshots/ui-polish-tweaks/before/settings-organization.jpgis excluded by!**/*.jpg
📒 Files selected for processing (13)
src/components/DataTable.vuesrc/components/Navbar.vuesrc/components/Sidebar.vuesrc/components/Tabs.vuesrc/components/auth/AuthPageShell.vuesrc/components/dashboard/ChartCard.vuesrc/components/tables/AppTable.vuesrc/layouts/admin.vuesrc/layouts/app.vuesrc/layouts/settings.vuesrc/pages/admin/dashboard/replication.vuesrc/pages/login.vuesrc/styles/style.css
| v-if="showEvolutionBadge" | ||
| class="inline-flex justify-center items-center rounded-full px-3 py-1 text-xs font-bold text-white shadow-sm" | ||
| :class="{ 'bg-cyan-500': (lastDayEvolution ?? 0) >= 0, 'bg-amber-500': (lastDayEvolution ?? 0) < 0 }" | ||
| :class="{ 'bg-cyan-600': (lastDayEvolution ?? 0) >= 0, 'bg-amber-600': (lastDayEvolution ?? 0) < 0 }" |
There was a problem hiding this comment.
Increase badge contrast for accessibility.
On Line 74, bg-cyan-600 / bg-amber-600 with text-white at text-xs likely remains below WCAG AA contrast for normal text. Please use darker badge backgrounds (or darker text) to keep the evolution value readable.
Proposed patch
- :class="{ 'bg-cyan-600': (lastDayEvolution ?? 0) >= 0, 'bg-amber-600': (lastDayEvolution ?? 0) < 0 }"
+ :class="{ 'bg-cyan-700': (lastDayEvolution ?? 0) >= 0, 'bg-amber-700': (lastDayEvolution ?? 0) < 0 }"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| :class="{ 'bg-cyan-600': (lastDayEvolution ?? 0) >= 0, 'bg-amber-600': (lastDayEvolution ?? 0) < 0 }" | |
| :class="{ 'bg-cyan-700': (lastDayEvolution ?? 0) >= 0, 'bg-amber-700': (lastDayEvolution ?? 0) < 0 }" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/dashboard/ChartCard.vue` at line 74, The badge's dynamic class
binding using 'bg-cyan-600' / 'bg-amber-600' for the element that shows
lastDayEvolution doesn't meet WCAG contrast; update the class binding in
ChartCard.vue (the expression referencing lastDayEvolution) to use darker
background colors (e.g., 'bg-cyan-800' and 'bg-amber-800') or switch to a
high-contrast text color (e.g., add 'text-black' instead of 'text-white') and
optionally increase weight/size (e.g., 'font-semibold') so the evolution value
meets accessibility contrast requirements.
| v-model:search="internalSearch" | ||
| :show-add="!isMobile" | ||
| :total="props.total ?? filteredApps.length" | ||
| :page-size="10" |
There was a problem hiding this comment.
Guard fixed page size to server-side mode only.
Line 324 hard-codes :page-size="10" even when filteredApps is the full unsliced list (client-side mode). That can make pagination controls/range move across pages while the table still renders all rows.
💡 Suggested fix
- :page-size="10"
+ :page-size="props.serverSidePagination ? 10 : undefined"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| :page-size="10" | |
| :page-size="props.serverSidePagination ? 10 : undefined" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/tables/AppTable.vue` at line 324, The table currently
hard-codes :page-size="10" regardless of mode, causing pagination UI to move
while the table shows the full client-side list; change the binding to a
computed value (e.g., pageSizeForMode) and set :page-size="pageSizeForMode",
where pageSizeForMode returns 10 only when the component is in server-side
pagination mode and returns filteredApps.length (or null/undefined) when using
client-side mode; update the computed or prop logic that determines server-side
mode (the same flag you already use to decide whether filteredApps is
pre-sliced) so the paginator and rendered rows stay in sync.
| if (!session?.access_token) | ||
| throw new Error('No session available and replication secret is not configured') | ||
|
|
There was a problem hiding this comment.
Update the no-session error text to match the new behavior.
Line 112 still mentions a replication secret fallback, but this flow now depends only on Supabase session auth. That message will confuse incident/debug triage.
Suggested patch
- if (!session?.access_token)
- throw new Error('No session available and replication secret is not configured')
+ if (!session?.access_token)
+ throw new Error('No authenticated session available for replication request')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!session?.access_token) | |
| throw new Error('No session available and replication secret is not configured') | |
| if (!session?.access_token) | |
| throw new Error('No authenticated session available for replication request') | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/admin/dashboard/replication.vue` around lines 111 - 113, The
current thrown Error when checking "if (!session?.access_token)" still
references a replication secret fallback; update the message to reflect that
replication now relies solely on Supabase session auth by changing the error
text thrown from that check (the `if (!session?.access_token)` branch) to a
clear message like "No Supabase session available; authentication is required to
perform replication" so it no longer mentions a replication secret.
|
I think the new pagination math is only partially wired up. That fallback breaks on a short final page. If a table has 25 total rows, a real page size of 10, and page 3 returns 5 rows, the range becomes Could we either keep |
KCDaemon
left a comment
KCDaemon
left a comment
There was a problem hiding this comment.
Rechecked the current diff and the issue is still present.
DataTable now computes pagination from props.pageSize ?? offset.value, but most server-paginated callers in this PR still do not pass page-size. I confirmed BuildTable.vue still passes the old :offset="offset" attribute, while BundleTable.vue, ChannelTable.vue, and DeviceTable.vue do not pass a page size at all even though their data loaders fetch fixed-size pages (offset = 10 or 20). offset is not declared as a DataTable prop, so it does not feed paginationPageSize; the component falls back to its internal default.
That breaks the new range / next-page math whenever the real server page size differs from the fallback. For example, BuildTable fetches 20 rows per page but DataTable will calculate with the default 10, so page 2 of 25 rows is displayed as 11-25 instead of 21-25, and fast-forward / next availability can target the wrong last page. The same class of bug appears on short final pages for the 10-row tables if the fallback diverges later.
Please either keep offset as a backwards-compatible alias on DataTable, or update all server-paginated callers to pass their fixed page size explicitly. A regression with a 25-row server-paginated table and a short last page would catch this. The PR is also currently merge-conflicted, and git apply --check does not apply cleanly against current main.
3 participants
Summary (AI generated)
Motivation (AI generated)
The console already has a strong base, but several repeated surfaces felt heavier or less tactile than they needed to be: inactive nav items had low contrast, active tabs blended into the page background, table controls had inconsistent target sizes, and the auth pages leaned on decorative blobs. These tweaks make the product feel cleaner without changing the underlying structure or workflows.
Business Impact (AI generated)
Screenshots (AI generated)
Test Plan (AI generated)
bun lintbun typecheckbun run buildbun test:front- 19 passedSummary by CodeRabbit
New Features
Style
Bug Fixes