From bc6d8a198fd38db0f7da920506f6c4e42487afe3 Mon Sep 17 00:00:00 2001 From: Akinola Akinbusola Date: Sat, 17 Oct 2026 21:44:18 +0100 Subject: [PATCH] fix: enhance accessibility across trade components - Refactor OrderBookPanel to implement a tablist for view switching with keyboard navigation. - Update RecentTradesTape to ensure scrollable region is focusable and properly labeled. - Improve leverage slider accessibility by adding aria-label. - Modify Table component to become a focusable region when overflowing, with appropriate aria attributes. - Document trigger parts usage in the UI library to prevent nested interactive elements. - Add tests for accessibility improvements in Table and TooltipTrigger components. --- .../853-857-design-system-accessibility.md | 8 ++ .../unreleased/858-trade-accessibility.md | 8 ++ .../unreleased/859-indexer-configuration.md | 8 ++ README.md | 37 ++++++ apps/web/.env.example | 12 +- apps/web/.env.mainnet | 6 + apps/web/.env.testnet | 8 ++ apps/web/src/app/config/indexer.test.ts | 47 +++++++ apps/web/src/app/config/indexer.ts | 33 ++++- .../design-system/design-system-page.tsx | 6 +- .../gallery/components/gallery-page.tsx | 28 ++++- .../trade/components/chart/TVChart.tsx | 7 +- .../chart/TVChartContainer.test.tsx | 32 ++++- .../components/chart/TVChartContainer.tsx | 41 +++--- .../orderbook/OrderBookPanel.test.tsx | 102 ++++++++++++--- .../components/orderbook/OrderBookPanel.tsx | 118 ++++++++++++------ .../orderbook/RecentTradesTape.test.tsx | 25 ++++ .../components/orderbook/RecentTradesTape.tsx | 12 +- .../positions/OrderExecutionFrozenBanner.tsx | 4 +- .../components/trade-panel/LeverageSlider.tsx | 1 + .../trade/store/chart-preferences-store.ts | 12 +- .../components/NetworkMismatchBanner.tsx | 4 +- apps/web/src/shared/hooks/useTxSubmit.tsx | 6 +- packages/ui/CONTRIBUTING.md | 38 ++++++ packages/ui/README.md | 19 +++ packages/ui/src/components/slider.test.tsx | 37 ++++++ packages/ui/src/components/slider.tsx | 9 ++ packages/ui/src/components/table.test.tsx | 79 ++++++++++++ packages/ui/src/components/table.tsx | 48 ++++++- .../src/components/tooltip-trigger.test.tsx | 40 ++++++ 30 files changed, 732 insertions(+), 103 deletions(-) create mode 100644 .changelog/unreleased/853-857-design-system-accessibility.md create mode 100644 .changelog/unreleased/858-trade-accessibility.md create mode 100644 .changelog/unreleased/859-indexer-configuration.md create mode 100644 apps/web/src/app/config/indexer.test.ts create mode 100644 packages/ui/src/components/table.test.tsx create mode 100644 packages/ui/src/components/tooltip-trigger.test.tsx diff --git a/.changelog/unreleased/853-857-design-system-accessibility.md b/.changelog/unreleased/853-857-design-system-accessibility.md new file mode 100644 index 00000000..56e3bdbb --- /dev/null +++ b/.changelog/unreleased/853-857-design-system-accessibility.md @@ -0,0 +1,8 @@ +--- +type: fixed +area: docs +pr: 853 +breaking: false +--- + +The component gallery no longer nests a button inside the tooltip trigger's own button, which caused a hydration error and an axe `nested-interactive` violation; the `render` pattern it should have used is now documented in the `@workspace/ui` docs. The gallery's slider, progress examples and scroll areas, and the design-system page's theme and viewport pickers, all have accessible names. diff --git a/.changelog/unreleased/858-trade-accessibility.md b/.changelog/unreleased/858-trade-accessibility.md new file mode 100644 index 00000000..2615a1a7 --- /dev/null +++ b/.changelog/unreleased/858-trade-accessibility.md @@ -0,0 +1,8 @@ +--- +type: fixed +area: trade +pr: 858 +breaking: false +--- + +The order book's view switcher is now a real tab list with arrow-key navigation, the price chart is announced as a region named after the market (for example "BTC/USD") instead of an image named after a raw contract address, the recent-trades list can be scrolled from the keyboard, and the leverage slider has an accessible name. diff --git a/.changelog/unreleased/859-indexer-configuration.md b/.changelog/unreleased/859-indexer-configuration.md new file mode 100644 index 00000000..d1c1fcba --- /dev/null +++ b/.changelog/unreleased/859-indexer-configuration.md @@ -0,0 +1,8 @@ +--- +type: changed +area: docs +pr: 859 +breaking: false +--- + +`VITE_INDEXER_GRAPHQL_URL` is now documented in the README and in every env file, including how to run the SubQuery indexer locally and what degrades without it. With no indexer configured the app logs its warning once per runtime instead of on every route. diff --git a/README.md b/README.md index 7b3faf20..948155f3 100644 --- a/README.md +++ b/README.md @@ -160,6 +160,43 @@ bun run --cwd apps/s03-indexer start Run `bun run check:integration` before pushing to mirror the CI matrix. +### Indexed history (SubQuery) + +Position and order history, executed fills, and fee/funding activity are read +from the SubQuery indexer in `apps/s03-indexer`, not from the chain. The web app +finds it through a single variable: + +| Variable | Scope | Default | +|---|---|---| +| `VITE_INDEXER_GRAPHQL_URL` | `apps/web` | unset | + +**No hosted indexer is deployed for testnet or mainnet yet**, so +`apps/web/.env.testnet` and `apps/web/.env.mainnet` ship with the variable +commented out. With it unset the indexer is disabled: the app starts normally, +logs a single `Indexer disabled: …` warning, and the affected panels — Positions, +Orders, Order history, Trades, Funding & Fees — say the data is unavailable +instead of rendering empty tables. + +To develop against indexed data, run the stack locally: + +```bash +# codegen + build + docker compose up (Postgres, indexer node, GraphQL engine). +# Requires Docker. +bun run indexer:dev +``` + +The GraphQL engine and its playground listen on +[http://localhost:3000](http://localhost:3000) — the same port the web dev +server uses — so run the web app elsewhere and point it at the indexer: + +```bash +VITE_INDEXER_GRAPHQL_URL=http://localhost:3000 \ + bun run --cwd apps/web dev -- --port 3001 +``` + +See [`apps/s03-indexer/README.md`](./apps/s03-indexer/README.md) for the full +indexer workflow. + ### Running the development server ```bash diff --git a/apps/web/.env.example b/apps/web/.env.example index 2756ac3f..dd1e6702 100644 --- a/apps/web/.env.example +++ b/apps/web/.env.example @@ -27,9 +27,15 @@ VITE_HORIZON_URL=https://horizon-testnet.stellar.org # ── SubQuery Indexer ────────────────────────────────────────────────────────── # SubQuery GraphQL endpoint for indexed historical data. -# Optional — when not set, indexer is disabled and app uses degraded fallback behavior. -# Local development default: http://localhost:3000 -# Testnet default: (add testnet indexer URL when available) +# Optional — when not set the indexer is disabled, and position/order history, +# executed fills and fee activity show an explicit "unavailable" state instead +# of data. See "Indexed history (SubQuery)" in the root README. +# +# There is no hosted endpoint yet for either network, so this is commented out +# in .env.testnet and .env.mainnet. Run the stack locally with +# `bun run --cwd apps/s03-indexer start` and point this at it. The GraphQL +# engine also listens on 3000, so start the web app on another port +# (`bun run --cwd apps/web dev -- --port 3001`). # VITE_INDEXER_GRAPHQL_URL=http://localhost:3000 # ── Contract addresses ──────────────────────────────────────────────────────── diff --git a/apps/web/.env.mainnet b/apps/web/.env.mainnet index f478d97c..4dfe2998 100644 --- a/apps/web/.env.mainnet +++ b/apps/web/.env.mainnet @@ -15,6 +15,12 @@ VITE_HORIZON_URL=https://horizon.stellar.org # Optional — remove to fall back to GMX infra # VITE_ORACLE_URL=https://arbitrum-api.gmxinfra.io +# ── SubQuery indexer ────────────────────────────────────────────────────────── +# Set this to the deployed mainnet SubQuery GraphQL endpoint. While unset the +# indexer is disabled and the history panels say so instead of showing empty +# tables. +# VITE_INDEXER_GRAPHQL_URL=https://your-mainnet-indexer.example.com + # Mainnet contract addresses — replace with deployed IDs VITE_CONTRACT_EXCHANGE_ROUTER=CXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX VITE_CONTRACT_SYNTHETICS_READER=CXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX diff --git a/apps/web/.env.testnet b/apps/web/.env.testnet index 6aa7ef61..3d1980ea 100644 --- a/apps/web/.env.testnet +++ b/apps/web/.env.testnet @@ -12,6 +12,14 @@ VITE_HORIZON_URL=https://horizon-testnet.stellar.org # Optional — remove to fall back to GMX infra # VITE_ORACLE_URL=https://arbitrum-api.gmxinfra.io +# ── SubQuery indexer ────────────────────────────────────────────────────────── +# No hosted testnet indexer is deployed yet. Left unset on purpose: the app +# then shows an explicit "unavailable" state in the history panels rather than +# empty tables. To develop against a local stack, run +# `bun run --cwd apps/s03-indexer start` and uncomment the line below (start +# the web app on another port — the GraphQL engine also uses 3000). +# VITE_INDEXER_GRAPHQL_URL=http://localhost:3000 + # ── Core contracts ───────────────────────────────────────────────────────────── VITE_CONTRACT_EXCHANGE_ROUTER=CBD6BQSQFROWIIT5QCYN7KL5LJJWUIH7CEWUSZIFMUJO6NPXE6CVGYNW VITE_CONTRACT_SYNTHETICS_READER=CC6OZUHF3LVO6PNP3V2EB36ORB3YSVYSH3LWD3RFLO4NUO3BYCXSWSYC diff --git a/apps/web/src/app/config/indexer.test.ts b/apps/web/src/app/config/indexer.test.ts new file mode 100644 index 00000000..8a7c6964 --- /dev/null +++ b/apps/web/src/app/config/indexer.test.ts @@ -0,0 +1,47 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" + +import { + INDEXER_CONFIG, + resetIndexerConfigReporting, + validateIndexerConfig, +} from "./indexer" + +// Issue #859: the app renders server-side, so the module that validates the +// indexer config is evaluated once per request as well as once in the browser. +// A disabled indexer used to warn on every route the user visited. +describe("validateIndexerConfig logging", () => { + beforeEach(() => { + resetIndexerConfigReporting() + }) + + afterEach(() => { + vi.restoreAllMocks() + resetIndexerConfigReporting() + }) + + it("is disabled when VITE_INDEXER_GRAPHQL_URL is not set", () => { + expect(INDEXER_CONFIG.enabled).toBe(false) + expect(INDEXER_CONFIG.graphqlUrl).toBeNull() + }) + + it("warns once about a disabled indexer, however often it is called", () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}) + + validateIndexerConfig() + validateIndexerConfig() + validateIndexerConfig() + + expect(warn).toHaveBeenCalledTimes(1) + expect(warn.mock.calls[0]?.[0]).toMatch( + /Indexer disabled: VITE_INDEXER_GRAPHQL_URL not set/, + ) + }) + + it("names the fallback behaviour so the warning is actionable", () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}) + + validateIndexerConfig() + + expect(warn.mock.calls[0]?.[0]).toMatch(/contract-only data/) + }) +}) diff --git a/apps/web/src/app/config/indexer.ts b/apps/web/src/app/config/indexer.ts index 059d6726..5ea012cd 100644 --- a/apps/web/src/app/config/indexer.ts +++ b/apps/web/src/app/config/indexer.ts @@ -52,13 +52,40 @@ export const INDEXER_CONFIG: IndexerConfig = { // Validation // ───────────────────────────────────────────────────────────────────────────── +/** + * Whether this runtime has already reported the indexer's state. + * + * The app renders server-side, so the module that calls this runs once per + * request as well as once in the browser — without this latch, a disabled + * indexer logged the same warning on every route the user visited. + * Validation itself is pure, so skipping the repeats loses nothing. + */ +let hasReported = false + +/** Reset the once-per-runtime log latch. Test-only. */ +export function resetIndexerConfigReporting(): void { + hasReported = false +} + /** * Validate indexer config at startup. * Throws descriptive error if required vars are missing. + * + * Safe to call more than once: the configuration errors below are raised every + * time, but the informational logs are emitted only on the first call. */ export function validateIndexerConfig(): void { + const report = !hasReported + hasReported = true + if (!INDEXER_CONFIG.enabled) { - console.warn("Indexer disabled: VITE_INDEXER_GRAPHQL_URL not set") + if (report) { + console.warn( + "Indexer disabled: VITE_INDEXER_GRAPHQL_URL not set. " + + "Position and order history, fills, and fee activity fall back to " + + "contract-only data. See apps/web/.env.example.", + ) + } return } @@ -70,5 +97,7 @@ export function validateIndexerConfig(): void { throw new Error(`Invalid network: ${INDEXER_CONFIG.network}`) } - console.log(`Indexer enabled: ${INDEXER_CONFIG.graphqlUrl} (${INDEXER_CONFIG.network})`) + if (report) { + console.log(`Indexer enabled: ${INDEXER_CONFIG.graphqlUrl} (${INDEXER_CONFIG.network})`) + } } diff --git a/apps/web/src/features/design-system/design-system-page.tsx b/apps/web/src/features/design-system/design-system-page.tsx index 2b8617dd..cb9c9498 100644 --- a/apps/web/src/features/design-system/design-system-page.tsx +++ b/apps/web/src/features/design-system/design-system-page.tsx @@ -308,7 +308,9 @@ function TableExamples() { Distribution history - + {/* The name is what the scroll container announces when the table is + too wide to fit and becomes a keyboard-reachable region. */} +
Epoch @@ -434,6 +436,7 @@ export function DesignSystemPage() {
inside the + thumb, which a
diff --git a/apps/web/src/features/trade/components/chart/TVChart.tsx b/apps/web/src/features/trade/components/chart/TVChart.tsx index 5c1eeb3f..68ceb414 100644 --- a/apps/web/src/features/trade/components/chart/TVChart.tsx +++ b/apps/web/src/features/trade/components/chart/TVChart.tsx @@ -1,6 +1,7 @@ import { useRef } from "react" import { cn } from "@workspace/ui/lib/utils" import { useChartPreferencesStore } from "../../store/chart-preferences-store" +import { useMarkets } from "../../hooks/useMarkets" import { ChartHeader } from "./ChartHeader" import { TVChartContainer } from "./TVChartContainer" import type { Period } from "../../store/chart-preferences-store" @@ -15,6 +16,10 @@ type Props = { export function TVChart({ symbol, onSelectToken }: Props) { const period = useChartPreferencesStore((s) => s.period) const setPeriod = useChartPreferencesStore((s) => s.setPeriod) + const { markets } = useMarkets() + // `symbol` is the index token's contract address. Resolve the market's + // display name so labels read "BTC/USD" rather than the raw address. + const marketLabel = markets.find((m) => m.indexTokenAddress === symbol)?.name const buttonRefs = useRef>(new Map()) // When the period changes, persist preference and invalidate the candles cache @@ -83,7 +88,7 @@ export function TVChart({ symbol, onSelectToken }: Props) { {/* Chart */}
{symbol ? ( - + ) : (
Select a market diff --git a/apps/web/src/features/trade/components/chart/TVChartContainer.test.tsx b/apps/web/src/features/trade/components/chart/TVChartContainer.test.tsx index 216520b0..ad84f36f 100644 --- a/apps/web/src/features/trade/components/chart/TVChartContainer.test.tsx +++ b/apps/web/src/features/trade/components/chart/TVChartContainer.test.tsx @@ -58,6 +58,16 @@ vi.mock("../../hooks/usePositions", () => ({ usePositions: () => ({ data: mockPositions }), })) +// The component also draws resting-order price lines. Without this the real +// hook runs `useQuery` outside a QueryClientProvider and every test in this +// file throws before rendering. +const mockOrders: Array> = [] + +vi.mock("../../hooks/useOrders", () => ({ + useOrders: () => ({ data: mockOrders }), + hasFrozenOrders: () => false, +})) + // JSDOM lacks ResizeObserver / MutationObserver const mockObserverDisconnect = vi.fn() class ObserverStub { @@ -137,8 +147,28 @@ describe("TVChartContainer", () => { mockCandles = SAMPLE_CANDLES render() - const chart = screen.getByRole("img", { name: /price chart for btc/i }) + // Issue #858: the canvas is pannable and zoomable, so it must not be + // announced as a static image (axe nested-interactive). + const chart = screen.getByRole("region", { name: /price chart for btc/i }) expect(chart).toBeInTheDocument() + expect(screen.queryByRole("img")).not.toBeInTheDocument() + }) + + it("labels the chart with the market name rather than the raw address", () => { + mockCandles = SAMPLE_CANDLES + render( + , + ) + + expect( + screen.getByRole("region", { name: "Price chart for BTC/USD" }), + ).toBeInTheDocument() + expect(screen.getByText(/5m chart for BTC\/USD/)).toBeInTheDocument() + expect(screen.queryByText(/CCFTOPHUPSUDO/)).not.toBeInTheDocument() }) it("describes the chart as linked from the summary", () => { diff --git a/apps/web/src/features/trade/components/chart/TVChartContainer.tsx b/apps/web/src/features/trade/components/chart/TVChartContainer.tsx index b4713897..7fae2d72 100644 --- a/apps/web/src/features/trade/components/chart/TVChartContainer.tsx +++ b/apps/web/src/features/trade/components/chart/TVChartContainer.tsx @@ -28,9 +28,6 @@ import type { OhlcBar } from "../../lib/oracle" import { useWalletStore } from "@/features/wallet/store/wallet-store" import { formatUsd } from "@/shared/lib/format" import { ENV } from "@/app/config/env" -import type { CandlestickData, IChartApi, IPriceLine, ISeriesApi, UTCTimestamp } from "lightweight-charts" -import type { OhlcBar } from "../../lib/oracle" -import { useWalletStore } from "@/features/wallet/store/wallet-store" type ChartLine = { id: string @@ -41,8 +38,15 @@ type ChartLine = { } type Props = { + /** Index-token address the chart data is fetched for. */ symbol: string period: string + /** + * Human-readable market name (e.g. "BTC/USD") used for every label a person + * reads or hears. Falls back to `symbol`, which on-chain is a raw contract + * address — unusable as an accessible name. + */ + marketLabel?: string } type ChartSummary = { @@ -101,7 +105,7 @@ function toChartBar(bar: OhlcBar): CandlestickData { } } -export function TVChartContainer({ symbol, period }: Props) { +export function TVChartContainer({ symbol, period, marketLabel }: Props) { const containerRef = useRef(null) const chartRef = useRef(null) const seriesRef = useRef | null>(null) @@ -144,7 +148,10 @@ export function TVChartContainer({ symbol, period }: Props) { const isNoHistory = !isLoading && !hasData && !isError && !isStale const isFailed = isError && !hasData - const summary = useMemo(() => getChartSummary(candles, liveBar, symbol, period), [candles, liveBar, symbol, period]) + // Everything a person reads or hears uses the market name; `symbol` stays + // the fetch key. + const label = marketLabel ?? symbol + const summary = useMemo(() => getChartSummary(candles, liveBar, label, period), [candles, liveBar, label, period]) const tableRows = useMemo(() => getRepresentativeRows(candles), [candles]) const currentSymbolRef = useRef(symbol) @@ -352,8 +359,8 @@ export function TVChartContainer({ symbol, period }: Props) { const now = Date.now() if (now - lastAnnounceRef.current < THROTTLE_MS) return lastAnnounceRef.current = now - announcer.announce(`${symbol} ${summary.latestPrice}`) - }, [liveBar, summary, symbol, announcer]) + announcer.announce(`${label} ${summary.latestPrice}`) + }, [liveBar, summary, label, announcer]) return (
@@ -384,10 +391,10 @@ export function TVChartContainer({ symbol, period }: Props) {
- Loading {period} chart data for {symbol}… + Loading {period} chart data for {label}…
)} @@ -406,7 +413,7 @@ export function TVChartContainer({ symbol, period }: Props) { {isNoHistory && (

- No trading history available for {symbol} + No trading history available for {label}

This market may be new. Check back once trading begins. @@ -426,7 +433,7 @@ export function TVChartContainer({ symbol, period }: Props) { @@ -445,7 +452,7 @@ export function TVChartContainer({ symbol, period }: Props) { @@ -453,12 +460,16 @@ export function TVChartContainer({ symbol, period }: Props) { )} {/* Lightweight Charts canvas — hidden from AT; the VisuallyHidden summary replaces it */} + {/* The canvas is pannable and zoomable, so `role="img"` made it an + interactive element inside a non-interactive one (axe + nested-interactive). A named region keeps it findable without + claiming the content is static. */}

@@ -474,7 +485,7 @@ export function TVChartContainer({ symbol, period }: Props) { aria-expanded={showTable} aria-controls="ohlc-data-table" > - {symbol} OHLC Data + {label} OHLC Data diff --git a/apps/web/src/features/trade/components/orderbook/OrderBookPanel.test.tsx b/apps/web/src/features/trade/components/orderbook/OrderBookPanel.test.tsx index 07f0ccdb..8403a7d1 100644 --- a/apps/web/src/features/trade/components/orderbook/OrderBookPanel.test.tsx +++ b/apps/web/src/features/trade/components/orderbook/OrderBookPanel.test.tsx @@ -1,6 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" -import { cleanup, render, screen } from "@testing-library/react" +import { cleanup, render, screen, within } from "@testing-library/react" import userEvent from "@testing-library/user-event" +import { useChartPreferencesStore } from "../../store/chart-preferences-store" import { OrderBookPanel } from "./OrderBookPanel" // ── Mocks ──────────────────────────────────────────────────────────────────── @@ -32,19 +33,6 @@ vi.mock("../../../settings/store/preferences-store", () => ({ usePreferencesStore: () => ({ slippageTolerance: 0.5 }), })) -vi.mock("../../store/chart-preferences-store", () => ({ - useChartPreferencesStore: (selector: any) => { - const state = { - period: "5m", - orderbookView: "trades", - setPeriod: vi.fn(), - setOrderbookView: vi.fn(), - reset: vi.fn(), - } - return selector(state) - }, -})) - vi.mock("./RecentTradesTape", () => ({ RecentTradesTape: ({ symbol }: any) =>
Recent Trades: {symbol}
, })) @@ -63,6 +51,9 @@ describe("OrderBookPanel", () => { beforeEach(() => { mockOrderBookCalls = 0 + // The real store is deliberately persisted (OB-069), so each test starts + // from the documented default rather than the previous test's leftovers. + useChartPreferencesStore.setState({ period: "5m", orderbookView: "trades" }) }) // ═════════════════════════════════════════════════════════════════════════ @@ -183,7 +174,7 @@ describe("OrderBookPanel", () => { expect(screen.getByText("Depth Chart: ETH")).toBeInTheDocument() }) - it("does not leak orderbook view state between panel instances", async () => { + it("keeps the chosen view across remounts (it is a persisted preference)", async () => { const user = userEvent.setup() const { unmount } = render( @@ -192,15 +183,14 @@ describe("OrderBookPanel", () => { await user.click(screen.getByRole("tab", { name: /depth chart/i })) - // Unmount and remount - should reset to default view unmount() - // New instance with different symbol + // New instance with a different symbol. The view lives in the persisted + // chart-preferences store (OB-069), not in component state, so it is + // deliberately carried over. render() - // Should default back to trades, not retain depth chart view - // (Note: in real app with store, this tests store isolation) - expect(screen.getByText("Recent Trades: ETH")).toBeInTheDocument() + expect(screen.getByText("Depth Chart: ETH")).toBeInTheDocument() }) it("header displays reference data indicator", () => { @@ -216,6 +206,78 @@ describe("OrderBookPanel", () => { expect(screen.getByText("Reference Data")).toBeInTheDocument() }) + // ═════════════════════════════════════════════════════════════════════════ + // Tab semantics (issue #858) + // ═════════════════════════════════════════════════════════════════════════ + + describe("tab semantics (#858)", () => { + it("puts the tabs in a named tablist", () => { + render() + + const tablist = screen.getByRole("tablist", { name: "Order book view" }) + expect(tablist).toBeInTheDocument() + expect(within(tablist).getAllByRole("tab")).toHaveLength(3) + }) + + it("links the selected tab to a labelled tabpanel", () => { + render() + + const tab = screen.getByRole("tab", { name: /^trades$/i }) + const panel = screen.getByRole("tabpanel", { name: /^trades$/i }) + + expect(tab).toHaveAttribute("aria-controls", panel.id) + expect(panel).toHaveAttribute("aria-labelledby", tab.id) + }) + + it("exposes a single tab stop with roving tabindex", () => { + render() + + expect(screen.getByRole("tab", { name: /^trades$/i })).toHaveAttribute("tabindex", "0") + expect(screen.getByRole("tab", { name: /order book/i })).toHaveAttribute("tabindex", "-1") + expect(screen.getByRole("tab", { name: /depth chart/i })).toHaveAttribute("tabindex", "-1") + }) + + it("moves between tabs with the arrow keys", async () => { + const user = userEvent.setup() + render() + + screen.getByRole("tab", { name: /^trades$/i }).focus() + await user.keyboard("{ArrowRight}") + + expect(screen.getByRole("tab", { name: /depth chart/i })).toHaveAttribute( + "aria-selected", + "true", + ) + + await user.keyboard("{ArrowRight}") + + expect(screen.getByRole("tab", { name: /order book/i })).toHaveAttribute( + "aria-selected", + "true", + ) + }) + + it("jumps to the first and last tab with Home and End", async () => { + const user = userEvent.setup() + render() + + screen.getByRole("tab", { name: /^trades$/i }).focus() + await user.keyboard("{End}") + + expect(screen.getByRole("tab", { name: /depth chart/i })).toHaveAttribute( + "aria-selected", + "true", + ) + + await user.keyboard("{Home}") + + expect(screen.getByRole("tab", { name: /order book/i })).toHaveAttribute( + "aria-selected", + "true", + ) + }) + }) + describe("source health (OB-049)", () => { beforeEach(() => { mockSource = null diff --git a/apps/web/src/features/trade/components/orderbook/OrderBookPanel.tsx b/apps/web/src/features/trade/components/orderbook/OrderBookPanel.tsx index 05f22737..192426b1 100644 --- a/apps/web/src/features/trade/components/orderbook/OrderBookPanel.tsx +++ b/apps/web/src/features/trade/components/orderbook/OrderBookPanel.tsx @@ -1,64 +1,91 @@ +import { useId, useRef } from "react" import { useOrderBook } from "../../hooks/useOrderBook" import { useSourceFreshness } from "../../hooks/useSourceFreshness" import { useChartPreferencesStore } from "../../store/chart-preferences-store" import { SourceHealthBadge } from "./SourceHealthBadge" import { RecentTradesTape } from "./RecentTradesTape" import { DepthChart } from "./DepthChart" +import type { KeyboardEvent as ReactKeyboardEvent } from "react" +import type { OrderbookView } from "../../store/chart-preferences-store" type Props = { symbol: string | undefined } +const ORDERBOOK_TABS = [ + { view: "book", label: "Order Book" }, + { view: "trades", label: "Trades" }, + { view: "chart", label: "Depth Chart" }, +] as const satisfies ReadonlyArray<{ view: OrderbookView; label: string }> + export function OrderBookPanel({ symbol }: Props) { const orderbookView = useChartPreferencesStore((s) => s.orderbookView) const setOrderbookView = useChartPreferencesStore((s) => s.setOrderbookView) const { bids, asks, status, isLoading } = useOrderBook(symbol) const source = useSourceFreshness(symbol) + const baseId = useId() + const tabId = (view: OrderbookView) => `${baseId}-tab-${view}` + const panelId = (view: OrderbookView) => `${baseId}-panel-${view}` + const tabRefs = useRef>(new Map()) + + // APG tabs: the tablist is a single tab stop, and Left/Right (plus Home/End) + // move between tabs. Without this the roving `tabIndex` below would strand + // keyboard users on whichever tab happens to be selected. + function handleKeyDown(event: ReactKeyboardEvent) { + const keys = ["ArrowLeft", "ArrowRight", "Home", "End"] + if (!keys.includes(event.key)) return + event.preventDefault() + + const current = ORDERBOOK_TABS.findIndex((t) => t.view === orderbookView) + const last = ORDERBOOK_TABS.length - 1 + let next = current + if (event.key === "ArrowLeft") next = current <= 0 ? last : current - 1 + else if (event.key === "ArrowRight") next = current >= last ? 0 : current + 1 + else if (event.key === "Home") next = 0 + else next = last + + const nextView = ORDERBOOK_TABS[next].view + setOrderbookView(nextView) + tabRefs.current.get(nextView)?.focus() + } + return (
{/* ── Panel Header Tabs ─────────────────────────────────────────────── */}
-
- - - +
+ {ORDERBOOK_TABS.map(({ view, label }) => { + const isActive = orderbookView === view + return ( + + ) + })}
Reference Data @@ -67,7 +94,16 @@ export function OrderBookPanel({ symbol }: Props) {
{/* ── Tab Content ──────────────────────────────────────────────────── */} -
+ {/* APG: a tabpanel is a tab stop only when it holds nothing focusable. + The trades tape owns a focusable scroll region, so making the panel + focusable too would just add a second, redundant stop. */} +
{orderbookView === "book" ? (
Executable order-book depth is unavailable until a verified matching source is connected. diff --git a/apps/web/src/features/trade/components/orderbook/RecentTradesTape.test.tsx b/apps/web/src/features/trade/components/orderbook/RecentTradesTape.test.tsx index d6b210dc..a6b9a929 100644 --- a/apps/web/src/features/trade/components/orderbook/RecentTradesTape.test.tsx +++ b/apps/web/src/features/trade/components/orderbook/RecentTradesTape.test.tsx @@ -414,6 +414,31 @@ describe("RecentTradesTape", () => { }) }) + // Issue #858: nothing inside the tape is focusable, so the scroll container + // itself has to be a named tab stop or the list cannot be reached or + // scrolled by keyboard (axe scrollable-region-focusable). + describe("scroll region accessibility (#858)", () => { + it("exposes the scrollable list as a focusable, named region", () => { + render() + + const region = screen.getByRole("region", { + name: "Recent trades for TWBTC", + }) + expect(region).toHaveAttribute("tabindex", "0") + expect(region.className).toContain("overflow-y-auto") + }) + + it("names the region even without a selected market", () => { + render() + + expect( + screen.getByRole("region", { + name: "Recent trades for the selected market", + }), + ).toBeInTheDocument() + }) + }) + describe("symbol changes", () => { it("updates when symbol prop changes", () => { const { rerender } = render() diff --git a/apps/web/src/features/trade/components/orderbook/RecentTradesTape.tsx b/apps/web/src/features/trade/components/orderbook/RecentTradesTape.tsx index 06e7f83f..892cf520 100644 --- a/apps/web/src/features/trade/components/orderbook/RecentTradesTape.tsx +++ b/apps/web/src/features/trade/components/orderbook/RecentTradesTape.tsx @@ -52,7 +52,17 @@ export function RecentTradesTape({ symbol }: Props) {
{/* ── Scrollable trades list ───────────────────────────────────────── */} -
+ {/* Nothing inside the tape is focusable, so the scroll container itself + is the tab stop — otherwise the list cannot be scrolled by keyboard + (axe scrollable-region-focusable). A named region makes that stop + announce what it is. */} +
{isLoading && trades.length === 0 ? (
diff --git a/apps/web/src/features/trade/components/positions/OrderExecutionFrozenBanner.tsx b/apps/web/src/features/trade/components/positions/OrderExecutionFrozenBanner.tsx index 2a7a3f55..e93666dd 100644 --- a/apps/web/src/features/trade/components/positions/OrderExecutionFrozenBanner.tsx +++ b/apps/web/src/features/trade/components/positions/OrderExecutionFrozenBanner.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect } from "react" +import { useEffect, useState } from "react" import { ORDER_EXECUTION_FROZEN_MESSAGE } from "@/lib/contracts" const SESSION_KEY = "so4-order-execution-frozen-dismissed" @@ -12,7 +12,7 @@ export function OrderExecutionFrozenBanner({ visible }: Props) { useEffect(() => { try { - if (typeof window !== "undefined" && sessionStorage) { + if (typeof window !== "undefined") { setDismissed(sessionStorage.getItem(SESSION_KEY) === "1") } } catch { diff --git a/apps/web/src/features/trade/components/trade-panel/LeverageSlider.tsx b/apps/web/src/features/trade/components/trade-panel/LeverageSlider.tsx index 7d63c51a..821a38b6 100644 --- a/apps/web/src/features/trade/components/trade-panel/LeverageSlider.tsx +++ b/apps/web/src/features/trade/components/trade-panel/LeverageSlider.tsx @@ -17,6 +17,7 @@ export function LeverageSlider({ min = 1, max = 50, step = 0.5, value, onChange {value}×
void - setOrderbookView: (value: SupportedChartView) => void + setOrderbookView: (value: OrderbookView) => void reset: () => void } @@ -48,8 +48,8 @@ function recoverPreferences( : CHART_PREFERENCES_DEFAULTS.period // Validate orderbookView is one of the allowed values - const orderbookView = SUPPORTED_CHART_VIEWS.includes(obj.orderbookView as SupportedChartView) - ? (obj.orderbookView as SupportedChartView) + const orderbookView = SUPPORTED_CHART_VIEWS.includes(obj.orderbookView as OrderbookView) + ? (obj.orderbookView as OrderbookView) : CHART_PREFERENCES_DEFAULTS.orderbookView return { period, orderbookView } @@ -68,7 +68,7 @@ export const useChartPreferencesStore = create()( } }, - setOrderbookView: (value: SupportedChartView) => { + setOrderbookView: (value: OrderbookView) => { // Only accept supported views; silently ignore invalid values if (SUPPORTED_CHART_VIEWS.includes(value)) { set({ orderbookView: value }) diff --git a/apps/web/src/features/wallet/components/NetworkMismatchBanner.tsx b/apps/web/src/features/wallet/components/NetworkMismatchBanner.tsx index ebb0b236..7c8aa98d 100644 --- a/apps/web/src/features/wallet/components/NetworkMismatchBanner.tsx +++ b/apps/web/src/features/wallet/components/NetworkMismatchBanner.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect } from "react" +import { useEffect, useState } from "react" import { useLocation } from "@tanstack/react-router" import { useNetwork } from "../hooks/useNetwork" import { useWalletStore } from "../store/wallet-store" @@ -14,7 +14,7 @@ export function NetworkMismatchBanner() { useEffect(() => { try { - if (typeof window !== "undefined" && sessionStorage) { + if (typeof window !== "undefined") { setDismissed(sessionStorage.getItem(SESSION_KEY) === "1") } } catch { diff --git a/apps/web/src/shared/hooks/useTxSubmit.tsx b/apps/web/src/shared/hooks/useTxSubmit.tsx index 7dff23a0..1700d948 100644 --- a/apps/web/src/shared/hooks/useTxSubmit.tsx +++ b/apps/web/src/shared/hooks/useTxSubmit.tsx @@ -1,8 +1,8 @@ -import { useCallback, useState, useRef } from "react" +import { useCallback, useRef, useState } from "react" import { toast } from "@workspace/ui/components/toast" +import { useTxFeedback } from "./useTxFeedback" import { sendAndPoll } from "@/lib/tx-builder" import { explorerTxUrl } from "@/app/config/network" -import { useTxFeedback } from "./useTxFeedback" export type SubmitTxOptions = { loadingMessage: string @@ -119,7 +119,7 @@ export function useTxSubmit() { const attemptId = `txn-${Date.now()}-${Math.random().toString(36).slice(2, 9)}` attemptIdRef.current = attemptId - const attempt = feedback.createAttempt(attemptId) + feedback.createAttempt(attemptId) // #733: Prevent duplicate submission with attempt tracking if (submitInProgressRef.current) { diff --git a/packages/ui/CONTRIBUTING.md b/packages/ui/CONTRIBUTING.md index f2f8d36e..1b1fea3f 100644 --- a/packages/ui/CONTRIBUTING.md +++ b/packages/ui/CONTRIBUTING.md @@ -33,6 +33,44 @@ If you're unsure, default to the feature directory — it's a much smaller chang 6. **Document in the gallery**: add your component to [`apps/web/src/features/gallery/components/gallery-page.tsx`](../../apps/web/src/features/gallery/components/gallery-page.tsx), rendering every variant/size it supports. This is how reviewers and future contributors discover what exists and see every state at a glance — see [`DESIGN.md`](../../DESIGN.md#the-component-gallery-gallery) for why it exists. 7. **Log in the changelog**: if the change is user-observable (new component, API change, deprecation, accessibility fix, token addition), add an entry to [`CHANGELOG.md`](./CHANGELOG.md) under the correct section — see the file's header for the expected sections. Breaking changes require a `### Migration note` subsection. +## Trigger parts: pass a custom element through `render`, never as a child + +Every Base UI trigger part — `TooltipTrigger`, `PopoverTrigger`, `DialogTrigger`, +`DropdownMenuTrigger`, `SelectTrigger`, `AlertDialogTrigger` — **renders its own +` + + + +``` + +React reports this at hydration (`} /> +``` + +Plain text is fine as a child, because text is not an element: + +```tsx +Execution fee +``` + +The same applies to any non-button element you want the trigger to be — pass +`render={}`, `render={
}` and so on, and put the content inside +the trigger as children. [`icon-button.tsx`](./src/components/icon-button.tsx), +[`copy-button.tsx`](./src/components/copy-button.tsx) and +[`address-display.tsx`](./src/components/address-display.tsx) are the +in-repo examples. + ## Commands Run from the repo root (or `--filter=@workspace/ui` via turbo to scope to just this package): diff --git a/packages/ui/README.md b/packages/ui/README.md index 91d61867..27542d09 100644 --- a/packages/ui/README.md +++ b/packages/ui/README.md @@ -33,6 +33,25 @@ All components are located in `src/components/` and exported through the package - **Spinner** - Busy indicator, hidden from assistive tech unless labelled - **LoadingButton** - Button that owns its pending state, spinner and `aria-busy` +## Composing triggers + +Base UI trigger parts (`TooltipTrigger`, `PopoverTrigger`, `DialogTrigger`, +`DropdownMenuTrigger`, …) render their own ` + + +// ✓ one button +Hover me} /> +``` + +See [CONTRIBUTING.md](./CONTRIBUTING.md#trigger-parts-pass-a-custom-element-through-render-never-as-a-child) +for the full rule. + ## Semantic tokens `src/styles/globals.css` defines the semantic colour roles every component draws diff --git a/packages/ui/src/components/slider.test.tsx b/packages/ui/src/components/slider.test.tsx index 6c2723c6..bb1317e9 100644 --- a/packages/ui/src/components/slider.test.tsx +++ b/packages/ui/src/components/slider.test.tsx @@ -22,3 +22,40 @@ describe("Slider accessibility", () => { expect(slider).toHaveAttribute("aria-valuenow") }) }) + +// Issue #857: the control a person actually operates is the `` Base UI nests inside the thumb. `aria-label` on the root only +// names the surrounding `role="group"`, so without forwarding the input stays +// anonymous (axe `label`). +describe("Slider labelling", () => { + it("names the range input from aria-label", () => { + const { container } = render() + + const input = container.querySelector("input[type='range']") + expect(input).toHaveAttribute("aria-label", "Leverage") + }) + + it("names the range input from aria-labelledby", () => { + const { container } = render( + <> + Example value + + , + ) + + const input = container.querySelector("input[type='range']") + expect(input).toHaveAttribute("aria-labelledby", "slider-label") + }) + + it("names every thumb of a range slider", () => { + const { container } = render( + , + ) + + const inputs = container.querySelectorAll("input[type='range']") + expect(inputs).toHaveLength(2) + for (const input of inputs) { + expect(input).toHaveAttribute("aria-label", "Price range") + } + }) +}) diff --git a/packages/ui/src/components/slider.tsx b/packages/ui/src/components/slider.tsx index d3cc4c38..4e3d1323 100644 --- a/packages/ui/src/components/slider.tsx +++ b/packages/ui/src/components/slider.tsx @@ -16,6 +16,13 @@ function Slider({ ? defaultValue : [min, max] + // Base UI renders the real control as an `` nested in + // each thumb, and `aria-label` on the root only names the surrounding + // `role="group"`. Without forwarding, the input itself has no accessible + // name (axe `label`), so the label is applied in both places. + const ariaLabel = props["aria-label"] + const ariaLabelledBy = props["aria-labelledby"] + return ( ))} diff --git a/packages/ui/src/components/table.test.tsx b/packages/ui/src/components/table.test.tsx new file mode 100644 index 00000000..0d125dc3 --- /dev/null +++ b/packages/ui/src/components/table.test.tsx @@ -0,0 +1,79 @@ +import { afterEach, describe, expect, it } from "vitest" +import { render, screen } from "@testing-library/react" + +import { Table, TableBody, TableCell, TableRow } from "./table" + +// jsdom has no layout, so overflow is simulated by overriding the two metrics +// the overflow hook measures. +function simulateWidths(scrollWidth: number, clientWidth: number) { + for (const [prop, value] of [ + ["scrollWidth", scrollWidth], + ["clientWidth", clientWidth], + ] as const) { + Object.defineProperty(HTMLDivElement.prototype, prop, { + configurable: true, + get: () => value, + }) + } +} + +function body() { + return ( + + + Cell + + + ) +} + +afterEach(() => { + Reflect.deleteProperty(HTMLDivElement.prototype, "scrollWidth") + Reflect.deleteProperty(HTMLDivElement.prototype, "clientWidth") +}) + +// Issue #857: a scroll container with no focusable content cannot be scrolled +// from the keyboard (axe `scrollable-region-focusable`). +describe("Table scroll container", () => { + it("is not a tab stop while the table fits", () => { + simulateWidths(400, 400) + const { container } = render(
{body()}
) + + const wrapper = container.querySelector('[data-slot="table-container"]') + expect(wrapper).not.toHaveAttribute("tabindex") + expect(wrapper).not.toHaveAttribute("role") + }) + + it("becomes a named, focusable region once it overflows", () => { + simulateWidths(900, 400) + render({body()}
) + + const region = screen.getByRole("region", { name: "Distributions" }) + expect(region).toHaveAttribute("data-slot", "table-container") + expect(region).toHaveAttribute("tabindex", "0") + }) + + it("supports aria-labelledby as the region's name", () => { + simulateWidths(900, 400) + render( + <> + Distribution history + {body()}
+ , + ) + + expect( + screen.getByRole("region", { name: "Distribution history" }), + ).toBeInTheDocument() + }) + + it("stays keyboard-reachable but unnamed when the table has no label", () => { + simulateWidths(900, 400) + const { container } = render({body()}
) + + const wrapper = container.querySelector('[data-slot="table-container"]') + expect(wrapper).toHaveAttribute("tabindex", "0") + // An anonymous region is worse than no region at all. + expect(wrapper).not.toHaveAttribute("role") + }) +}) diff --git a/packages/ui/src/components/table.tsx b/packages/ui/src/components/table.tsx index d7cfbeee..e88caa6a 100644 --- a/packages/ui/src/components/table.tsx +++ b/packages/ui/src/components/table.tsx @@ -1,3 +1,8 @@ +"use client" + +import * as React from "react" + +import { useOverflowEdges } from "@workspace/ui/hooks/use-overflow-edges" import { cn } from "@workspace/ui/lib/utils" /** @@ -5,16 +10,57 @@ import { cn } from "@workspace/ui/lib/utils" * * `Table` wraps the `` in its own horizontally scrollable container so * wide financial tables never push the page body sideways on mobile. + * + * A scroll container with no focusable content cannot be scrolled by keyboard + * (axe `scrollable-region-focusable`), so the container becomes a tab stop — + * but only while it actually overflows, so tables that fit do not litter the + * page with empty tab stops. Give the `
` an `aria-label` or + * `aria-labelledby` and the stop is announced as a named region; without a + * name it stays an unlabelled tab stop, because an anonymous `role="region"` + * is worse than none. */ function Table({ className, containerClassName, ...props }: React.ComponentProps<"table"> & { containerClassName?: string }) { + const [scrollable, setScrollable] = React.useState(false) + const { rootRef, viewportRef } = useOverflowEdges( + "horizontal", + React.useCallback( + (edges: { left: boolean; right: boolean }) => + setScrollable(edges.left || edges.right), + [] + ) + ) + + // The container is both the measured root and the scrolling viewport. + const setContainer = React.useCallback( + (node: HTMLDivElement | null) => { + rootRef.current = node + viewportRef.current = node + }, + [rootRef, viewportRef] + ) + + const ariaLabel = props["aria-label"] + const ariaLabelledBy = props["aria-labelledby"] + const labelled = Boolean(ariaLabel || ariaLabelledBy) + return (
. Passing a +// Button as a *child* produced , which is +// invalid HTML (hydration mismatch) and an axe `nested-interactive` +// violation. A custom element has to go through the `render` prop instead. +describe("TooltipTrigger render prop", () => { + it("renders the custom element as the trigger, not inside one", () => { + const { container } = render( + + Hover me} /> + Tip + , + ) + + const buttons = container.querySelectorAll("button") + expect(buttons).toHaveLength(1) + expect(buttons[0]).toHaveAttribute("data-slot", "tooltip-trigger") + expect(screen.getByRole("button", { name: "Hover me" })).toBeInTheDocument() + }) + + it("never nests an interactive element inside the trigger", () => { + const { container } = render( + + Open} /> + Tip + , + ) + + const trigger = container.querySelector('[data-slot="tooltip-trigger"]') + expect(trigger).not.toBeNull() + expect( + trigger?.querySelectorAll("button, a[href], input, select, textarea"), + ).toHaveLength(0) + }) +})