feat(web): adapt UI for mobile viewports#108
Merged
Merged
Conversation
Make settings, dashboard, data-table, and detail pages responsive: stack header/action rows on small screens, allow horizontal table scrolling via ScrollArea, and constrain widths to prevent overflow. Default the servers list to grid view on mobile and split server cell renderers and view-mode resolution into dedicated modules.
Move mobile-view.test.ts into the components/ directory so it sits beside mobile-view.ts, matching the index-cells test layout.
The servers table view and traffic ranking table were missing the max-w-[calc(100vw-1.5rem)] page-root constraint that other table pages already use. Without a definite width ceiling the table-fixed + min-w-max intrinsic width propagated up through main, so the ScrollArea viewport equalled the table width and columns were clipped unreachably at 375px instead of scrolling. Add the established page-root clamp and mirror the ui/data-table max-w-full wrapper on the shared DataTable.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR transforms the web app into a mobile-responsive layout system by adding width constraints, horizontal scrolling, responsive grid breakpoints, and column-stacking flex layouts. It centralizes mobile view detection for the servers list and updates 40+ UI components and pages with consistent responsive patterns. ChangesMobile-First Responsive Layout System
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/src/routes/_authed/traffic/index.tsx (1)
268-268: 💤 Low valueConsider extracting the hardcoded padding value.
The
max-w-[calc(100vw-1.5rem)]hardcodes the main layout's horizontal padding (p-3 = 0.75rem × 2 from_authed.tsxline 233). If the layout padding changes, this calculation will become incorrect.♻️ Optional refactor to reduce coupling
Define a shared CSS custom property in your theme or use a Tailwind config value:
/* In your global CSS or theme */ :root { --main-horizontal-padding: 1.5rem; } `@media` (min-width: 640px) { :root { --main-horizontal-padding: 2rem; /* matches sm:p-4 */ } }Then reference it:
-<div className="w-full min-w-0 max-w-[calc(100vw-1.5rem)] overflow-hidden sm:max-w-full"> +<div className="w-full min-w-0 max-w-[calc(100vw-var(--main-horizontal-padding))] overflow-hidden sm:max-w-full">Or define the constraint pattern in a shared constant if you prefer JavaScript.
🤖 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 `@apps/web/src/routes/_authed/traffic/index.tsx` at line 268, The hardcoded horizontal padding used in the div with className "w-full min-w-0 max-w-[calc(100vw-1.5rem)] overflow-hidden sm:max-w-full" should be extracted to a shared value (either a CSS custom property like --main-horizontal-padding or a JS/Tailwind config constant) and referenced in the max-width calc instead of the literal 1.5rem so layout padding changes remain consistent; update the JSX to use that shared value (or the corresponding Tailwind pattern) and adjust responsive overrides (sm) to mirror the padding changes defined in the shared value, ensuring the calc expression and responsive variants reference the same single source of truth.
🤖 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 `@apps/web/src/routes/_authed/settings/alerts.tsx`:
- Around line 483-489: The mapped list is returning a Fragment (<>...</>) while
the key is placed on an inner <div key={rule.id}>, which causes React warnings
and reconciliation issues; fix it by moving the key to the top-level returned
node—either replace the shorthand Fragment with an explicit React.Fragment and
set key={rule.id} on it, or remove the Fragment and put the key on the outermost
element that wraps the mapped content (referencing the map callback that uses
rule.id and the Fragment surrounding the <div className="flex ...">).
---
Nitpick comments:
In `@apps/web/src/routes/_authed/traffic/index.tsx`:
- Line 268: The hardcoded horizontal padding used in the div with className
"w-full min-w-0 max-w-[calc(100vw-1.5rem)] overflow-hidden sm:max-w-full" should
be extracted to a shared value (either a CSS custom property like
--main-horizontal-padding or a JS/Tailwind config constant) and referenced in
the max-width calc instead of the literal 1.5rem so layout padding changes
remain consistent; update the JSX to use that shared value (or the corresponding
Tailwind pattern) and adjust responsive overrides (sm) to mirror the padding
changes defined in the shared value, ensuring the calc expression and responsive
variants reference the same single source of truth.
🪄 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: abf2bb5f-59aa-4d76-9d73-9f58bf88580e
📒 Files selected for processing (38)
apps/web/src/components/dashboard/dashboard-editor-view.tsxapps/web/src/components/dashboard/dashboard-grid.tsxapps/web/src/components/dashboard/widget-picker.tsxapps/web/src/components/data-table/data-table-pagination.tsxapps/web/src/components/data-table/data-table-skeleton.tsxapps/web/src/components/data-table/data-table-toolbar.tsxapps/web/src/components/data-table/data-table.test.tsxapps/web/src/components/data-table/data-table.tsxapps/web/src/components/server/server-edit-dialog.tsxapps/web/src/components/task/scheduled-task-dialog.tsxapps/web/src/components/ui/data-table.tsxapps/web/src/components/ui/scroll-area.tsxapps/web/src/components/ui/table.tsxapps/web/src/routes/_authed.tsxapps/web/src/routes/_authed/files.$serverId.tsxapps/web/src/routes/_authed/network/$serverId.tsxapps/web/src/routes/_authed/servers/components/index-cells.tsxapps/web/src/routes/_authed/servers/components/mobile-view.test.tsapps/web/src/routes/_authed/servers/components/mobile-view.tsapps/web/src/routes/_authed/servers/index.cells.test.tsxapps/web/src/routes/_authed/servers/index.tsxapps/web/src/routes/_authed/settings/alerts.tsxapps/web/src/routes/_authed/settings/api-keys.tsxapps/web/src/routes/_authed/settings/appearance.tsxapps/web/src/routes/_authed/settings/audit-logs.tsxapps/web/src/routes/_authed/settings/capabilities.tsxapps/web/src/routes/_authed/settings/index.tsxapps/web/src/routes/_authed/settings/mobile-devices.tsxapps/web/src/routes/_authed/settings/network-probes.tsxapps/web/src/routes/_authed/settings/notifications.tsxapps/web/src/routes/_authed/settings/ping-tasks.tsxapps/web/src/routes/_authed/settings/security.tsxapps/web/src/routes/_authed/settings/service-monitors.tsxapps/web/src/routes/_authed/settings/status-pages.tsxapps/web/src/routes/_authed/settings/users.tsxapps/web/src/routes/_authed/terminal.$serverId.tsxapps/web/src/routes/_authed/traffic/index.tsxapps/web/src/routes/login.tsx
Comment on lines
483
to
489
| <> | ||
| <div className="flex items-center justify-between px-4 py-3" key={rule.id}> | ||
| <div className="flex items-center gap-3"> | ||
| <div | ||
| className="flex flex-col gap-3 px-4 py-3 sm:flex-row sm:items-center sm:justify-between" | ||
| key={rule.id} | ||
| > | ||
| <div className="flex min-w-0 items-center gap-3"> | ||
| <AlertTriangle |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -name "alerts.tsx" -path "*/_authed/settings/*" 2>/dev/null | head -5Repository: ZingerLittleBee/ServerBee
Length of output: 51
🏁 Script executed:
# Check if file exists and get its structure
fd -p "alerts.tsx" | grep "_authed/settings"Repository: ZingerLittleBee/ServerBee
Length of output: 117
🏁 Script executed:
# Search for the file from repo root
find . -name "alerts.tsx" -type f | grep settings | head -3Repository: ZingerLittleBee/ServerBee
Length of output: 119
🏁 Script executed:
# Use git to find the file
git ls-files | grep "alerts.tsx" | grep settingsRepository: ZingerLittleBee/ServerBee
Length of output: 117
🏁 Script executed:
wc -l ./apps/web/src/routes/_authed/settings/alerts.tsxRepository: ZingerLittleBee/ServerBee
Length of output: 123
🏁 Script executed:
# Read the file with context around lines 483-489
sed -n '470,510p' ./apps/web/src/routes/_authed/settings/alerts.tsxRepository: ZingerLittleBee/ServerBee
Length of output: 2223
🏁 Script executed:
# Continue reading to see the closing of the Fragment
sed -n '483,530p' ./apps/web/src/routes/_authed/settings/alerts.tsxRepository: ZingerLittleBee/ServerBee
Length of output: 2505
🏁 Script executed:
# Continue reading to see the closing Fragment and the expanded content
sed -n '520,560p' ./apps/web/src/routes/_authed/settings/alerts.tsxRepository: ZingerLittleBee/ServerBee
Length of output: 2200
🏁 Script executed:
# Continue reading to see the closing of the Fragment
sed -n '555,590p' ./apps/web/src/routes/_authed/settings/alerts.tsxRepository: ZingerLittleBee/ServerBee
Length of output: 1906
Move the key attribute to the top-level mapped element.
The key is currently on the inner <div>, but the top-level returned node from the map callback is a Fragment. React expects the key on the outermost element and will warn about a missing key, potentially causing reconciliation issues with expanded rows.
Suggested fix
- return (
- <>
- <div
- className="flex flex-col gap-3 px-4 py-3 sm:flex-row sm:items-center sm:justify-between"
- key={rule.id}
- >
+ return (
+ <div key={rule.id}>
+ <div className="flex flex-col gap-3 px-4 py-3 sm:flex-row sm:items-center sm:justify-between">
<div className="flex min-w-0 items-center gap-3">
@@
- {expandedRuleId === rule.id && (
+ {expandedRuleId === rule.id && (
<div className="border-t bg-muted/20 px-4 py-2">
@@
- )}
- </>
+ )}
+ </div>
)📝 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.
Suggested change
| <> | |
| <div className="flex items-center justify-between px-4 py-3" key={rule.id}> | |
| <div className="flex items-center gap-3"> | |
| <div | |
| className="flex flex-col gap-3 px-4 py-3 sm:flex-row sm:items-center sm:justify-between" | |
| key={rule.id} | |
| > | |
| <div className="flex min-w-0 items-center gap-3"> | |
| <AlertTriangle | |
| return ( | |
| <div key={rule.id}> | |
| <div className="flex flex-col gap-3 px-4 py-3 sm:flex-row sm:items-center sm:justify-between"> | |
| <div className="flex min-w-0 items-center gap-3"> | |
| <AlertTriangle | |
| {/* ... existing content ... */} | |
| </div> | |
| {expandedRuleId === rule.id && ( | |
| <div className="border-t bg-muted/20 px-4 py-2"> | |
| {/* ... existing content ... */} | |
| </div> | |
| )} | |
| </div> | |
| </div> | |
| ) |
🤖 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 `@apps/web/src/routes/_authed/settings/alerts.tsx` around lines 483 - 489, The
mapped list is returning a Fragment (<>...</>) while the key is placed on an
inner <div key={rule.id}>, which causes React warnings and reconciliation
issues; fix it by moving the key to the top-level returned node—either replace
the shorthand Fragment with an explicit React.Fragment and set key={rule.id} on
it, or remove the Fragment and put the key on the outermost element that wraps
the mapped content (referencing the map callback that uses rule.id and the
Fragment surrounding the <div className="flex ...">).
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
1 participant
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.
Summary
flex-col … sm:flex-row), widths constrained (min-w-0,max-w-[calc(100vw-1.5rem)]) to eliminate horizontal page overflow, and sharedScrollArea/Tableprimitives now allow horizontal scrolling.mobile-view.tsand server cell renderers intocomponents/index-cells.tsx.table-fixed+min-w-maxpropagated a runaway intrinsic width up throughmainand columns were clipped unreachably at 375px instead of scrolling. Added the establishedmax-w-[calc(100vw-1.5rem)]clamp.Test plan
tsc --noEmitpassesmobile-viewanddata-tablemobile-layout tests)/servers?view=tableand/traffic(viewport clamped to 349px,scrollLeftadvances, no page overflow)Summary by CodeRabbit
New Features
Style
Tests