fix(theme): improve send status badge contrast in dark mode#253
fix(theme): improve send status badge contrast in dark mode#253Junman140 merged 3 commits intoPi-Defi-world:mainfrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces new global CSS custom properties for status colors (success, warning, neutral) with light and dark mode variants, updates the badge styling in the send page to use these new theme properties via a refactored helper function, and downgrades eslint dev dependencies while removing the packageManager field. Changes
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly Related PRs
Poem
🚥 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)
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 |
|
@abimbolaalabi Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/send/page.tsx (1)
487-507:⚠️ Potential issue | 🟡 MinorSuccess dialog still uses hardcoded
green-*classes — inconsistent with the new status tokens.The PR is about improving dark-mode contrast for status badges, but the success icon circle at lines 489-490 still hardcodes
bg-green-100 dark:bg-green-900/text-green-600 dark:text-green-300instead of the new--status-success*tokens. For visual consistency between the "Pending" badge directly below it and the success chrome above, consider driving the icon container from the same tokens.🎨 Proposed fix
- <div className="rounded-full bg-green-100 dark:bg-green-900 p-4 mb-4"> - <Check className="h-8 w-8 text-green-600 dark:text-green-300" /> - </div> + <div className="rounded-full bg-[var(--color-status-success)] p-4 mb-4"> + <Check className="h-8 w-8 text-[var(--color-status-success-foreground)]" /> + </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/send/page.tsx` around lines 487 - 507, The success icon container still hardcodes green utility classes (bg-green-100 dark:bg-green-900 and text-green-600 dark:text-green-300) making it inconsistent with the new status tokens; update the DialogContent icon wrapper to use the same status tokens/styles as the badge (i.e., drive background and icon color from the status CSS tokens used by the badges) instead of hardcoded green classes—mirror how the badge is styled (see getStatusBadgeClassName("pending")) so the icon container uses the same --status-success* tokens or a helper that returns those token-based classes.
🧹 Nitpick comments (2)
package.json (1)
82-83: Missing trailing newline.POSIX convention and most linters/formatters expect a trailing newline at EOF.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 82 - 83, The file package.json is missing a trailing newline at EOF; add a single newline character after the final closing brace (the last "}" in package.json) so the file ends with a newline to satisfy POSIX and linters/formatters.app/send/page.tsx (1)
198-209: HoistgetStatusBadgeClassNameout of the component.The helper closes over nothing from
SendPagestate/props, so defining it inside the component re-creates the function on every render. Moving it to module scope (or a shared utility) avoids that and keeps the render body focused. The indentation is also inconsistent with the surrounding component body.♻️ Proposed refactor
-export default function SendPage() { +function getStatusBadgeClassName(status: string) { + switch (status) { + case "completed": + return "border-[var(--color-status-success-border)] bg-[var(--color-status-success)] text-[var(--color-status-success-foreground)]"; + case "pending": + return "border-[var(--color-status-warning-border)] bg-[var(--color-status-warning)] text-[var(--color-status-warning-foreground)]"; + case "failed": + return "border-destructive/40 bg-destructive/15 text-destructive"; + default: + return "border-[var(--color-status-neutral-border)] bg-[var(--color-status-neutral)] text-[var(--color-status-neutral-foreground)]"; + } +} + +export default function SendPage() {And remove the in-component definition at lines 198-209.
Also, since
@theme inlinealready exposes these as Tailwind color utilities, you could use the idiomatic v4 forms (e.g.bg-status-success text-status-success-foreground border-status-success-border) instead of the arbitrarybg-[var(...)]syntax — shorter and easier to grep.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/send/page.tsx` around lines 198 - 209, getStatusBadgeClassName is defined inside SendPage but closes over nothing, causing a new function to be recreated each render; move getStatusBadgeClassName to module scope (or a shared util) and remove the in-component definition so SendPage no longer recreates it on every render, updating all references in the component to use the hoisted function; while moving, optionally replace the long arbitrary var(...) Tailwind classes with the v4 shorthand (e.g. bg-status-success text-status-success-foreground border-status-success-border) to simplify and make the classes easier to grep.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Around line 68-83: Restore the top-level "packageManager" field to pin pnpm
(e.g., "pnpm@10.15.0+sha512...") so Corepack enforces a consistent pnpm version
across contributors/CI; in package.json add that field back at the root. Also
re-evaluate the eslint/@eslint/js version changes: if you intentionally
downgraded to "^9.7.0" keep it but add a short PR note explaining the decision,
otherwise update "eslint" and "@eslint/js" to the current stable (ESLint 10.x)
and prepare the required migration from eslintrc to flat config; ensure
"typescript-eslint" remains compatible with the chosen ESLint major.
---
Outside diff comments:
In `@app/send/page.tsx`:
- Around line 487-507: The success icon container still hardcodes green utility
classes (bg-green-100 dark:bg-green-900 and text-green-600 dark:text-green-300)
making it inconsistent with the new status tokens; update the DialogContent icon
wrapper to use the same status tokens/styles as the badge (i.e., drive
background and icon color from the status CSS tokens used by the badges) instead
of hardcoded green classes—mirror how the badge is styled (see
getStatusBadgeClassName("pending")) so the icon container uses the same
--status-success* tokens or a helper that returns those token-based classes.
---
Nitpick comments:
In `@app/send/page.tsx`:
- Around line 198-209: getStatusBadgeClassName is defined inside SendPage but
closes over nothing, causing a new function to be recreated each render; move
getStatusBadgeClassName to module scope (or a shared util) and remove the
in-component definition so SendPage no longer recreates it on every render,
updating all references in the component to use the hoisted function; while
moving, optionally replace the long arbitrary var(...) Tailwind classes with the
v4 shorthand (e.g. bg-status-success text-status-success-foreground
border-status-success-border) to simplify and make the classes easier to grep.
In `@package.json`:
- Around line 82-83: The file package.json is missing a trailing newline at EOF;
add a single newline character after the final closing brace (the last "}" in
package.json) so the file ends with a newline to satisfy POSIX and
linters/formatters.
🪄 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: e1fdee81-6a65-41f0-90a6-8ac87d6c977d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
app/globals.cssapp/send/page.tsxpackage.json
| "devDependencies": { | ||
| "@eslint/js": "^10.0.1", | ||
| "@eslint/js": "^9.7.0", | ||
| "@tailwindcss/postcss": "^4.1.9", | ||
| "@types/node": "^22", | ||
| "@types/react": "^19", | ||
| "@types/react-dom": "^19", | ||
| "eslint": "^10.2.0", | ||
| "eslint": "^9.7.0", | ||
| "eslint-plugin-react": "^7.37.5", | ||
| "globals": "^17.5.0", | ||
| "postcss": "^8.5", | ||
| "tailwindcss": "^4.1.9", | ||
| "tw-animate-css": "1.3.3", | ||
| "typescript": "^5", | ||
| "typescript-eslint": "^8.58.2" | ||
| }, | ||
| "packageManager": "pnpm@10.15.0+sha512.486ebc259d3e999a4e8691ce03b5cac4a71cbeca39372a9b762cb500cfdf0873e2cb16abe3d951b1ee2cf012503f027b98b6584e4df22524e0c7450d9ec7aa7b" | ||
| } | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is ESLint 10 released and stable as of April 2026, or is 9.x still the current major line?
💡 Result:
Yes, ESLint 10 is released and stable as of April 2026. The current stable version is 10.2.1, released on April 17, 2026. ESLint 10.0.0 was released on February 6, 2026, followed by several patch and minor releases including 10.0.1-10.0.3, 10.1.0, 10.2.0, and the latest 10.2.1. These are production releases with bug fixes, features, and documentation updates, indicating stability two months after the major release. The 9.x line (latest 9.39.4) receives only limited maintenance, while 10.x is the actively developed major version. Official sources confirm this: npm shows 10.2.1 as latest, GitHub releases list v10.2.1 as the most recent on Apr 17, and eslint.org blog announces v10.0.0 and subsequent 10.x releases.
Citations:
- 1: https://eslint.org/blog/2026/02/eslint-v10.0.0-released/
- 2: https://github.com/eslint/eslint/releases/tag/v10.0.0
- 3: https://github.com/eslint/eslint/releases
- 4: https://github.com/eslint/eslint/releases/tag/v10.0.1
- 5: https://eslint.org/blog/2026/04/eslint-v10.2.0-released/
- 6: https://github.com/eslint/eslint/releases/tag/v10.2.0
- 7: https://github.com/eslint/eslint/releases/tag/v10.2.1
- 8: https://registry.npmjs.org/eslint
- 9: https://www.npmjs.com/package/eslint
- 10: https://github.com/eslint/eslint/tree/latest
🏁 Script executed:
#!/bin/bash
# Check git history for packageManager field changes in package.json
git log -p --follow -S "packageManager" -- package.json | head -100Repository: Pi-Defi-world/acbu-frontend
Length of output: 53
🏁 Script executed:
#!/bin/bash
# Check for compatibility information about ESLint 9 and typescript-eslint 8.58.2
rg "typescript-eslint|@eslint/js" --type json -A 2 -B 2 package.jsonRepository: Pi-Defi-world/acbu-frontend
Length of output: 289
🌐 Web query:
typescript-eslint 8.58.2 ESLint 9.7.0 compatibility
💡 Result:
Yes, typescript-eslint version 8.58.2 is compatible with ESLint 9.7.0. The official typescript-eslint documentation states that it supports ESLint versions ^8.57.0 || ^9.0.0 || ^10.0.0. Since ESLint 9.7.0 falls within the ^9.0.0 range, it is fully supported. This peer dependency range is confirmed in the package details from the v8.58.0 release (patch versions like 8.58.2 maintain the same compatibility), and the dependency versions page explicitly lists this support as current.
Citations:
- 1: https://typescript-eslint.io/users/dependency-versions
- 2: https://github.com/typescript-eslint/typescript-eslint/releases/latest
- 3: https://redirect.github.com/typescript-eslint/typescript-eslint/compare/v8.57.2...v8.58.0
🌐 Web query:
ESLint 9.x vs 10.x breaking changes April 2026
💡 Result:
ESLint v10.0.0 was released on February 6, 2026, introducing several breaking changes from v9.x. No further major breaking changes have been reported as of April 23, 2026. Key breaking changes include: For users: - Node.js versions below v20.19.0, all v21.x, and v23.x are no longer supported. Requires Node.js ^20.19.0 || ^22.13.0 || >=24. - Complete removal of eslintrc configuration support (.eslintrc.*, package.json eslintConfig, ESLINT_USE_FLAT_CONFIG env var). Must use flat config (eslint.config.js). - New default configuration file lookup: starts from the linted file's directory upward (v10_config_lookup_from_file flag removed). - eslint:recommended updated with new rules enabled. - eslint-env comments now reported as errors; configure globals in flat config instead. - Jiti < v2.2.0 no longer supported for TypeScript configs. - Stylish formatter uses native styleText instead of chalk. - Rule changes: radix rule deprecates "always"/"as-needed" options; no-shadow-restricted-names reports globalThis by default; func-names schema stricter; no-invalid-regexp allowConstructorFlags requires unique items. - Glob patterns now support POSIX character classes. - JSX references now tracked for scope analysis. - name property added to core configs. For plugin/rule developers: - Removed deprecated rule context members (e.g., context.getCwd, context.getFilename). - Removed deprecated SourceCode methods. - Program AST node range spans entire source text. - RuleFixer methods require string 'text' arguments. - Stricter RuleTester assertions; no errors/output on valid cases. - ScopeManager must use addGlobals. - Removed LintMessage#nodeType and TestCaseError#type. Migration steps: 1. Upgrade Node.js if needed. 2. Migrate to flat config using npx @eslint/migrate-config. 3. Remove eslintrc files, eslint-env comments, legacy flags. 4. Update eslint:recommended if disabling new rules. 5. Test thoroughly, especially in monorepos and with plugins. Refer to official migration guide: https://eslint.org/docs/latest/use/migrate-to-10.0.0
Citations:
- 1: https://eslint.org/blog/2026/02/eslint-v10.0.0-released/
- 2: https://github.com/eslint/eslint/blob/main/docs/src/use/migrate-to-10.0.0.md
- 3: https://github.com/eslint/eslint/releases/tag/v10.0.0
- 4: https://eslint.org/docs/latest/use/migrate-to-10.0.0
Removing packageManager drops pnpm version pinning.
The top-level "packageManager": "pnpm@10.15.0+sha512..." field was what Corepack used to guarantee every contributor/CI ran the exact same pnpm version. Removing it means local installs and CI can now diverge on pnpm version, risking lockfile churn or subtly different resolutions.
Downgrade to ESLint 9.7.0 avoids mandatory migration but sacrifices long-term support.
Dropping eslint/@eslint/js to ^9.7.0 is unusual since ESLint 10.2.1 is the current stable release (April 2026). While typescript-eslint@^8.58.2 is compatible with both ESLint 9.x and 10.x, ESLint 10 introduced breaking changes requiring migration from eslintrc to flat config. If this downgrade is intentional to defer that migration, confirm it's a deliberate choice — ESLint 9.x will receive limited maintenance while 10.x remains the actively developed line.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 68 - 83, Restore the top-level "packageManager"
field to pin pnpm (e.g., "pnpm@10.15.0+sha512...") so Corepack enforces a
consistent pnpm version across contributors/CI; in package.json add that field
back at the root. Also re-evaluate the eslint/@eslint/js version changes: if you
intentionally downgraded to "^9.7.0" keep it but add a short PR note explaining
the decision, otherwise update "eslint" and "@eslint/js" to the current stable
(ESLint 10.x) and prepare the required migration from eslintrc to flat config;
ensure "typescript-eslint" remains compatible with the chosen ESLint major.
|
@abimbolaalabi resolve conflicts |
|
@Junman140 The conflict have been resolved. |
2 participants
Closes #219
Summary by CodeRabbit
Style
Chores