Fix light mode active request payload modal - #1714
diegosouzapw merged 4 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new --color-card CSS variable across light and dark themes and updates the styling of the ActiveRequestsPanel component. The changes replace bg-card with bg-surface and adjust the modal's backdrop blur, borders, and shadow effects. Feedback was provided regarding the use of hardcoded values and custom shadows in the modal, suggesting a return to theme-aware variables and standard Tailwind scales to ensure design consistency and maintainability.
| <div className="fixed inset-0 z-50 flex items-center justify-center bg-black/55 px-4 py-6 backdrop-blur-[2px]"> | ||
| <div className="flex max-h-[85vh] w-full max-w-5xl flex-col overflow-hidden rounded-2xl border border-black/10 bg-surface shadow-[0_24px_80px_rgba(15,23,42,0.24)] dark:border-white/10 dark:shadow-2xl"> |
There was a problem hiding this comment.
The modal styling introduces several arbitrary values and inconsistencies with the application's design system. Using border-black/10 and a custom hardcoded shadow deviates from the theme variables (like border-border and --shadow-elevated) and other modals in the app (e.g., AuditLogTab.tsx). This makes the UI less consistent and harder to maintain if theme colors are updated. Additionally, the custom shadow is only applied in light mode, creating a different sense of depth compared to the shadow-2xl used in dark mode. Consider using standard Tailwind scales and theme-aware variables to ensure a unified design language.
| <div className="fixed inset-0 z-50 flex items-center justify-center bg-black/55 px-4 py-6 backdrop-blur-[2px]"> | |
| <div className="flex max-h-[85vh] w-full max-w-5xl flex-col overflow-hidden rounded-2xl border border-black/10 bg-surface shadow-[0_24px_80px_rgba(15,23,42,0.24)] dark:border-white/10 dark:shadow-2xl"> | |
| <div className="fixed inset-0 z-50 flex items-center justify-center bg-black/55 px-4 py-6 backdrop-blur-sm"> | |
| <div className="flex max-h-[85vh] w-full max-w-5xl flex-col overflow-hidden rounded-2xl border border-border bg-surface shadow-2xl"> |
There was a problem hiding this comment.
Pull request overview
Fixes light-mode readability issues in the Active Request payload panel/modal by ensuring theme tokens and modal surfaces are opaque and consistently themed.
Changes:
- Adds a missing
--color-cardtheme token for consistentbg-cardresolution across light/dark themes. - Updates
ActiveRequestsPanelcontainer and modal to use opaquebg-surfacebackgrounds, and refines overlay/border/shadow styling. - Adds an integration wiring test to prevent regressions in the theme surface/modal class usage and token presence.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/integration/integration-wiring.test.ts | Adds a regression test asserting --color-card presence and that the modal/panel use bg-surface (and not translucent bg-card/...). |
| src/shared/components/ActiveRequestsPanel.tsx | Switches panel + modal surfaces to bg-surface, adjusts overlay opacity/blur, and tunes border/shadow for light mode. |
| src/app/globals.css | Introduces --color-card for both light and dark theme definitions and exposes it in the Tailwind @theme block. |
| it("keeps the active request payload modal on opaque theme surfaces", () => { | ||
| const activeRequests = readProjectFile("src/shared/components/ActiveRequestsPanel.tsx"); | ||
| const globals = readProjectFile("src/app/globals.css"); | ||
|
|
||
| assert.ok(activeRequests, "ActiveRequestsPanel should exist"); | ||
| assert.ok(globals, "globals.css should exist"); | ||
| assert.match(globals, /--color-card:\s+#ffffff/); | ||
| assert.match(globals, /--color-card:\s+#161b22/); | ||
| assert.match(globals, /--color-card:\s+var\(--color-card\)/); | ||
| assert.match(activeRequests, /rounded-xl border border-border bg-surface/); | ||
| assert.match(activeRequests, /backdrop-blur-sm/); | ||
| assert.match( | ||
| activeRequests, | ||
| /rounded-2xl[^"]*border border-border[^"]*bg-surface[^"]*shadow-2xl/ |
There was a problem hiding this comment.
The repo’s coverage gate is enforced via npm run test:coverage (c8 --check-coverage at 60%+). This PR changes production code in src/, so please run that command (or CI equivalent) and ensure it passes; if it fails, add/update tests in this PR until the gate is green.
|
Thanks @rdself for fixing the light mode payload modal! 🎉 Adding the missing card token and switching to opaque surfaces with backdrop-blur was the right call. Merged into release/v3.7.3. |
Integrated into release/v3.7.3 — fixes light mode payload modal transparency
Integrated into release/v3.7.3 — fixes light mode payload modal transparency
Integrated into release/v3.7.3 — fixes light mode payload modal transparency
Summary
cardtheme token so existingbg-cardutilities resolve consistently.Validation
prettier --check src/app/globals.css src/shared/components/ActiveRequestsPanel.tsxeslint src/shared/components/ActiveRequestsPanel.tsx