Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions apps/web/src/lib/api.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,8 @@
/**
* API client utilities for the web app.
*
* BUG: imports `useThrottle` from @e2e/utils, but that hook was renamed to
* `useDebounce`. This causes a TypeScript error and a runtime crash.
*
* Fix: change the import to `useDebounce`.
*/

// BUG: useThrottle no longer exists — was renamed to useDebounce
import { useThrottle } from "@e2e/utils"
import { useSearchDebounce } from "@e2e/utils"
import { formatDate, formatAUD } from "@e2e/utils"

export const BASE_URL = process.env.API_URL ?? "http://localhost:3000"
Expand All @@ -28,5 +22,5 @@ export async function fetchPosts() {
// Re-export formatting utilities used throughout the app
export { formatDate, formatAUD }

// Re-export the debounce hook (currently broken import)
export { useThrottle as useSearchDebounce }
// Re-export the debounce hook
export { useSearchDebounce }
54 changes: 45 additions & 9 deletions packages/ui/src/components/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,36 @@ type Props = {
"aria-label"?: string
}

const FALLBACK_ICON_LABEL = "Button"

function deriveTextLabel(node: React.ReactNode): string | undefined {
if (typeof node === "string") {
const trimmed = node.trim()
return trimmed === "" ? undefined : trimmed
}
if (typeof node === "number") return String(node)
if (Array.isArray(node)) {
const parts = node.map(deriveTextLabel).filter((p): p is string => Boolean(p))
return parts.length === 0 ? undefined : parts.join(" ")
}
return undefined
}

/**
* Button component.
*
* BUG: When `iconOnly` is true, the button renders without visible text.
* An `aria-label` is required for screen reader accessibility (WCAG 2.1 SC 4.1.2),
* but the component does not enforce or warn about its absence.
* An icon-only button has no visible text, so it must expose an accessible
* name via aria-label (WCAG 2.2 SC 4.1.2 Name, Role, Value). The aria-label
* prop is always forwarded to the rendered <button>. When iconOnly is set and
* no aria-label is supplied we derive one from the children, falling back to a
* generic label, and warn in development so the missing label gets fixed.
*
* The icon is only marked aria-hidden when the button carries an aria-label to
* act as its accessible name. Without a label the icon's own text content is
* left exposed, since it may be the button's only accessible name.
*
* The test in Button.test.tsx checks that an icon-only button has an accessible name.
* Fix: throw/warn in development when `iconOnly && !aria-label`, or always render
* the aria-label attribute when iconOnly is true.
* No `type` is hard-coded: the browser default (submit) applies so the
* component stays usable as a form submit button.
*/
export function Button({
children,
Expand All @@ -34,15 +54,31 @@ export function Button({
onClick,
"aria-label": ariaLabel,
}: Props) {
let accessibleLabel = ariaLabel

if (iconOnly && !accessibleLabel) {
accessibleLabel = deriveTextLabel(children) ?? FALLBACK_ICON_LABEL

if (process.env.NODE_ENV !== "production") {
console.warn(
"Button: `iconOnly` buttons require an explicit `aria-label` to have a " +
`meaningful accessible name (WCAG 2.2 SC 4.1.2). Falling back to "${accessibleLabel}".`,
)
}
}

return (
<button
className={`btn btn-${variant}`}
disabled={disabled}
onClick={onClick}
// BUG: aria-label is not applied when iconOnly is true and no ariaLabel is passed
// The component should enforce aria-label for icon-only buttons
aria-label={accessibleLabel}
>
{icon && <span className="btn-icon">{icon}</span>}
{icon && (
<span className="btn-icon" aria-hidden={accessibleLabel ? "true" : undefined}>
{icon}
</span>
)}
{!iconOnly && children}
</button>
)
Expand Down
39 changes: 20 additions & 19 deletions packages/ui/src/components/DataTable/DataTable.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import React, { useState } from "react"
import React, { useCallback, useState } from "react"

type SortDir = "asc" | "desc"

type SortState<T> = {
key: keyof T | null
dir: SortDir
}

type Column<T> = {
key: keyof T
label: string
Expand All @@ -16,27 +21,23 @@ type Props<T extends Record<string, unknown>> = {
/**
* DataTable with client-side sorting.
*
* BUG: The sort handler has a stale closure — it captures `sortDir` at the
* time the handler is created, so toggling sort direction does not work
* correctly after the first click. The second click always sorts in the same
* direction as the first.
*
* Fix: use the functional form of setState — `setSortDir(prev => ...)` —
* so the toggle always reads the current value.
* Sort key and direction are held in a single state object and updated via the
* functional form of setState, so the toggle always derives from the current
* value rather than one captured when the handler was created. This keeps
* repeated clicks on the same column toggling between ascending and descending
* even when several clicks are batched into one render.
*/
export function DataTable<T extends Record<string, unknown>>({ data, columns }: Props<T>) {
const [sortKey, setSortKey] = useState<keyof T | null>(null)
const [sortDir, setSortDir] = useState<SortDir>("asc")
const [sort, setSort] = useState<SortState<T>>({ key: null, dir: "asc" })
const { key: sortKey, dir: sortDir } = sort

// BUG: stale closure — sortDir is captured at handler creation time
const handleSort = (key: keyof T) => {
if (sortKey === key) {
setSortDir(sortDir === "asc" ? "desc" : "asc") // BUG: reads stale sortDir
} else {
setSortKey(key)
setSortDir("asc")
}
}
const handleSort = useCallback((key: keyof T) => {
setSort((prev) =>
prev.key === key
? { key, dir: prev.dir === "asc" ? "desc" : "asc" }
: { key, dir: "asc" },
)
}, [])

const sorted = sortKey
? [...data].sort((a, b) => {
Expand Down
30 changes: 16 additions & 14 deletions packages/utils/src/format/date.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,28 @@
/**
* Date formatting utilities.
*
* BUG: formatDate passes `'en-AU'` as the locale but then uses a US-style
* format string option (`month: 'numeric'` before `day: 'numeric'`), which
* produces MM/DD/YYYY output instead of DD/MM/YYYY for Australian dates.
*
* Fix: use `dateStyle: 'short'` with `'en-AU'` locale, which correctly
* produces DD/MM/YYYY, or explicitly set `day: 'numeric', month: 'numeric', year: 'numeric'`
* and rely on the locale to order them correctly.
* Australian dates are day/month/year. `Intl.DateTimeFormat("en-AU", ...)`
* gives the correct field order, but CLDR pads the day to two digits whenever
* the pattern is fully numeric, producing "01/03/2024". We want an unpadded
* day with a zero-padded month ("1/03/2024"), so the day part is taken from
* `formatToParts` and the leading zero stripped while the locale keeps
* ownership of field order and separators.
*/
const AU_DATE_FORMAT = new Intl.DateTimeFormat("en-AU", {
day: "numeric",
month: "2-digit",
year: "numeric",
})

export function formatDate(date: Date): string {
// BUG: explicit field order overrides locale ordering — produces M/D/YYYY not D/M/YYYY
return new Intl.DateTimeFormat("en-AU", {
month: "numeric",
day: "numeric",
year: "numeric",
}).format(date)
return AU_DATE_FORMAT.formatToParts(date)
.map((part) => (part.type === "day" ? String(Number(part.value)) : part.value))
.join("")
}

export function formatDateTime(date: Date): string {
return new Intl.DateTimeFormat("en-AU", {
dateStyle: "short",
timeStyle: "short",
}).format(date)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@
* Debounce a value — returns the value only after it has stopped changing
* for `delay` milliseconds.
*
* NOTE: This hook was recently renamed from `useThrottle` to `useDebounce`.
* Any code importing `useThrottle` from this package will break.
* Canonical export name: `useSearchDebounce`.
*/
import { useState, useEffect } from "react"

export function useDebounce<T>(value: T, delay: number): T {
export function useSearchDebounce<T>(value: T, delay: number): T {
const [debounced, setDebounced] = useState(value)

useEffect(() => {
Expand Down
2 changes: 1 addition & 1 deletion packages/utils/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export { useDebounce } from "./hooks/useDebounce"
export { useSearchDebounce } from "./hooks/useSearchDebounce"
export { usePagination } from "./hooks/usePagination"
export { formatAUD } from "./format/currency"
export { formatDate, formatDateTime } from "./format/date"
1 change: 1 addition & 0 deletions tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"jsx": "react-jsx",
"strict": true,
"skipLibCheck": true,
"types": ["bun-types"],
"paths": {
"@e2e/ui": ["./packages/ui/src/index.ts"],
"@e2e/utils": ["./packages/utils/src/index.ts"]
Expand Down
Loading