Skip to content

fix(frontend): keep billing limit banner above settings drawer and show plan exhausted card - #5693

Open
NicholasKissel wants to merge 2 commits into
mainfrom
fix/billing-banner-drawer-offset
Open

fix(frontend): keep billing limit banner above settings drawer and show plan exhausted card#5693
NicholasKissel wants to merge 2 commits into
mainfrom
fix/billing-banner-drawer-offset

Conversation

@NicholasKissel

Copy link
Copy Markdown
Member

Description

  • Fix the free plan overage banner rendering behind the settings drawer. The drawer offset is now derived from the same billing usage data as the banner instead of a CSS variable set from an effect.
  • Add a card at the top of the billing settings panel when a free plan project has reached its usage limit, naming the exhausted metrics with an upgrade button.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Loaded the dashboard with mocked billing responses at 85% and 477% usage and confirmed the banner sits above the drawer in both cases, the exhausted card only appears at 100% or more, and the OSS flavor without billing renders unchanged.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@railway-app

railway-app Bot commented Sep 10, 2026

Copy link
Copy Markdown

🚅 Deployed to the actors-pr-5693 environment in rivet-frontend

Service Status Web Updated
frontend-cloud 😴 Sleeping (View Logs) Web Sep 10, 2026 at 7:47 pm UTC
frontend-inspector 😴 Sleeping (View Logs) Web Sep 10, 2026 at 7:43 pm UTC
kitchen-sink 😴 Sleeping (View Logs) Web Sep 10, 2026 at 7:43 pm UTC
ladle ✅ Success (View Logs) Web Sep 10, 2026 at 7:38 pm UTC
website ❌ Build Failed (View Logs) Web Sep 10, 2026 at 7:35 pm UTC
mcp-hub ✅ Success (View Logs) Web Sep 10, 2026 at 7:10 pm UTC

@the-company-company the-company-company Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 1 medium-severity finding

Reviewed commit e2951f8.

// metered metrics and the compute budget). Paid plans just bill overage, so
// only the free plan gets the call-out. The names list which allotments ran
// out so the user knows what to look at in the table below.
const planExhausted = plan === "free" && usage.highestPercent >= 100;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Medium · Keep the exhausted-plan card in sync after an upgrade

The plan picker invalidates only the billing-details query on a successful plan change; the usage query that supplies usage.plan is left cached. After a free project upgrades, the banner will disappear from the refreshed details response, but this new card continues rendering from the cached usage.plan === "free" (and old usage) until a later refetch such as focus.

Invalidate the current-project billing-usage query along with billing details after changing a plan, or drive this card from the refreshed plan state.

root.style.removeProperty("--billing-banner-height");
};
}, [hidden]);
return {

@jog1t jog1t Sep 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of switching this back to be passed around as a business value, we should use useLayoutEffect, and fix the z-index issues, css property was fine since you're passing in on the CSS layer (its not part of the business logic, its translucent, do not need to be passed around)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 1ead3f8. Restored the CSS variable, set it from a useLayoutEffect, and moved the drawer to z-40 with the banner at z-[45] so the banner stacks above the drawer and below z-50 popovers and dialogs.

@claude

claude Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Review

Overall a focused fix: the layering/timing issue (banner vs. drawer top) and the new "plan exhausted" card are both reasonable, low-risk changes. A few things worth addressing before merge.

1. Duplicate formatList — reuse the existing utility
billing-panel.tsx adds a hand-rolled formatList (around line 295) that reimplements exactly what formatList in frontend/src/components/lib/formatter.ts already does via Intl.ListFormat({ type: "conjunction" }). This file already imports formatCurrency from that same module, so importing formatList from there instead costs nothing and removes the duplicate implementation.

2. usage.computeBudgetPercent — verify the field, or derive it locally for consistency
computeBudgetPercent does not appear anywhere else in the frontend (this is the only reference in the repo, and the type comes from the external @rivet-gg/cloud EE package, so I cannot confirm it exists). More importantly, this same function already computes billedCompute and computeCapUsd locally (from useBilledComputeCost()) to render the ComputeUsageRow progress bar. Deriving the "Compute exhausted" check from those same local values (e.g. computeCapUsd != null && billedCompute >= computeCapUsd) instead of a separate backend field would guarantee the exhausted-card list cannot drift from what the progress bar actually shows, and would avoid a silent bug if the field is actually undefined (undefined >= 100 is false, so "Compute" would quietly never appear in the list even when capped, defeating the point of the card).

3. Missing Ladle story for PlanExhaustedCard
Per frontend/CLAUDE.md, new UI components should ship a Ladle story unless doing so requires mocking route loaders/auth/data-provider stack. PlanExhaustedCard takes plain props (exhaustedNames: string[], onUpgrade: () => void) with no such dependency, so it is a cheap, good candidate that was skipped. Worth covering the empty-names fallback vs. the 1/2/3+ name list states.

4. Minor: effect-based cross-component layering (pre-existing pattern, not newly introduced)
The --billing-banner-height CSS variable set from useLayoutEffect in BillingLimitAlertInner and read via inline style in SettingsDrawer predates this PR, but this PR extends it (switches useEffect to useLayoutEffect, adds a transition-[top] on the consumer). frontend/CLAUDE.md asks to derive state during render instead of syncing via effects, and to check in with the user before adding one where unavoidable. Not a blocker since the pattern already existed and this fix is narrowly scoped, but worth a follow-up: since both the banner and drawer already have access to the same billing query (useHighestUsagePercent / the underlying data provider), the drawer could compute its own top offset directly in render, guarded the same way BillingLimitAlertGuard is, instead of going through a DOM style side-channel.

Nit
Some of the diff in billing-panel.tsx (import reordering, ComputeUsageRow ternary reflow) looks like unrelated formatter output rather than part of the fix. Fine to leave, just flagging so it is not mistaken for an intentional change.

Testing
No new tests, understandable for a CSS/layering fix, but see #3 above for the one component that would benefit from a story.

🤖 Generated with Claude Code

@the-company-company the-company-company Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found

Reviewed commit 1ead3f8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants