feat: improve mobile dashboard responsiveness#31
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR adds Playwright E2E testing support with a gated mock auth session route and responsive dashboard spec, and updates navigation, button sizing, CSS utilities, and multiple admin/public tables to mobile-friendly card-table layouts. ChangesE2E Testing Infrastructure
Responsive Mobile UI Overhaul
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PlaywrightTest
participant SessionRoute
participant AuthSession
PlaywrightTest->>SessionRoute: POST { role }
SessionRoute->>SessionRoute: isE2EAuthEnabled(request)
SessionRoute->>AuthSession: getAuthSession()
SessionRoute->>AuthSession: set address, chainId, permissions, viewMode
AuthSession-->>SessionRoute: save()
SessionRoute-->>PlaywrightTest: serialized session
PlaywrightTest->>SessionRoute: DELETE
SessionRoute->>AuthSession: destroy()
SessionRoute-->>PlaywrightTest: { ok: true }
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Pull request overview
This PR adds a Playwright-based E2E smoke suite to validate responsive behavior across key dashboard/admin routes, and it refactors several admin tables and header interactions to improve mobile usability and reduce horizontal overflow.
Changes:
- Introduces Playwright E2E infrastructure (config, scripts, docs, mock-session API) to run viewport-based responsiveness checks.
- Updates admin/report tables to a mobile “card table” layout via shared CSS utilities and per-cell labels.
- Improves mobile navigation and UI touch targets (header drawer, button sizing, responsive form controls).
Reviewed changes
Copilot reviewed 22 out of 24 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/responsive-dashboard.spec.ts | Adds responsive smoke/E2E coverage and overflow/error-overlay checks across routes and roles. |
| playwright.config.ts | Configures Playwright projects for multiple viewports and starts next dev with E2E auth enabled. |
| package.json | Adds pnpm test:e2e and pnpm test:e2e:install scripts plus Playwright devDependency. |
| pnpm-lock.yaml | Locks Playwright and its transitive dependencies. |
| README.md | Documents E2E setup, browser deps install, and how mock sessions work. |
| .env.example | Adds E2E_AUTH_ENABLED example flag for local-only mock sessions. |
| eslint.config.mjs | Ignores Playwright output directories. |
| .gitignore | Ignores Playwright report and test-results output. |
| src/app/api/e2e/session/route.ts | Adds local-only mock session endpoint for E2E role switching. |
| src/app/globals.css | Adds drawer animations, box-sizing/body margin reset, and mobile card-table CSS utilities. |
| src/components/app-header.tsx | Adds a mobile drawer navigation menu and reorganizes header layout for small screens. |
| src/components/auth/wallet-connect.tsx | Adjusts profile dropdown sizing/positioning for small viewports. |
| src/components/ui/button.tsx | Increases default button hit areas on mobile via responsive size variants. |
| src/app/page.tsx | Adjusts public home logo sizing for better mobile fit. |
| src/app/reports/page.tsx | Makes report action buttons full-width on mobile for improved usability. |
| src/app/reports/quarters/[id]/page.tsx | Converts ranked/balance tables to mobile card-table layout with per-cell labels. |
| src/app/proposals/page.tsx | Converts proposals activity table to mobile card-table layout with per-cell labels. |
| src/app/membership/page.tsx | Converts cleric roles + membership activity tables to mobile card-table layout. |
| src/app/rips/page.tsx | Converts RIPs table to mobile card-table layout and adjusts padding for responsiveness. |
| src/app/raids/page.tsx | Converts raid tables to mobile card-table layout via a labeled TableLinkCell helper. |
| src/app/admin/providers/page.tsx | Converts providers ranking table to mobile card-table layout with per-cell labels. |
| src/app/admin/treasury-accounts/page.tsx | Converts treasury accounts ranking table to mobile card-table-lg layout with per-cell labels. |
| src/app/admin/quarters/[id]/transactions/classification-linked-fields.tsx | Makes select inputs full-width/min-width safe for narrow screens. |
| src/app/admin/quarters/[id]/transactions/page.tsx | Adds multiple responsive/overflow mitigations and card-table layouts within transaction review. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
src/components/app-header.tsx (1)
343-351: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant inline style overriding Tailwind class on close button.
classNamealready setsz-[60], but the inlinestyle={{ pointerEvents: "auto", zIndex: 1000 }}silently overrides it (inline styles win over classes), rendering thez-[60]class dead. This looks like a leftover workaround rather than an intentional decision. If there's a real stacking-context issue this is compensating for, prefer fixing the root cause via Tailwind (z-[1000]) rather than mixing inline styles with utility classes.♻️ Suggested cleanup
<button type="button" aria-label="Close menu" onClick={() => setMobileMenuOpen(false)} - className="drawer-panel-enter fixed right-4 top-4 z-[60] inline-flex size-10 cursor-pointer items-center justify-center rounded-lg border border-scroll-300/25 bg-moloch-900/80 text-scroll-100 shadow-xl shadow-black/30 transition-all hover:bg-scroll-100/10 focus-visible:ring-2 focus-visible:ring-scroll-300" - style={{ pointerEvents: "auto", zIndex: 1000 }} + className="drawer-panel-enter pointer-events-auto fixed right-4 top-4 z-[1000] inline-flex size-10 cursor-pointer items-center justify-center rounded-lg border border-scroll-300/25 bg-moloch-900/80 text-scroll-100 shadow-xl shadow-black/30 transition-all hover:bg-scroll-100/10 focus-visible:ring-2 focus-visible:ring-scroll-300" >🤖 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/app-header.tsx` around lines 343 - 351, The close button in app-header has redundant inline styling that overrides the Tailwind z-index utility, making the existing z-[60] in the button’s className ineffective. Update the button markup so the stacking behavior is handled consistently through Tailwind in the same component (or remove the inline zIndex if it’s unnecessary), and keep pointerEvents only if it is actually required for the drawer close control.src/app/admin/quarters/[id]/transactions/page.tsx (2)
1092-1109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate transaction-explorer-link markup across cards.
The exact same anchor/fallback-span structure (link container,
truncatehash span,ExternalLinkicon, sr-only text) is repeated in the classifiedTransferCard, the unclassifiedTransferCard(~1201-1215), andManualLedgerEntryCard. Consider extracting a smallTransactionExplorerLink({ url, hash })helper to avoid tripling this markup and its future maintenance cost.♻️ Proposed extraction
+function TransactionExplorerLink({ + hash, + url, +}: { + hash: string; + url: string | null; +}) { + if (!url) { + return ( + <span className="font-mono text-xs text-muted-foreground"> + {formatHash(hash)} + </span> + ); + } + + return ( + <a + href={url} + target="_blank" + rel="noreferrer" + className="inline-flex min-w-0 items-center gap-1 rounded-md border border-border bg-background px-3 py-2 text-xs font-medium text-muted-foreground transition-colors hover:text-foreground" + > + <span className="truncate">{formatHash(hash)}</span> + <ExternalLink className="size-3" aria-hidden="true" /> + <span className="sr-only">Open transaction explorer</span> + </a> + ); +}Also applies to: 1362-1388
🤖 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/app/admin/quarters/`[id]/transactions/page.tsx around lines 1092 - 1109, The transaction explorer link markup is duplicated across multiple card components, making it harder to maintain. Extract the repeated anchor/fallback display into a shared helper such as TransactionExplorerLink used by TransferCard and ManualLedgerEntryCard, and pass in the url and hash so the truncate span, ExternalLink icon, and sr-only text are defined once.
1555-1580: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFragile nested
calc(100% - 0.25rem)width hack.Two nested elements independently apply
width/maxWidth: calc(100% - 0.25rem)via inline styles. The purpose of this specific 4px offset isn't documented, and stacking the same magic number on parent and child is easy to break during future edits. Consider replacing with standard Tailwind width/overflow utilities (w-full max-w-full overflow-hidden) or documenting why the offset is needed.🤖 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/app/admin/quarters/`[id]/transactions/page.tsx around lines 1555 - 1580, The layout in the transactions page uses a fragile nested `calc(100% - 0.25rem)` width hack on the outer wrapper and inner content block, which is hard to maintain. Update the section and its containing divs in `page.tsx` to use standard sizing/overflow utilities such as `w-full`, `max-w-full`, and `overflow-hidden` where possible, or clearly document the offset if it must remain. Keep the existing structure around the transactions header/quarter summary elements intact while removing duplicated magic-number sizing from these nested wrappers.
🤖 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/app/api/e2e/session/route.ts`:
- Around line 12-17: The session route’s E2E auth gate only checks NODE_ENV and
E2E_AUTH_ENABLED, so it can still be abused on non-local deployments. Update
isE2EAuthEnabled in the session route to also verify the request is local-only
(for example by validating host/origin against localhost or requiring a shared
secret) before allowing the POST/DELETE handlers to mint or clear sessions. Make
the same guard apply consistently in the route handler logic that processes the
session creation and deletion paths so non-local requests are rejected early.
In `@src/app/globals.css`:
- Line 145: Stylelint is flagging plain declarations that անմmediately follow
Tailwind `@apply` rules in globals.css; add the required blank line before each
affected declaration so the declaration-empty-line-before rule is satisfied.
Update the specific blocks around box-sizing: border-box and each overflow-wrap:
anywhere occurrence after `@apply` in the same stylesheet, keeping the existing
semantics but separating the declaration groups consistently.
- Around line 245-251: The mobile table labeling in the `.mobile-card-table`
styles relies on `td::before` generated content while hiding `thead`, which can
leave screen readers without column names. Update the table markup used by the
components behind these styles so each `td` includes the label text in the DOM
(for example via an `sr-only` span or equivalent), and keep the existing CSS
only for visual presentation. Use the `.mobile-card-table` selectors in
globals.css to locate the affected responsive table behavior.
In `@src/app/proposals/page.tsx`:
- Around line 84-85: The wrapper around the table in the proposals page uses
`mobile-card-table-lg` but removes padding too early with `md:p-0`, creating a
breakpoint mismatch. Update the surrounding card container for this table to
keep its padding until the same `lg` breakpoint used by `mobile-card-table-lg`,
matching the pattern used in `TransactionsPage` so the stacked-card layout and
outer card styling transition together.
In `@src/components/app-header.tsx`:
- Around line 299-354: The mobile drawer in app-header.tsx needs proper modal
accessibility behavior. Update the mobile menu container/aside to use dialog
semantics with aria-modal, move initial focus into the drawer when
mobileMenuOpen becomes true (prefer the close button via a ref), trap Tab
navigation inside the open drawer, and restore focus to the hamburger trigger
when it closes. Use the existing mobile menu state and the drawer button/aside
elements in app-header.tsx to wire this behavior.
---
Nitpick comments:
In `@src/app/admin/quarters/`[id]/transactions/page.tsx:
- Around line 1092-1109: The transaction explorer link markup is duplicated
across multiple card components, making it harder to maintain. Extract the
repeated anchor/fallback display into a shared helper such as
TransactionExplorerLink used by TransferCard and ManualLedgerEntryCard, and pass
in the url and hash so the truncate span, ExternalLink icon, and sr-only text
are defined once.
- Around line 1555-1580: The layout in the transactions page uses a fragile
nested `calc(100% - 0.25rem)` width hack on the outer wrapper and inner content
block, which is hard to maintain. Update the section and its containing divs in
`page.tsx` to use standard sizing/overflow utilities such as `w-full`,
`max-w-full`, and `overflow-hidden` where possible, or clearly document the
offset if it must remain. Keep the existing structure around the transactions
header/quarter summary elements intact while removing duplicated magic-number
sizing from these nested wrappers.
In `@src/components/app-header.tsx`:
- Around line 343-351: The close button in app-header has redundant inline
styling that overrides the Tailwind z-index utility, making the existing z-[60]
in the button’s className ineffective. Update the button markup so the stacking
behavior is handled consistently through Tailwind in the same component (or
remove the inline zIndex if it’s unnecessary), and keep pointerEvents only if it
is actually required for the drawer close control.
🪄 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: 3feff739-b1a7-42e9-94d9-5dce399dfd2d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
.env.example.gitignoreREADME.mdeslint.config.mjspackage.jsonplaywright.config.tssrc/app/admin/providers/page.tsxsrc/app/admin/quarters/[id]/transactions/classification-linked-fields.tsxsrc/app/admin/quarters/[id]/transactions/page.tsxsrc/app/admin/treasury-accounts/page.tsxsrc/app/api/e2e/session/route.tssrc/app/globals.csssrc/app/membership/page.tsxsrc/app/page.tsxsrc/app/proposals/page.tsxsrc/app/raids/page.tsxsrc/app/reports/page.tsxsrc/app/reports/quarters/[id]/page.tsxsrc/app/rips/page.tsxsrc/components/app-header.tsxsrc/components/auth/wallet-connect.tsxsrc/components/ui/button.tsxtests/e2e/responsive-dashboard.spec.ts
Member
Author
|
Addressed the review feedback in d2f5855. What changed:
Verified locally:
|
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
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.
This pull request introduces Playwright-based end-to-end (E2E) testing to the project, including all necessary configuration, scripts, and documentation for running E2E tests across multiple device viewports. It also makes minor UI improvements for better accessibility and responsive design in admin pages. The most important changes are grouped below:
End-to-End Testing Infrastructure:
@playwright/test), updatespnpm-lock.yaml, and provides install/test scripts inpackage.jsonfor running E2E tests. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]playwright.config.ts) to run tests in parallel across mobile, tablet, and desktop viewports, and to launch the Next.js dev server with E2E mock authentication enabled..env.exampleto include a newE2E_AUTH_ENABLEDvariable for local mock session support during E2E tests.README.md, including installation, running tests, and environment requirements.Admin UI Accessibility & Responsiveness:
src/app/admin/providers/page.tsxfor better accessibility and mobile responsiveness by adding semantic table attributes and updating the container styling. [1] [2] [3] [4] [5] [6] [7]src/app/admin/quarters/[id]/transactions/classification-linked-fields.tsxto be full-width and responsive for improved usability. (src/app/admin/quarters/[id]/transactions/classification-linked-fields.tsxL149-R149, src/app/admin/quarters/[id]/transactions/classification-linked-fields.tsxL182-R182, src/app/admin/quarters/[id]/transactions/classification-linked-fields.tsxL227-R227)Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests