Skip to content

Commit 1cb99ba

Browse files
committed
fix(webapp): billing alerts review fixes and accessible copy affordance
- getAlertPreviewLimitCents takes the real percentage mode from BillingAlertsSection instead of inferring it from threshold conversion, so absolute-dollar default alerts are no longer read as percentages (regression test added) - CopyableText renders a real button with an aria-label, visible on focus and coarse pointers, not only on mouse hover - default billing alert seeding only writes when no alerts exist, so a slow seed can't clobber the user's first alert edit after the timeout
1 parent 722e240 commit 1cb99ba

5 files changed

Lines changed: 86 additions & 35 deletions

File tree

apps/webapp/app/components/billing/BillingAlertsSection.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,8 @@ export function BillingAlertsSection({
136136
const alertPreviewLimitCents = getAlertPreviewLimitCents(
137137
alerts,
138138
effectiveLimitCents,
139-
planLimitCents
139+
planLimitCents,
140+
isPercentageMode
140141
);
141142
const maxAlerts = isPercentageMode ? MAX_PERCENTAGE_ALERTS : MAX_ABSOLUTE_ALERTS;
142143

apps/webapp/app/components/billing/billingAlertsFormat.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -313,15 +313,25 @@ function percentageAlertAmountMatches(
313313
return amountCents === effectiveLimitCents || amountCents === planLimitCents;
314314
}
315315

316-
/** Cents base for dollar preview when displaying saved percentage alerts. */
316+
/**
317+
* Cents base for dollar preview when displaying saved percentage alerts.
318+
* `isPercentageMode` must come from the billing limit mode, not from the alert
319+
* levels: absolute dollar alerts (e.g. the seeded defaults) also convert to
320+
* non-empty UI thresholds and must not be treated as percentages of the limit.
321+
*/
317322
export function getAlertPreviewLimitCents(
318323
alerts: BillingAlertsFormData,
319324
effectiveLimitCents: number,
320-
planLimitCents: number
325+
planLimitCents: number,
326+
isPercentageMode: boolean
321327
): number {
322328
const amountCents = getSavedAlertAmountCents(alerts);
323329
// Percentages always apply to the current limit, not the base stored at last save.
324-
if (amountCents > 0 && percentageAlertLevelsToUiThresholds(alerts.alertLevels).length > 0) {
330+
if (
331+
isPercentageMode &&
332+
amountCents > 0 &&
333+
percentageAlertLevelsToUiThresholds(alerts.alertLevels).length > 0
334+
) {
325335
return effectiveLimitCents;
326336
}
327337
if (percentageAlertAmountMatches(amountCents, effectiveLimitCents, planLimitCents)) {

apps/webapp/app/components/primitives/CopyableText.tsx

Lines changed: 27 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ export function CopyableText({
1212
asChild,
1313
variant,
1414
hideTooltip,
15+
ariaLabel,
1516
}: {
1617
value: string;
1718
copyValue?: string;
@@ -24,18 +25,27 @@ export function CopyableText({
2425
* fire Radix's global "one tooltip open at a time" close and dismiss the parent.
2526
*/
2627
hideTooltip?: boolean;
28+
/** Accessible label for the copy button. Defaults to "Copy". */
29+
ariaLabel?: string;
2730
}) {
2831
const [isHovered, setIsHovered] = useState(false);
2932
const { copy, copied } = useCopy(copyValue ?? value);
3033

3134
const resolvedVariant = variant ?? "icon-right";
3235

3336
if (resolvedVariant === "icon-right") {
37+
// Real button semantics so keyboard and touch users can discover and trigger copying.
38+
// The affordance is revealed on row hover, keyboard focus, and coarse (touch) pointers.
3439
const iconButton = (
35-
<span
40+
<button
41+
type="button"
42+
onClick={copy}
43+
onMouseDown={(e) => e.stopPropagation()}
44+
aria-label={copied ? "Copied!" : (ariaLabel ?? "Copy")}
3645
className={cn(
37-
"ml-1 flex size-6 items-center justify-center rounded border border-border-bright bg-background-hover",
46+
"absolute -right-6 top-0 z-10 flex size-6 items-center justify-center rounded border border-border-bright bg-background-hover font-sans",
3847
asChild && "p-1",
48+
"pointer-events-none opacity-0 transition-opacity group-hover:pointer-events-auto group-hover:opacity-100 focus-visible:opacity-100 [@media(pointer:coarse)]:pointer-events-auto [@media(pointer:coarse)]:opacity-100",
3949
copied
4050
? "text-green-500"
4151
: "text-text-dimmed hover:border-border-bright hover:bg-background-raised hover:text-text-bright"
@@ -46,35 +56,24 @@ export function CopyableText({
4656
) : (
4757
<ClipboardIcon className="size-3.5" />
4858
)}
49-
</span>
59+
</button>
5060
);
5161

5262
return (
53-
<span
54-
className={cn("group relative inline-flex h-6 items-center", className)}
55-
onMouseLeave={() => setIsHovered(false)}
56-
>
57-
<span onMouseEnter={() => setIsHovered(true)}>{value}</span>
58-
<span
59-
onClick={copy}
60-
onMouseDown={(e) => e.stopPropagation()}
61-
className={cn(
62-
"absolute -right-6 top-0 z-10 size-6 font-sans",
63-
isHovered ? "flex" : "hidden"
64-
)}
65-
>
66-
{hideTooltip ? (
67-
iconButton
68-
) : (
69-
<SimpleTooltip
70-
button={iconButton}
71-
content={copied ? "Copied!" : "Copy"}
72-
className="font-sans"
73-
disableHoverableContent
74-
asChild={asChild}
75-
/>
76-
)}
77-
</span>
63+
<span className={cn("group relative inline-flex h-6 items-center", className)}>
64+
<span>{value}</span>
65+
{hideTooltip ? (
66+
iconButton
67+
) : (
68+
// asChild so the Radix trigger merges onto our button instead of nesting a button.
69+
<SimpleTooltip
70+
button={iconButton}
71+
content={copied ? "Copied!" : "Copy"}
72+
className="font-sans"
73+
disableHoverableContent
74+
asChild
75+
/>
76+
)}
7877
</span>
7978
);
8079
}

apps/webapp/app/models/organization.server.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { env } from "~/env.server";
1515
import { featuresForUrl } from "~/features.server";
1616
import { createApiKeyForEnv, createPkApiKeyForEnv, envSlug } from "./api-key.server";
1717
import {
18+
getBillingAlerts,
1819
getDefaultEnvironmentConcurrencyLimit,
1920
isBillingConfigured,
2021
setBillingAlert,
@@ -150,8 +151,8 @@ async function seedDefaultBillingAlerts(organizationId: string): Promise<void> {
150151
});
151152

152153
const [error] = await tryCatch(
153-
Promise.race([setBillingAlert(organizationId, buildDefaultBillingAlerts()), timeout]).finally(
154-
() => clearTimeout(timer)
154+
Promise.race([writeDefaultBillingAlertsIfUnset(organizationId), timeout]).finally(() =>
155+
clearTimeout(timer)
155156
)
156157
);
157158
if (error) {
@@ -162,6 +163,19 @@ async function seedDefaultBillingAlerts(organizationId: string): Promise<void> {
162163
}
163164
}
164165

166+
/**
167+
* Only writes defaults when the org has no alerts yet. A slow seed that finishes
168+
* after org creation returned would otherwise overwrite the user's first alert edit.
169+
*/
170+
async function writeDefaultBillingAlertsIfUnset(organizationId: string): Promise<void> {
171+
const existing = await getBillingAlerts(organizationId);
172+
if (existing && existing.alertLevels.length > 0) {
173+
return;
174+
}
175+
176+
await setBillingAlert(organizationId, buildDefaultBillingAlerts());
177+
}
178+
165179
export async function createEnvironment({
166180
organization,
167181
project,

apps/webapp/test/billingAlertsDefaults.test.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
import { describe, expect, it } from "vitest";
22
import { buildDefaultBillingAlerts } from "~/services/billingAlertsDefaults.server";
3-
import { ABSOLUTE_ALERT_BASE_CENTS } from "~/components/billing/billingAlertsFormat";
3+
import {
4+
ABSOLUTE_ALERT_BASE_CENTS,
5+
getAlertPreviewLimitCents,
6+
storedAlertsToThresholds,
7+
type BillingAlertsFormData,
8+
} from "~/components/billing/billingAlertsFormat";
49

510
describe("buildDefaultBillingAlerts", () => {
611
it("uses the absolute dollar base so alert levels read as dollar thresholds", () => {
@@ -23,4 +28,26 @@ describe("buildDefaultBillingAlerts", () => {
2328
first.alertLevels.push(9999);
2429
expect(second.alertLevels).toEqual([5, 100, 500, 1000, 2500]);
2530
});
31+
32+
it("does not treat the default absolute-dollar payload as percentages of the limit", () => {
33+
const defaults = buildDefaultBillingAlerts();
34+
const alerts: BillingAlertsFormData = {
35+
amount: defaults.amount / 100, // API cents -> stored dollars ($1 base)
36+
emails: defaults.emails ?? [],
37+
alertLevels: [...(defaults.alertLevels ?? [])],
38+
};
39+
40+
// Absolute (none) mode: levels stay dollar thresholds, not percentages of the limit.
41+
expect(storedAlertsToThresholds(alerts, "none", 50_000, 50_000)).toEqual([
42+
5, 100, 500, 1000, 2500,
43+
]);
44+
45+
// With the real percentage mode (false for absolute alerts) the preview must not fall
46+
// into the percentage branch, even though a level like 100 looks like a percent. Here a
47+
// limit matches the $1 base, so inferring percentages would wrongly return the limit
48+
// (50000) instead of the absolute base (100).
49+
expect(getAlertPreviewLimitCents(alerts, 50_000, ABSOLUTE_ALERT_BASE_CENTS, false)).toBe(
50+
ABSOLUTE_ALERT_BASE_CENTS
51+
);
52+
});
2653
});

0 commit comments

Comments
 (0)