feat(billing): add persistent funding banner with reusable banner variants - #3442
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (11)
📝 WalkthroughWalkthroughAdds a reusable ChangesFunding banner and top-app layout
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: ✨ 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 install timed out. The project may have too many dependencies for the sandbox. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3442 +/- ##
==========================================
- Coverage 72.62% 71.62% -1.00%
==========================================
Files 1167 1078 -89
Lines 29552 27228 -2324
Branches 7330 6873 -457
==========================================
- Hits 21463 19503 -1960
+ Misses 7104 6776 -328
+ Partials 985 949 -36
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
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)
apps/deploy-web/src/components/layout/Layout.tsx (1)
108-108: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDesktop sidebar offset should use the header height var.
mdDrawerClassNameis applied to the permanentmddrawer, but it still hardcodesmt-[97px]/h-[calc(100%-40px)]offhasBanner. If the banner wraps, the sidebar can drift out of sync with the actual header height.🤖 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/deploy-web/src/components/layout/Layout.tsx` at line 108, Update the mdDrawerClassName styling for the permanent desktop drawer in Layout so its top offset and height derive from the shared header-height variable rather than hardcoded values or only hasBanner. Keep the drawer aligned with the actual, potentially wrapped header height in all banner states.
🧹 Nitpick comments (1)
packages/ui/components/banner/banner.spec.tsx (1)
1-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood, meaningful assertions — one coverage gap worth adding.
Consider a test asserting that keyboard-activating the dismiss button (Enter/Space) while
onClickis also set does not invokeonClick— this is exactly the scenario the bubbling bug inbanner.tsx(Lines 66-71) breaks today.🤖 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 `@packages/ui/components/banner/banner.spec.tsx` around lines 1 - 63, Add a Banner test covering keyboard activation of the dismiss control when both onClick and onClose are provided. In the existing dismiss-control interaction coverage, trigger Enter and Space on the button named “Dismiss,” assert onClose is invoked appropriately, and verify the parent onClick handler is never called.
🤖 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 `@packages/ui/components/banner/banner.tsx`:
- Around line 36-71: Update activateOnEnterOrSpace to ignore keydown events
whose target is the nested dismiss button, while preserving Enter/Space
activation for the banner itself and other valid targets. Keep the existing
dismiss stopPropagation behavior unchanged.
---
Outside diff comments:
In `@apps/deploy-web/src/components/layout/Layout.tsx`:
- Line 108: Update the mdDrawerClassName styling for the permanent desktop
drawer in Layout so its top offset and height derive from the shared
header-height variable rather than hardcoded values or only hasBanner. Keep the
drawer aligned with the actual, potentially wrapped header height in all banner
states.
---
Nitpick comments:
In `@packages/ui/components/banner/banner.spec.tsx`:
- Around line 1-63: Add a Banner test covering keyboard activation of the
dismiss control when both onClick and onClose are provided. In the existing
dismiss-control interaction coverage, trigger Enter and Space on the button
named “Dismiss,” assert onClose is invoked appropriately, and verify the parent
onClick handler is never called.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1df83de7-fa38-445a-9a82-ff2eb6c8f868
📒 Files selected for processing (12)
apps/deploy-web/src/components/layout/FundingBanner/FundingBanner.spec.tsxapps/deploy-web/src/components/layout/FundingBanner/FundingBanner.tsxapps/deploy-web/src/components/layout/Layout.tsxapps/deploy-web/src/components/layout/Nav.tsxapps/deploy-web/src/components/layout/TopBanner.tsxapps/deploy-web/src/hooks/useHasCreditCardBanner.tsapps/deploy-web/src/hooks/useHasFundingBanner.spec.tsapps/deploy-web/src/hooks/useHasFundingBanner.tsapps/deploy-web/src/hooks/useTopBanner.tsxpackages/ui/components/banner/banner.spec.tsxpackages/ui/components/banner/banner.tsxpackages/ui/components/index.tsx
💤 Files with no reviewable changes (1)
- apps/deploy-web/src/hooks/useHasCreditCardBanner.ts
2338a8c to
1b38515
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/deploy-web/src/components/layout/TopBanner.tsx`:
- Line 16: Update the effect in TopBanner to clear the isUpgrading state when
date is absent before returning. Preserve the existing date-based upgrade
handling for non-empty dates.
- Around line 107-108: Update the sidebar layout logic associated with hasBanner
to use the --app-header-height CSS variable for its top margin and remaining
height instead of fixed mt-[97px] and h-[calc(100%-40px)] values. Preserve the
existing non-banner layout and ensure the desktop drawer remains positioned
below the rendered header, including when FundingBanner wraps.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2416bce0-2aea-44b3-9f71-839bb24bc72b
📒 Files selected for processing (13)
apps/deploy-web/src/components/layout/FundingBanner/FundingBanner.spec.tsxapps/deploy-web/src/components/layout/FundingBanner/FundingBanner.tsxapps/deploy-web/src/components/layout/Layout.tsxapps/deploy-web/src/components/layout/Nav.tsxapps/deploy-web/src/components/layout/TopBanner.spec.tsxapps/deploy-web/src/components/layout/TopBanner.tsxapps/deploy-web/src/hooks/useHasCreditCardBanner.tsapps/deploy-web/src/hooks/useHasFundingBanner.spec.tsapps/deploy-web/src/hooks/useHasFundingBanner.tsapps/deploy-web/src/hooks/useTopBanner.tsxpackages/ui/components/banner/banner.spec.tsxpackages/ui/components/banner/banner.tsxpackages/ui/components/index.tsx
💤 Files with no reviewable changes (1)
- apps/deploy-web/src/hooks/useHasCreditCardBanner.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/deploy-web/src/components/layout/FundingBanner/FundingBanner.tsx
- apps/deploy-web/src/components/layout/Nav.tsx
- packages/ui/components/banner/banner.spec.tsx
- apps/deploy-web/src/hooks/useHasFundingBanner.spec.ts
- packages/ui/components/banner/banner.tsx
- apps/deploy-web/src/hooks/useTopBanner.tsx
- apps/deploy-web/src/components/layout/Layout.tsx
- apps/deploy-web/src/hooks/useHasFundingBanner.ts
…iants Introduce a theme-aware Banner primitive in @akashnetwork/ui with toast-like variants (error/warning/success/info/neutral) and optional click and dismiss affordances. Replace the credit-card top banner with a funding banner shown to signed-in trial users who haven't funded yet. Clicking it opens the Add Credits sheet, and it hides once the account is funded. Move the existing network-status, maintenance, and announcement banners onto the shared Banner with semantic variants.
1b38515 to
b49420a
Compare
Why
Closes CON-321
Ref CON-679
Trial users only had a path to fund their account from a gated template tile or the billing page. This adds a persistent funding prompt in the global navigation so an unfunded user can start their first purchase from anywhere — the funding-based unlock that improves trial-to-paid conversion.
Along the way, the app-wide banners were each bespoke
<div>s with ad-hoc styling and duplicated positioning. This standardizes them into one reusable, theme-aware primitive with semantic variants, so every global banner shares a consistent look.What
@akashnetwork/ui— newBannerprimitivecva-based variants (error/warning/success/info/neutral), theme-aware for light and dark, with optional click (»affordance, keyboard-activatable) and dismiss (✕) controls. Layout-agnostic.deploy-web — funding banner (replaces the credit-card banner)
FundingBanner(infovariant, Figma copy) shown to signed-in trial users who haven't funded yet, via a newuseHasFundingBannerhook. Hidden once the account is funded, and suppressed inside the stripped onboarding deploy funnel (the configure page for first-time users) where the page owns its own funding / GPU-unlock UI. Clicking it opens the Add Credits sheet.CreditCardBanner/useHasCreditCardBannerare removed.deploy-web — existing banners + layout
error, maintenance →warning, announcements →neutral, all now rendered through the sharedBanner.--app-header-height), and the content offset tracks it, so the banner stays fully visible and wraps correctly on small screens.Verification
Banner(ui),FundingBanner,useHasFundingBanner(visibility truth table incl. the onboarding-funnel case), plus the existingTopBanner/useTopBannerspecs.tsc --noEmitintroduces no new errors over the pre-existing baseline (verified by stashing the change and diffing the erroring-file set — 81 pre-existing, none in the changed files).Summary by CodeRabbit
New Features
Bug Fixes
Tests