Skip to content

Commit d08a857

Browse files
committed
Merge upstream/main into feat/ds-015-018-icon-button-input-field-textarea
Resolves conflicts in numeric.tsx and button.test.tsx against PR SO4-Markets#445, which landed its own independent (and more complete) fix for the same NumericText regression this branch also fixed. Took upstream's version of numeric.tsx as-is: it keeps Numeric's original "danger"/"brand-long"/ "brand-short" roles under a LegacyNumericRole alias for backward compatibility, alongside a properly separated NumericText/numericTextVariants for the newer neutral/muted/positive/negative/warning/accent role set. Reverted this branch's role="warning" edits in TradeInfoRows.tsx and PositionsList.tsx back to role="danger" to match the restored LegacyNumericRole type Numeric now uses upstream. button.test.tsx conflict was just import ordering; kept both the sorted imports and the `screen` import upstream's new test needs.
2 parents 92a9bc2 + e15c6d6 commit d08a857

8 files changed

Lines changed: 233 additions & 102 deletions

File tree

apps/web/src/features/trade/components/positions/PositionsList.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ export function PositionsList({ onSelectPosition }: Props) {
153153
header: "Liq.",
154154
accessor: (p) => {
155155
const closeToLiq = Math.abs(p.markPrice - p.liquidationPrice) / p.markPrice <= 0.1
156-
return <Numeric value={p.liquidationPrice} format="usd" role={closeToLiq ? "warning" : "neutral"} />
156+
return <Numeric value={p.liquidationPrice} format="usd" role={closeToLiq ? "danger" : "neutral"} />
157157
},
158158
},
159159
{

apps/web/src/features/trade/components/trade-panel/TradeInfoRows.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ export function TradeInfoRows({
6262
<div className="min-w-0 space-y-1 overflow-x-hidden text-xs">
6363
<Row label="Entry price" value={estimatedEntryPrice > 0 ? <Numeric value={estimatedEntryPrice} format="usd" role="neutral" /> : "-"} />
6464
{tradeMode === "Limit" && <Row label="Limit price" value="-" />}
65-
<Row label="Liq. price" value={liquidationPrice > 0 ? <Numeric value={liquidationPrice} format="usd" role="warning" /> : "-"} />
65+
<Row label="Liq. price" value={liquidationPrice > 0 ? <Numeric value={liquidationPrice} format="usd" role="danger" /> : "-"} />
6666
<Row
6767
label="Funding"
6868
value={
@@ -72,7 +72,7 @@ export function TradeInfoRows({
7272
}
7373
/>
7474
<Row label="Position fee" value={<Numeric value={fees.positionFeeUsd} format="usd" role="neutral" />} />
75-
<Row label="Price impact" value={<Numeric value={priceImpactPct} format="pct" role={Math.abs(priceImpactPct) > 0.5 ? "warning" : "neutral"} />} />
75+
<Row label="Price impact" value={<Numeric value={priceImpactPct} format="pct" role={Math.abs(priceImpactPct) > 0.5 ? "danger" : "neutral"} />} />
7676
<ExecutionFeeRow value={executionFeeDisplay} />
7777
<div className="border-t border-border pt-1">
7878
<Row label="Total fees" value={<Numeric value={fees.totalFeesUsd} format="usd" role="neutral" />} bold />

packages/ui/src/components/button.test.tsx

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,27 @@
11
import { describe, expect, it } from "vitest"
2-
import { render } from "@testing-library/react"
2+
import { render, screen } from "@testing-library/react"
33
import { axe } from "vitest-axe"
4-
import { Button } from "./button"
4+
import { Button, buttonVariants } from "./button"
5+
6+
const VARIANTS = [
7+
"default",
8+
"outline",
9+
"secondary",
10+
"ghost",
11+
"destructive",
12+
"link",
13+
] as const
14+
15+
const SIZES = [
16+
"default",
17+
"xs",
18+
"sm",
19+
"lg",
20+
"icon",
21+
"icon-xs",
22+
"icon-sm",
23+
"icon-lg",
24+
] as const
525

626
describe("Button accessibility", () => {
727
it("has no accessibility violations", async () => {
@@ -26,3 +46,44 @@ describe("Button accessibility", () => {
2646
expect(results).toHaveNoViolations()
2747
})
2848
})
49+
50+
describe("Button variants and sizes", () => {
51+
it.each(VARIANTS)("renders the %s variant without violations", async (variant) => {
52+
const { container } = render(<Button variant={variant}>Action</Button>)
53+
expect(await axe(container)).toHaveNoViolations()
54+
})
55+
56+
it.each(SIZES)("renders the %s size without violations", async (size) => {
57+
const isIconSize = size.startsWith("icon")
58+
const { container } = render(
59+
isIconSize ? (
60+
<Button size={size} aria-label="Action">
61+
<svg viewBox="0 0 24 24" />
62+
</Button>
63+
) : (
64+
<Button size={size}>Action</Button>
65+
)
66+
)
67+
expect(await axe(container)).toHaveNoViolations()
68+
})
69+
70+
it("every variant defines default, hover, active, and disabled classes", () => {
71+
for (const variant of VARIANTS) {
72+
const className = buttonVariants({ variant })
73+
expect(className).toMatch(/hover:/)
74+
expect(className).toMatch(/active:/)
75+
}
76+
// disabled + focus-visible states live in the shared base classes.
77+
expect(buttonVariants({})).toMatch(/disabled:/)
78+
expect(buttonVariants({})).toMatch(/focus-visible:/)
79+
})
80+
81+
it("keeps the icon-only button's accessible name from aria-label, not the icon", () => {
82+
render(
83+
<Button size="icon" aria-label="Settings">
84+
<svg viewBox="0 0 24 24" />
85+
</Button>
86+
)
87+
expect(screen.getByRole("button", { name: "Settings" })).toBeInTheDocument()
88+
})
89+
})

packages/ui/src/components/button.tsx

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,17 @@ const buttonVariants = cva(
99
{
1010
variants: {
1111
variant: {
12-
default: "bg-primary text-primary-foreground hover:bg-primary/80",
12+
default:
13+
"bg-primary text-primary-foreground hover:bg-primary/80 active:not-aria-[haspopup]:bg-primary/70",
1314
outline:
14-
"border-border hover:bg-input/50 hover:text-foreground aria-expanded:bg-muted aria-expanded:text-foreground dark:bg-input/30",
15+
"border-border hover:bg-input/50 hover:text-foreground aria-expanded:bg-muted aria-expanded:text-foreground active:not-aria-[haspopup]:bg-input/70 dark:bg-input/30 dark:active:not-aria-[haspopup]:bg-input/50",
1516
secondary:
16-
"bg-secondary text-secondary-foreground hover:bg-secondary/80 aria-expanded:bg-secondary aria-expanded:text-secondary-foreground",
17+
"bg-secondary text-secondary-foreground hover:bg-secondary/80 aria-expanded:bg-secondary aria-expanded:text-secondary-foreground active:not-aria-[haspopup]:bg-secondary/70",
1718
ghost:
18-
"hover:bg-muted hover:text-foreground aria-expanded:bg-muted aria-expanded:text-foreground dark:hover:bg-muted/50",
19+
"hover:bg-muted hover:text-foreground aria-expanded:bg-muted aria-expanded:text-foreground active:not-aria-[haspopup]:bg-muted/70 dark:hover:bg-muted/50 dark:active:not-aria-[haspopup]:bg-muted/40",
1920
destructive:
20-
"bg-destructive/10 text-destructive hover:bg-destructive/20 focus-visible:border-destructive/40 focus-visible:ring-destructive/20 dark:bg-destructive/20 dark:hover:bg-destructive/30 dark:focus-visible:ring-destructive/40",
21-
link: "text-primary underline-offset-4 hover:underline",
21+
"bg-destructive/10 text-destructive hover:bg-destructive/20 focus-visible:border-destructive/40 focus-visible:ring-destructive/20 active:not-aria-[haspopup]:bg-destructive/30 dark:bg-destructive/20 dark:hover:bg-destructive/30 dark:focus-visible:ring-destructive/40 dark:active:not-aria-[haspopup]:bg-destructive/40",
22+
link: "text-primary underline-offset-4 hover:underline active:not-aria-[haspopup]:text-primary/70",
2223
},
2324
size: {
2425
default:
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import { describe, it, expect } from "vitest"
2+
import { render, screen } from "@testing-library/react"
3+
import { axe } from "vitest-axe"
4+
import { LoadingButton } from "./loading-button"
5+
6+
describe("LoadingButton", () => {
7+
it("is disabled and aria-busy while loading, with text content", async () => {
8+
const { container } = render(
9+
<LoadingButton isLoading loadingText="Confirming...">
10+
Stake
11+
</LoadingButton>
12+
)
13+
const button = screen.getByRole("button", { name: "Confirming..." })
14+
expect(button).toBeDisabled()
15+
expect(button).toHaveAttribute("aria-busy", "true")
16+
expect(await axe(container)).toHaveNoViolations()
17+
})
18+
19+
it("falls back to children as the loading label when loadingText is omitted", () => {
20+
render(<LoadingButton isLoading>Stake</LoadingButton>)
21+
expect(screen.getByRole("button", { name: "Stake" })).toBeInTheDocument()
22+
})
23+
24+
it("is not disabled and not aria-busy when not loading", () => {
25+
render(<LoadingButton>Stake</LoadingButton>)
26+
const button = screen.getByRole("button", { name: "Stake" })
27+
expect(button).not.toBeDisabled()
28+
expect(button).not.toHaveAttribute("aria-busy")
29+
})
30+
31+
it("keeps the width-reserving spinner slot present (but hidden) when not loading", () => {
32+
const { container } = render(<LoadingButton>Stake</LoadingButton>)
33+
const slot = container.querySelector('[aria-hidden="true"]')
34+
expect(slot).toBeInTheDocument()
35+
expect(slot).toHaveClass("invisible")
36+
})
37+
38+
it("supports an icon-only loading button, keeping its accessible name", async () => {
39+
const { container } = render(
40+
<LoadingButton isLoading size="icon" aria-label="Stake">
41+
<svg viewBox="0 0 24 24" />
42+
</LoadingButton>
43+
)
44+
const button = screen.getByRole("button", { name: "Stake" })
45+
expect(button).toBeDisabled()
46+
expect(button).toHaveAttribute("aria-busy", "true")
47+
expect(await axe(container)).toHaveNoViolations()
48+
})
49+
})

packages/ui/src/components/loading-button.tsx

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ type LoadingButtonProps = ComponentProps<typeof Button> & {
1313
* Button that owns its own pending presentation: disables itself, flags
1414
* `aria-busy` and swaps in a spinner. The spinner is `aria-hidden`, so the
1515
* accessible name is exactly `loadingText` (or the children).
16+
*
17+
* The spinner sits in a fixed-size slot that's always present (just hidden
18+
* via `invisible` when not loading), so entering the loading state never
19+
* changes the button's width.
1620
*/
1721
function LoadingButton({
1822
isLoading = false,
@@ -30,14 +34,16 @@ function LoadingButton({
3034
className={cn("gap-1.5", className)}
3135
{...props}
3236
>
33-
{isLoading ? (
34-
<>
35-
<Spinner />
36-
{loadingText ?? children}
37-
</>
38-
) : (
39-
children
40-
)}
37+
<span
38+
aria-hidden="true"
39+
className={cn(
40+
"inline-flex size-3 shrink-0 items-center justify-center",
41+
!isLoading && "invisible"
42+
)}
43+
>
44+
{isLoading && <Spinner />}
45+
</span>
46+
{isLoading ? (loadingText ?? children) : children}
4147
</Button>
4248
)
4349
}

0 commit comments

Comments
 (0)