From 83a4246a884457dbc6f3a3c452c0faa8c8c14000 Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Thu, 6 Aug 2026 14:45:28 +0900 Subject: [PATCH] refactor: take one product's concepts back out of the package Re-read against a single question: is this a rendering component, or does it need to know something only a product knows. Four answers came back wrong, and all four had arrived by lifting product files rather than designing an interface. The sibling AI package, whose components were written against view models from the start, has none of them. `DataTable` wrote to `localStorage` itself. Where column preferences live, under which key, per user or per workspace, or whether they persist at all, is a decision only a consumer can make; a component that answers it cannot be reused by one that answers differently, and the table stopped working anywhere `localStorage` is absent. It now takes `columnState` and reports changes. `ErrorState` took an error category, one of network, configuration, model, permission, generic. "model" is one product's vocabulary, and the five names resolved to three colours anyway. It now takes a tone and an optional icon. `EmptyState` shipped ten drawings named after one product's screens: chat, models, creations, benchmark, logs, statistics, schedule. No other consumer has a "creations" screen to draw for. `illustration` is a node now, and the drawings go back to the product. This reverses 0.1.0-alpha.3, which unblocked that product's migration by widening the wrong side of the boundary. `Tabs` carried a four-name guide-tag system. That one had already announced itself: the badge variant lived here while the label text had to be passed in from the consumer's locale bundle, so a single concept sat in two repositories. A consumer renders its badge through the `labelExtra` slot that already existed. All four are breaking. They land while there is exactly one consumer and no stable release, which is the cheapest this will ever be. --- CHANGELOG.md | 59 +- package.json | 2 +- src/components/DataTable/DataTable.test.tsx | 67 +- src/components/DataTable/DataTable.tsx | 101 +-- .../EmptyState/EmptyState.example.tsx | 20 +- src/components/EmptyState/EmptyState.test.tsx | 136 +--- src/components/EmptyState/EmptyState.tsx | 72 +- src/components/EmptyState/illustrations.tsx | 620 ------------------ src/components/EmptyState/index.ts | 29 +- src/components/ErrorState/ErrorState.css | 26 +- .../ErrorState/ErrorState.example.tsx | 16 +- src/components/ErrorState/ErrorState.test.tsx | 22 +- src/components/ErrorState/ErrorState.tsx | 38 +- src/components/ErrorState/index.ts | 2 +- src/components/Tabs/Tabs.css | 9 +- src/components/Tabs/Tabs.test.tsx | 161 +---- src/components/Tabs/Tabs.tsx | 67 +- src/components/Tabs/index.ts | 4 +- src/components/Tabs/tabTags.test.ts | 65 -- src/components/Tabs/tabTags.ts | 24 - 20 files changed, 285 insertions(+), 1255 deletions(-) delete mode 100644 src/components/EmptyState/illustrations.tsx delete mode 100644 src/components/Tabs/tabTags.test.ts delete mode 100644 src/components/Tabs/tabTags.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index bc4d995..a62dabe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,62 @@ Versioning follows the policy in [CONTRIBUTING.md](CONTRIBUTING.md#versioning). ## [Unreleased] +## [0.1.0-alpha.6] + +Four places where one product's concepts had come across with the code, found +by re-reading the package against a single question: is this a rendering +component, or does it need to know something only a product knows. All four +are breaking, and they land while there is exactly one consumer. + +### Changed + +- **`DataTable` no longer writes to `localStorage`.** `persistKey` is replaced + by `columnState` and `onColumnStateChange`. Where column preferences live, + under which key, per user or per workspace, or whether they persist at all, + is a decision only the consumer can make, and a component that answers it + cannot be reused by a consumer that answers differently. It also stopped the + table working anywhere `localStorage` is absent. +- **`ErrorState` takes a tone, not an error category.** `type` was + `"network" | "configuration" | "model" | "permission" | "generic"`; `"model"` + in particular is one product's vocabulary. All five resolved to three colours + anyway, so the prop is now `tone: "danger" | "warning" | "accent"`, plus an + `icon` slot. A consumer maps its own categories onto tones. +- **`EmptyState` takes an illustration, not the name of one.** The ten drawings + that shipped here (chat, models, creations, benchmark, logs, statistics, + schedule and the rest) are one product's information architecture; no other + consumer has a "creations" screen to draw for. `illustration` is now a + `ReactNode`, and the drawings move to the product that owns those screens. + This reverses the export added in 0.1.0-alpha.3, which unblocked a consumer + by widening the wrong side of the boundary. +- **`Tabs` no longer carries a guide-tag system.** `tag`, the deprecated + `required`, `TabTagType`, `TabTagLabels`, `TAG_CONFIG` and `tagLabels` are + removed. The arrangement had already split across the boundary, with the + badge variant here and the label text passed in from the consumer's locale + bundle, which is what a wrong boundary looks like. A consumer renders its own + badge through the existing `TabItem.labelExtra` slot and owns both halves; + the `.tabs__tag-badge` class stays for the styling. + +### Migration + +```tsx +// DataTable +- ++ + +// ErrorState +- → tone="danger" +- → tone="warning" +- → tone="accent" + +// EmptyState +- ++ } ... /> + +// Tabs +- { id, label, content, tag: "beta" } ++ { id, label, content, labelExtra: Beta } +``` + ## [0.1.0-alpha.5] ### Fixed @@ -83,7 +139,8 @@ mid-migration. validation, and a clean external React install fixture. - Apache-2.0 license and the initial public boundary rules. -[Unreleased]: https://github.com/lablup/ui-common/compare/v0.1.0-alpha.5...HEAD +[Unreleased]: https://github.com/lablup/ui-common/compare/v0.1.0-alpha.6...HEAD +[0.1.0-alpha.6]: https://github.com/lablup/ui-common/compare/v0.1.0-alpha.5...v0.1.0-alpha.6 [0.1.0-alpha.5]: https://github.com/lablup/ui-common/compare/v0.1.0-alpha.4...v0.1.0-alpha.5 [0.1.0-alpha.4]: https://github.com/lablup/ui-common/compare/v0.1.0-alpha.3...v0.1.0-alpha.4 [0.1.0-alpha.3]: https://github.com/lablup/ui-common/compare/v0.1.0-alpha.2...v0.1.0-alpha.3 diff --git a/package.json b/package.json index 31ff531..2219038 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@lablup/ui-common", - "version": "0.1.0-alpha.5", + "version": "0.1.0-alpha.6", "description": "Shared, product-neutral UI components and design tokens for Lablup products", "license": "Apache-2.0", "author": "Lablup Inc.", diff --git a/src/components/DataTable/DataTable.test.tsx b/src/components/DataTable/DataTable.test.tsx index e611fa9..126015e 100644 --- a/src/components/DataTable/DataTable.test.tsx +++ b/src/components/DataTable/DataTable.test.tsx @@ -135,17 +135,13 @@ describe("DataTable", () => { expect(onClick).toHaveBeenCalledWith({ id: "a", name: "Alpha", score: 3 }); }); - it("loads persisted widths from localStorage on mount", () => { - window.localStorage.setItem( - "dataTable.test-key", - JSON.stringify({ widths: { name: 333 }, visibility: {} }), - ); + it("applies the column widths it is given", () => { const { container } = render( r.id} - persistKey="test-key" + columnState={{ widths: { name: 333 }, visibility: {} }} />, ); const headers = container.querySelectorAll("th"); @@ -153,13 +149,6 @@ describe("DataTable", () => { }); it("hides columns whose visibility flag is false unless alwaysVisible", () => { - window.localStorage.setItem( - "dataTable.test-key-2", - JSON.stringify({ - widths: {}, - visibility: { name: false, id: false }, - }), - ); const cols: DataTableColumn[] = [ { ...COLUMNS[0]!, alwaysVisible: true }, { ...COLUMNS[1]! }, @@ -169,13 +158,63 @@ describe("DataTable", () => { columns={cols} rows={ROWS} getRowKey={(r) => r.id} - persistKey="test-key-2" + columnState={{ widths: {}, visibility: { name: false, id: false } }} />, ); expect(screen.getByText("Name")).toBeInTheDocument(); expect(screen.queryByText("ID")).not.toBeInTheDocument(); }); + it("touches no storage of its own", () => { + const setItem = vi.spyOn(Storage.prototype, "setItem"); + const getItem = vi.spyOn(Storage.prototype, "getItem"); + + render( + r.id} + columnState={{ widths: { name: 200 }, visibility: {} }} + onColumnStateChange={() => {}} + />, + ); + + // Where column preferences live is the consumer's decision, and a host + // without `localStorage` has to keep working. + expect(setItem).not.toHaveBeenCalled(); + expect(getItem).not.toHaveBeenCalled(); + setItem.mockRestore(); + getItem.mockRestore(); + }); + + it("does not report the state it was handed back to the caller", () => { + const onColumnStateChange = vi.fn(); + const columnState = { widths: { name: 250 }, visibility: {} }; + + const { rerender } = render( + r.id} + columnState={columnState} + onColumnStateChange={onColumnStateChange} + />, + ); + rerender( + r.id} + columnState={columnState} + onColumnStateChange={onColumnStateChange} + />, + ); + + // A caller that persists on change and feeds the result back must not + // find itself in a loop. + expect(onColumnStateChange).not.toHaveBeenCalled(); + }); + // ---- Sorting: aria-sort ------------------------------------------------- it("renders aria-sort='none' on sortable columns when no sort is active", () => { diff --git a/src/components/DataTable/DataTable.tsx b/src/components/DataTable/DataTable.tsx index d2e807a..6b0e0e6 100644 --- a/src/components/DataTable/DataTable.tsx +++ b/src/components/DataTable/DataTable.tsx @@ -143,11 +143,22 @@ export interface DataTableProps { /** When `true`, the loading slot replaces the table body. */ loading?: boolean; /** - * When provided, column widths and visibility settings are persisted - * to `localStorage` under this key. Use a stable, namespaced string - * (e.g. `"sessions.activeTab"`). + * Column widths and visibility, controlled by the caller. + * + * This component used to take a `persistKey` and write to `localStorage` + * itself. Storage is a policy decision that belongs to the consumer: where + * it goes, under which key, whether it is per user or per workspace, and + * whether it exists at all in a host that has no `localStorage`. A + * rendering component that answers those questions on its own cannot be + * reused by a consumer that answers them differently. + */ + columnState?: DataTablePersistedState; + /** + * Called whenever the user resizes a column or toggles its visibility. + * Pair it with `columnState` to persist wherever the consumer keeps + * preferences; omit both to get a table that forgets on unmount. */ - persistKey?: string; + onColumnStateChange?: (state: DataTablePersistedState) => void; /** Extra class for the table's wrapping element. */ className?: string; /** ARIA label for the table. Defaults to "Data table". */ @@ -190,58 +201,7 @@ export interface DataTableProps { ) => void; } -// ============================================================================ -// Storage helpers -// ============================================================================ - -const STORAGE_NAMESPACE = "dataTable"; - -function buildStorageKey(persistKey: string): string { - return `${STORAGE_NAMESPACE}.${persistKey}`; -} - -/** - * Load persisted state from `localStorage`. Returns an empty state on - * any failure (missing storage, malformed JSON, schema drift) so the - * component degrades gracefully. - */ -function loadPersistedState(persistKey: string | undefined): DataTablePersistedState { - if (!persistKey || typeof window === "undefined") { - return { widths: {}, visibility: {} }; - } - try { - const raw = window.localStorage.getItem(buildStorageKey(persistKey)); - if (!raw) return { widths: {}, visibility: {} }; - const parsed = JSON.parse(raw) as unknown; - if (typeof parsed !== "object" || parsed === null) { - return { widths: {}, visibility: {} }; - } - const obj = parsed as { widths?: unknown; visibility?: unknown }; - const widths = - typeof obj.widths === "object" && obj.widths !== null - ? (obj.widths as Record) - : {}; - const visibility = - typeof obj.visibility === "object" && obj.visibility !== null - ? (obj.visibility as Record) - : {}; - return { widths, visibility }; - } catch { - return { widths: {}, visibility: {} }; - } -} - -function savePersistedState( - persistKey: string | undefined, - state: DataTablePersistedState, -): void { - if (!persistKey || typeof window === "undefined") return; - try { - window.localStorage.setItem(buildStorageKey(persistKey), JSON.stringify(state)); - } catch { - // Quota exceeded / disabled storage — silently ignore. - } -} +const EMPTY_COLUMN_STATE: DataTablePersistedState = { widths: {}, visibility: {} }; // ============================================================================ // Sorting helpers @@ -389,7 +349,8 @@ function DataTableInner({ emptyState, loadingState, loading = false, - persistKey, + columnState, + onColumnStateChange, className = "", ariaLabel = "Data table", testId, @@ -399,16 +360,17 @@ function DataTableInner({ sortDirection: controlledSortDirection, onSortChange, }: DataTableProps) { - // ---- Persisted state (widths + visibility) ------------------------------- - const [persisted, setPersisted] = useState(() => - loadPersistedState(persistKey), + // ---- Column state (widths + visibility) ---------------------------------- + // + // Held internally so a caller that does not care about persistence gets a + // working table, and re-seeded whenever the caller supplies a new one. + const [persisted, setPersisted] = useState( + () => columnState ?? EMPTY_COLUMN_STATE, ); - // Refresh persisted state when the storage key changes (defensive — the - // expected use case is a stable key). useEffect(() => { - setPersisted(loadPersistedState(persistKey)); - }, [persistKey]); + if (columnState) setPersisted(columnState); + }, [columnState]); // ---- Sorting state (uncontrolled fallback) ------------------------------- // @@ -537,10 +499,15 @@ function DataTableInner({ [], ); - // Persist when state changes + // Report changes so the caller can persist them. Skipped while `persisted` + // still holds what the caller last handed in, so echoing the callback back + // through `columnState` does not loop. + const reported = useRef(persisted); useEffect(() => { - savePersistedState(persistKey, persisted); - }, [persistKey, persisted]); + if (reported.current === persisted) return; + reported.current = persisted; + onColumnStateChange?.(persisted); + }, [persisted, onColumnStateChange]); // ---- Sorted rows -------------------------------------------------------- const sortedRows = useMemo(() => { diff --git a/src/components/EmptyState/EmptyState.example.tsx b/src/components/EmptyState/EmptyState.example.tsx index b7b0f61..7b72235 100644 --- a/src/components/EmptyState/EmptyState.example.tsx +++ b/src/components/EmptyState/EmptyState.example.tsx @@ -46,7 +46,7 @@ export function EmptyStateExamples() {

Chat Empty State

} title="No conversations yet" description="Start a new chat to begin interacting with your AI model" primaryAction={{ @@ -64,7 +64,7 @@ export function EmptyStateExamples() {

Models Empty State

} title="No models installed" description="Download a model from Hugging Face to get started" primaryAction={{ @@ -82,7 +82,7 @@ export function EmptyStateExamples() {

Creations Empty State

} title="No creations yet" description="Your generated images and content will appear here" primaryAction={{ @@ -96,7 +96,7 @@ export function EmptyStateExamples() {

Benchmark Empty State

} title="No benchmarks run" description="Run a benchmark to compare model performance" primaryAction={{ @@ -114,7 +114,7 @@ export function EmptyStateExamples() {

Logs Empty State

} title="No logs available" description="Application logs will appear here when events occur" /> @@ -124,7 +124,7 @@ export function EmptyStateExamples() {

Statistics Empty State

} title="No statistics yet" description="Usage statistics will be displayed once you start using the application" /> @@ -134,7 +134,7 @@ export function EmptyStateExamples() {

Error Empty State

} title="Something went wrong" description="We encountered an error while loading your data" primaryAction={{ @@ -152,7 +152,7 @@ export function EmptyStateExamples() {

Generic Empty State

} title="No items found" description="There are no items to display at this time" primaryAction={{ @@ -166,7 +166,7 @@ export function EmptyStateExamples() {

Without Illustration

} title="Simple Empty State" description="This example doesn't show an illustration" showIllustration={false} @@ -181,7 +181,7 @@ export function EmptyStateExamples() {

Without Actions

} title="Informational Only" description="This empty state provides information without any actions" /> diff --git a/src/components/EmptyState/EmptyState.test.tsx b/src/components/EmptyState/EmptyState.test.tsx index eebc633..842b524 100644 --- a/src/components/EmptyState/EmptyState.test.tsx +++ b/src/components/EmptyState/EmptyState.test.tsx @@ -6,13 +6,12 @@ import { describe, it, expect, vi } from "vitest"; import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { EmptyState } from "./EmptyState"; -import { BenchmarkIllustration } from "./illustrations"; describe("EmptyState", () => { it("renders with required props", () => { render( } title="Test Title" description="Test Description" />, @@ -24,7 +23,11 @@ describe("EmptyState", () => { it("renders illustration by default", () => { const { container } = render( - , + } + title="Test" + description="Test" + />, ); const illustration = container.querySelector(".empty-state__illustration"); @@ -34,7 +37,7 @@ describe("EmptyState", () => { it("hides illustration when showIllustration is false", () => { const { container } = render( } title="Test" description="Test" showIllustration={false} @@ -49,7 +52,7 @@ describe("EmptyState", () => { const handleClick = vi.fn(); render( } title="Test" description="Test" primaryAction={{ @@ -69,7 +72,7 @@ describe("EmptyState", () => { render( } title="Test" description="Test" primaryAction={{ @@ -88,7 +91,7 @@ describe("EmptyState", () => { it("renders secondary action as link when href is provided", () => { render( } title="Test" description="Test" secondaryAction={{ @@ -107,7 +110,7 @@ describe("EmptyState", () => { const handleClick = vi.fn(); render( } title="Test" description="Test" secondaryAction={{ @@ -127,7 +130,7 @@ describe("EmptyState", () => { render( } title="Test" description="Test" secondaryAction={{ @@ -146,7 +149,7 @@ describe("EmptyState", () => { it("applies custom className", () => { const { container } = render( } title="Test" description="Test" className="custom-class" @@ -157,40 +160,27 @@ describe("EmptyState", () => { expect(emptyState).toHaveClass("custom-class"); }); - it("applies illustration type class", () => { - const { container } = render( - , + it("renders the illustration it is given", () => { + render( + } + title="Test" + description="Test" + />, ); - const emptyState = container.querySelector(".empty-state"); - expect(emptyState).toHaveClass("empty-state--benchmark"); - }); - - it("renders all illustration types correctly", () => { - const types = [ - "chat", - "models", - "creations", - "benchmark", - "logs", - "statistics", - "error", - "generic", - ] as const; - - types.forEach((type) => { - const { container } = render( - , - ); - - const emptyState = container.querySelector(".empty-state"); - expect(emptyState).toHaveClass(`empty-state--${type}`); - }); + // The ten named types this replaced were one product's screens, and the + // modifier class they produced styled nothing. + expect(screen.getByTestId("illustration")).toBeInTheDocument(); }); it("has proper accessibility attributes", () => { const { container } = render( - , + } + title="Test" + description="Test" + />, ); const emptyState = container.querySelector(".empty-state"); @@ -200,7 +190,11 @@ describe("EmptyState", () => { it("does not render actions section when no actions provided", () => { const { container } = render( - , + } + title="Test" + description="Test" + />, ); const actionsSection = container.querySelector(".empty-state__actions"); @@ -213,7 +207,7 @@ describe("EmptyState", () => { render( } title="Test" description="Test" primaryAction={{ @@ -231,65 +225,3 @@ describe("EmptyState", () => { expect(screen.getByRole("button", { name: "Secondary" })).toBeInTheDocument(); }); }); - -describe("BenchmarkIllustration", () => { - it("renders with the illustration-needle animation class on the needle group", () => { - const { container } = render(); - - const needleGroup = container.querySelector(".illustration-needle"); - expect(needleGroup).toBeInTheDocument(); - }); - - it("needle group is a element wrapping a ", () => { - const { container } = render(); - - const needleGroup = container.querySelector("g.illustration-needle"); - expect(needleGroup).toBeInTheDocument(); - expect(needleGroup?.tagName.toLowerCase()).toBe("g"); - - const needlePath = needleGroup?.querySelector("path"); - expect(needlePath).toBeInTheDocument(); - }); - - it("speedometer body paths are rendered outside the needle group", () => { - const { container } = render(); - - const svg = container.querySelector("svg"); - expect(svg).toBeInTheDocument(); - - // There should be paths outside the needle group (speedometer body) - const allPaths = svg?.querySelectorAll("path"); - const needleGroup = svg?.querySelector("g.illustration-needle"); - const needlePaths = needleGroup?.querySelectorAll("path"); - - expect(allPaths?.length).toBeGreaterThan(needlePaths?.length ?? 0); - }); - - it("accepts and applies className prop", () => { - const { container } = render(); - - const svg = container.querySelector("svg"); - expect(svg).toHaveClass("custom-class"); - }); - - it("has aria-hidden attribute for accessibility", () => { - const { container } = render(); - - const svg = container.querySelector("svg"); - expect(svg).toHaveAttribute("aria-hidden", "true"); - }); - - it("renders correctly when used inside EmptyState with benchmark illustration type", () => { - const { container } = render( - , - ); - - const needleGroup = container.querySelector(".illustration-needle"); - expect(needleGroup).toBeInTheDocument(); - expect(needleGroup?.tagName.toLowerCase()).toBe("g"); - }); -}); diff --git a/src/components/EmptyState/EmptyState.tsx b/src/components/EmptyState/EmptyState.tsx index 6b91791..467df79 100644 --- a/src/components/EmptyState/EmptyState.tsx +++ b/src/components/EmptyState/EmptyState.tsx @@ -17,32 +17,8 @@ import { memo, useCallback, useMemo, type ReactNode } from "react"; import { Button } from "../Button"; -import { - ChatIllustration, - ModelsIllustration, - CreationsIllustration, - TextIllustration, - BenchmarkIllustration, - LogsIllustration, - StatisticsIllustration, - ErrorIllustration, - ScheduleIllustration, - GenericIllustration, -} from "./illustrations"; import "./EmptyState.css"; -export type IllustrationType = - | "chat" - | "models" - | "creations" - | "text" - | "benchmark" - | "logs" - | "statistics" - | "error" - | "schedule" - | "generic"; - export interface EmptyStateAction { label: string; onClick: () => void; @@ -55,8 +31,16 @@ export interface EmptyStateSecondaryAction { } export interface EmptyStateProps { - /** Type of illustration to display */ - illustration: IllustrationType; + /** + * The drawing shown above the text. + * + * This used to be one of ten names: chat, models, creations, benchmark, + * logs, statistics, schedule and so on, each resolving to an SVG shipped + * inside this package. Those names are one product's information + * architecture, and no other consumer has a "creations" screen to draw for. + * A consumer passes its own artwork and keeps its own vocabulary. + */ + illustration?: ReactNode; /** Main heading */ title: string; /** Descriptive text */ @@ -80,26 +64,6 @@ export interface EmptyStateProps { showIllustration?: boolean; } -/** - * Static map of illustration types to components. - * Defined outside component to avoid recreation on each render. - */ -const illustrationMap: Record< - IllustrationType, - React.ComponentType<{ className?: string }> -> = { - chat: ChatIllustration, - models: ModelsIllustration, - creations: CreationsIllustration, - text: TextIllustration, - benchmark: BenchmarkIllustration, - logs: LogsIllustration, - statistics: StatisticsIllustration, - error: ErrorIllustration, - schedule: ScheduleIllustration, - generic: GenericIllustration, -}; - /** * EmptyState Component * @@ -128,24 +92,16 @@ function EmptyStateComponent({ secondaryOnClick?.(); }, [secondaryOnClick]); - // Memoize illustration component lookup - const Illustration = useMemo(() => illustrationMap[illustration], [illustration]); - // Memoize class name computation const containerClass = useMemo( - () => - ["empty-state", `empty-state--${illustration}`, className] - .filter(Boolean) - .join(" "), - [illustration, className], + () => ["empty-state", className].filter(Boolean).join(" "), + [className], ); return (
- {showIllustration && ( -
- -
+ {showIllustration && illustration && ( +
{illustration}
)}
diff --git a/src/components/EmptyState/illustrations.tsx b/src/components/EmptyState/illustrations.tsx deleted file mode 100644 index f4e9768..0000000 --- a/src/components/EmptyState/illustrations.tsx +++ /dev/null @@ -1,620 +0,0 @@ -/** - * EmptyState Illustrations - * - * SVG illustration components for different empty state contexts. - * All illustrations are inline React components for optimal performance. - * Each component is memoized to prevent unnecessary re-renders. - */ - -import { memo } from "react"; - -export interface IllustrationProps { - className?: string; -} - -/** - * Chat illustration - Speech bubbles with sparkles - */ -export const ChatIllustration = memo(function ChatIllustration({ - className, -}: IllustrationProps) { - return ( - - ); -}); - -/** - * Models illustration - Package/download box - */ -export const ModelsIllustration = memo(function ModelsIllustration({ - className, -}: IllustrationProps) { - return ( - - ); -}); - -/** - * Creations illustration - Gallery frame - */ -export const CreationsIllustration = memo(function CreationsIllustration({ - className, -}: IllustrationProps) { - return ( - - ); -}); - -/** - * Benchmark illustration - Speedometer - */ -export const BenchmarkIllustration = memo(function BenchmarkIllustration({ - className, -}: IllustrationProps) { - return ( - - ); -}); - -/** - * Logs illustration - Document stack - */ -export const LogsIllustration = memo(function LogsIllustration({ - className, -}: IllustrationProps) { - return ( - - ); -}); - -/** - * Statistics illustration - Chart placeholder - */ -export const StatisticsIllustration = memo(function StatisticsIllustration({ - className, -}: IllustrationProps) { - return ( - - ); -}); - -/** - * Error illustration - Warning/error state - */ -export const ErrorIllustration = memo(function ErrorIllustration({ - className, -}: IllustrationProps) { - return ( - - ); -}); - -/** - * Text illustration - Document with bookmark - */ -export const TextIllustration = memo(function TextIllustration({ - className, -}: IllustrationProps) { - return ( - - ); -}); - -/** - * Schedule illustration - Calendar with clock - */ -export const ScheduleIllustration = memo(function ScheduleIllustration({ - className, -}: IllustrationProps) { - return ( - - ); -}); - -/** - * Generic illustration - Placeholder icon - */ -export const GenericIllustration = memo(function GenericIllustration({ - className, -}: IllustrationProps) { - return ( - - ); -}); diff --git a/src/components/EmptyState/index.ts b/src/components/EmptyState/index.ts index caf04aa..822c7da 100644 --- a/src/components/EmptyState/index.ts +++ b/src/components/EmptyState/index.ts @@ -1,5 +1,10 @@ /** * EmptyState Component - Barrel Export + * + * The ten illustrations that used to ship here were one product's information + * architecture: chat, models, creations, benchmark, logs, statistics, + * schedule. No other consumer has a "creations" screen to draw for. A consumer + * passes its own artwork through `illustration` and keeps its own vocabulary. */ export { EmptyState } from "./EmptyState"; @@ -7,28 +12,4 @@ export type { EmptyStateProps, EmptyStateAction, EmptyStateSecondaryAction, - IllustrationType, } from "./EmptyState"; - -/** - * The illustrations are also exported on their own. - * - * They shipped inside the package from the first release but no barrel named - * them, so a consumer that composes its own empty state, which is the case - * `EmptyState` cannot cover, had no way to reach one. Same shape as the - * stylesheets in 0.1.0-alpha.1: present in the tarball, addressable by - * nothing. - */ -export { - ChatIllustration, - ModelsIllustration, - CreationsIllustration, - BenchmarkIllustration, - LogsIllustration, - StatisticsIllustration, - ErrorIllustration, - TextIllustration, - ScheduleIllustration, - GenericIllustration, -} from "./illustrations"; -export type { IllustrationProps } from "./illustrations"; diff --git a/src/components/ErrorState/ErrorState.css b/src/components/ErrorState/ErrorState.css index 9b70c56..4469c04 100644 --- a/src/components/ErrorState/ErrorState.css +++ b/src/components/ErrorState/ErrorState.css @@ -80,34 +80,20 @@ * Error Type Variants * ============================================ */ -/* Network Error - Connection issues */ -.error-state--network .error-state__icon { - color: var(--token-colorWarning, #9a5d00); - background-color: rgba(250, 173, 20, 0.1); -} - -/* Configuration Error - Setup/config issues */ -.error-state--configuration .error-state__icon { - color: var(--token-colorPrimary, #8b5cf6); - background-color: var(--token-colorPrimaryBg, rgba(139, 92, 246, 0.1)); -} - -/* Model Error - Model-related issues */ -.error-state--model .error-state__icon { +/* Tones. A consumer maps its own error categories onto these. */ +.error-state--danger .error-state__icon { color: var(--token-colorError, #c82333); background-color: rgba(255, 77, 79, 0.1); } -/* Permission Error - Access denied */ -.error-state--permission .error-state__icon { +.error-state--warning .error-state__icon { color: var(--token-colorWarning, #9a5d00); background-color: rgba(250, 173, 20, 0.1); } -/* Generic Error - Default fallback */ -.error-state--generic .error-state__icon { - color: var(--token-colorError, #c82333); - background-color: rgba(255, 77, 79, 0.1); +.error-state--accent .error-state__icon { + color: var(--token-colorPrimary, #8b5cf6); + background-color: var(--token-colorPrimaryBg, rgba(139, 92, 246, 0.1)); } /* ============================================ diff --git a/src/components/ErrorState/ErrorState.example.tsx b/src/components/ErrorState/ErrorState.example.tsx index 019cc90..ff49b16 100644 --- a/src/components/ErrorState/ErrorState.example.tsx +++ b/src/components/ErrorState/ErrorState.example.tsx @@ -14,7 +14,7 @@ import { ErrorState } from "./ErrorState"; export function NetworkErrorExample() { return ( @@ -147,7 +147,7 @@ export function MinimalErrorExample() { export function CompactErrorExample() { return ( { describe("Error Types", () => { it("applies network error type class", () => { - render(); + render(); - const container = document.querySelector(".error-state--network"); + const container = document.querySelector(".error-state--warning"); expect(container).toBeInTheDocument(); }); it("applies configuration error type class", () => { - render(); + render(); - const container = document.querySelector(".error-state--configuration"); + const container = document.querySelector(".error-state--accent"); expect(container).toBeInTheDocument(); }); it("applies model error type class", () => { - render(); + render(); - const container = document.querySelector(".error-state--model"); + const container = document.querySelector(".error-state--danger"); expect(container).toBeInTheDocument(); }); it("applies permission error type class", () => { - render(); + render(); - const container = document.querySelector(".error-state--permission"); + const container = document.querySelector(".error-state--warning"); expect(container).toBeInTheDocument(); }); it("applies generic error type class by default", () => { render(); - const container = document.querySelector(".error-state--generic"); + const container = document.querySelector(".error-state--danger"); expect(container).toBeInTheDocument(); }); }); @@ -178,14 +178,14 @@ describe("ErrorState", () => { it("preserves error type class when custom className is applied", () => { render( , ); - const container = document.querySelector(".error-state--network.custom-error"); + const container = document.querySelector(".error-state--warning.custom-error"); expect(container).toBeInTheDocument(); }); }); diff --git a/src/components/ErrorState/ErrorState.tsx b/src/components/ErrorState/ErrorState.tsx index 22ed770..5531ecc 100644 --- a/src/components/ErrorState/ErrorState.tsx +++ b/src/components/ErrorState/ErrorState.tsx @@ -5,19 +5,29 @@ * Provides consistent error presentation across all pages with actionable buttons. * * Features: - * - Error type variants (network, configuration, model, permission, generic) + * - Three visual tones * - Primary and secondary action buttons * - Accessible (ARIA attributes, focus management) * - Dark/light theme support */ +import type { ReactNode } from "react"; import { useCallback } from "react"; import { AlertCircleIcon } from "../../icons/AlertCircleIcon"; import { Button } from "../Button"; import "./ErrorState.css"; -export type ErrorType = - "network" | "configuration" | "model" | "permission" | "generic"; +/** + * How the error reads, not what it is about. + * + * This prop used to be a union of five names: network, configuration, model, + * permission, generic. Two of those, and "model" in particular, are one + * product's categories rather than anything a shared component can reason + * about, and all five resolved to three colours anyway: network and permission + * were the same amber, model and generic the same red. A consumer decides + * which of its own error categories reads as which tone. + */ +export type ErrorTone = "danger" | "warning" | "accent"; export interface ErrorAction { label: string; @@ -25,8 +35,10 @@ export interface ErrorAction { } export interface ErrorStateProps { - /** Error type determines icon and default styling */ - type?: ErrorType; + /** How the error reads. Defaults to `danger`. */ + tone?: ErrorTone; + /** Replaces the default alert icon. */ + icon?: ReactNode; /** Error title - main heading */ title: string; /** Detailed error message */ @@ -41,20 +53,12 @@ export interface ErrorStateProps { showIcon?: boolean; } -/** - * Get icon for error type - */ -function getErrorIcon(_type: ErrorType): React.ReactNode { - // All error types use the same AlertCircleIcon for now - // Can be extended with different icons per type in the future - return ; -} - /** * ErrorState Component */ export function ErrorState({ - type = "generic", + tone = "danger", + icon, title, message, primaryAction, @@ -70,7 +74,7 @@ export function ErrorState({ secondaryAction?.onClick(); }, [secondaryAction]); - const containerClass = ["error-state", `error-state--${type}`, className] + const containerClass = ["error-state", `error-state--${tone}`, className] .filter(Boolean) .join(" "); @@ -78,7 +82,7 @@ export function ErrorState({
{showIcon && ( )} diff --git a/src/components/ErrorState/index.ts b/src/components/ErrorState/index.ts index 1a61d6c..661b225 100644 --- a/src/components/ErrorState/index.ts +++ b/src/components/ErrorState/index.ts @@ -3,4 +3,4 @@ */ export { ErrorState } from "./ErrorState"; -export type { ErrorStateProps, ErrorType, ErrorAction } from "./ErrorState"; +export type { ErrorStateProps, ErrorTone, ErrorAction } from "./ErrorState"; diff --git a/src/components/Tabs/Tabs.css b/src/components/Tabs/Tabs.css index dadb6c9..3523144 100644 --- a/src/components/Tabs/Tabs.css +++ b/src/components/Tabs/Tabs.css @@ -112,7 +112,14 @@ display: inline-block; } -/* Guide tag badge rendered after the tab label */ +/** + * Styling for a small badge a consumer renders through `TabItem.labelExtra`. + * + * The component no longer renders one itself: the four-name guide vocabulary + * that used to drive it belongs to the consumer, along with its wording. This + * class stays so the arrangement still looks right when a consumer opts into + * that shape, and it is documented in the README rather than applied here. + */ .tabs__tag-badge { font-size: var(--token-fontSizeXXS, 0.5rem); line-height: 1; diff --git a/src/components/Tabs/Tabs.test.tsx b/src/components/Tabs/Tabs.test.tsx index 43eebd2..e8c4755 100644 --- a/src/components/Tabs/Tabs.test.tsx +++ b/src/components/Tabs/Tabs.test.tsx @@ -7,7 +7,7 @@ * - Keyboard navigation (Arrow keys, Home, End) * - Accessibility attributes (ARIA roles, aria-selected) * - Controlled vs uncontrolled state - * - Required badge display + * - The labelExtra slot * - Group separators and labels * - Overflow menu (overflowMode="menu") * - Mobile dropdown (overflowMode="dropdown") @@ -17,14 +17,7 @@ */ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { - render, - screen, - within, - waitFor, - fireEvent, - act, -} from "@testing-library/react"; +import { render, screen, waitFor, fireEvent, act } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { Tabs, type TabItem, type TabGroupMeta } from "./Tabs"; @@ -371,7 +364,6 @@ describe("Tabs", () => { label: "Tab 2", content:
Content 2
, groupId: "group1", - required: true, }, { id: "tab3", @@ -420,22 +412,6 @@ describe("Tabs", () => { expect(separator).toHaveAttribute("role", "separator"); expect(separator).toHaveAttribute("aria-hidden", "true"); }); - - it("should render required badge for required tabs", () => { - render(); - - const tab2 = screen.getByRole("tab", { name: /Tab 2/i }); - const requiredBadge = within(tab2).getByText("Required"); - expect(requiredBadge).toBeInTheDocument(); - }); - - it("should not render required badge for non-required tabs", () => { - render(); - - const tab1 = screen.getByRole("tab", { name: "Tab 1" }); - const badge = within(tab1).queryByText("Required"); - expect(badge).not.toBeInTheDocument(); - }); }); describe("Overflow Menu (overflowMode='menu')", () => { @@ -741,139 +717,24 @@ describe("Tabs", () => { }); }); - describe("Guide Tags", () => { - it("should render experimental tag badge after tab label", () => { - const tabsWithTag: TabItem[] = [ - { - id: "tab-exp", - label: "ACP", - content:
ACP Content
, - tag: "experimental", - }, - { id: "tab-normal", label: "Normal", content:
Normal
}, - ]; - - const { container } = render(); - - const badge = container.querySelector(".tabs__tag-badge"); - expect(badge).toBeInTheDocument(); - expect(badge).toHaveTextContent("Experimental"); - }); - - it("should render beta tag with info variant class", () => { - const tabsWithBeta: TabItem[] = [ - { - id: "tab-beta", - label: "Beta Feature", - content:
Beta
, - tag: "beta", - }, - ]; - - const { container } = render(); - - const badge = container.querySelector(".tabs__tag-badge"); - expect(badge).toBeInTheDocument(); - expect(badge).toHaveTextContent("Beta"); - // Badge should use info variant - expect(badge).toHaveClass("badge--info"); - }); - - it("should render recommended tag with primary variant class", () => { - const tabsWithRecommended: TabItem[] = [ - { - id: "tab-rec", - label: "Recommended Tab", - content:
Recommended
, - tag: "recommended", - }, - ]; - - const { container } = render(); - - const badge = container.querySelector(".tabs__tag-badge"); - expect(badge).toBeInTheDocument(); - expect(badge).toHaveTextContent("Recommended"); - expect(badge).toHaveClass("badge--primary"); - }); - - it("should maintain backward compatibility with required boolean prop", () => { - const tabsWithRequired: TabItem[] = [ - { - id: "tab-req", - label: "Required Tab", - content:
Required
, - required: true, - }, - ]; - - const { container } = render(); - - const badge = container.querySelector(".tabs__tag-badge"); - expect(badge).toBeInTheDocument(); - expect(badge).toHaveTextContent("Required"); - // Required uses warning variant - expect(badge).toHaveClass("badge--warning"); - }); - - it("should not render tag badge for tabs without tag or required", () => { - const { container } = render(); - - const badges = container.querySelectorAll(".tabs__tag-badge"); - expect(badges).toHaveLength(0); - }); - - it("should prefer tag prop over required boolean when both are set", () => { - const tabsWithBoth: TabItem[] = [ - { - id: "tab-both", - label: "Conflicted Tab", - content:
Both
, - required: true, - tag: "beta", - }, - ]; - - const { container } = render(); - - const badge = container.querySelector(".tabs__tag-badge"); - expect(badge).toBeInTheDocument(); - // Should use `tag` (beta) not `required` - expect(badge).toHaveClass("badge--info"); - expect(badge).toHaveTextContent("Beta"); - }); - - it("should render tag badge in overflow menu items", async () => { - const tabsWithTag: TabItem[] = [ + describe("labelExtra", () => { + it("renders a trailing node after the tab label", () => { + const tabs: TabItem[] = [ { id: "tab-exp", label: "ACP", content:
ACP Content
, - tag: "experimental", + labelExtra: Experimental, }, { id: "tab-normal", label: "Normal", content:
Normal
}, ]; - const { container } = render(); - - const overflowButton = container.querySelector( - ".tabs__overflow-btn", - ) as HTMLButtonElement; - - act(() => { - fireEvent.click(overflowButton); - }); + render(); - await waitFor(() => { - const menu = container.querySelector(".tabs__overflow-menu"); - expect(menu).toBeInTheDocument(); - // The experimental tab's badge should appear in the menu - const menuItems = container.querySelectorAll(".tabs__overflow-item"); - const expMenuItem = menuItems[0]; - const menuBadge = expMenuItem?.querySelector(".badge"); - expect(menuBadge).toBeInTheDocument(); - expect(menuBadge).toHaveTextContent("Experimental"); - }); + // The four-name guide vocabulary this replaced was one product's, and + // its label text had to come from that product's locale bundle anyway. + // A slot is what a shared component can offer. + expect(screen.getByTestId("trailing")).toHaveTextContent("Experimental"); }); }); diff --git a/src/components/Tabs/Tabs.tsx b/src/components/Tabs/Tabs.tsx index 377d9bf..50c3be4 100644 --- a/src/components/Tabs/Tabs.tsx +++ b/src/components/Tabs/Tabs.tsx @@ -25,41 +25,22 @@ import { type KeyboardEvent, type TouchEvent, } from "react"; -import { Badge } from "../Badge"; -import { type TabTagType, type TabTagLabels, TAG_CONFIG } from "./tabTags"; import "./Tabs.css"; -// Re-export for consumers that import from Tabs.tsx directly -export type { TabTagType, TabTagLabels } from "./tabTags"; -export { TAG_CONFIG } from "./tabTags"; - -/** English default for every {@link TabTagType} badge label. */ -const DEFAULT_TAG_LABELS: TabTagLabels = { - required: "Required", - recommended: "Recommended", - beta: "Beta", - experimental: "Experimental", -}; - export interface TabItem { id: string; label: string; content: ReactNode; /** - * Show a "Required" badge next to the tab label. - * @deprecated Use `tag="required"` instead. Kept for backward compatibility. - */ - required?: boolean; - /** - * Guide tag displayed after the tab label as a small badge. - * When set, takes precedence over the `required` boolean. - */ - tag?: TabTagType; - /** - * Optional extra trailing node rendered next to the tab label (e.g. a - * count badge). Distinct from `tag` (which uses a fixed preset). Use this - * when migrating segmented controls that rendered their own inline count - * pill alongside the label. + * Trailing node rendered next to the tab label: a count pill, a status + * badge, whatever the consumer's vocabulary calls for. + * + * This used to sit beside a `tag` prop that took one of four fixed names, + * required / recommended / beta / experimental, and rendered a Badge for + * it. Those are one product's guide vocabulary, and the arrangement had + * already split across the boundary: the badge variant lived here while the + * label text had to be passed in from the consumer's locale bundle. A + * consumer that wants that badge renders it here, and owns both halves. */ labelExtra?: ReactNode; /** @@ -141,11 +122,6 @@ export interface TabsProps { scrollLeftLabel?: string; /** Accessible label for the right scroll-arrow button. Default: "Scroll right" */ scrollRightLabel?: string; - /** - * English defaults for every guide-tag badge label (required / recommended - * / beta / experimental). Override individual entries to translate them. - */ - tagLabels?: TabTagLabels; } // ============================================================================ @@ -171,7 +147,6 @@ export function Tabs({ selectTabLabel = "Select tab", scrollLeftLabel = "Scroll left", scrollRightLabel = "Scroll right", - tagLabels = DEFAULT_TAG_LABELS, }: TabsProps) { // Auto-detect overflow mode from groups presence const overflowMode = @@ -564,20 +539,9 @@ export function Tabs({ // Reset menu item refs array each render menuItemRefs.current = []; - /** Resolve the effective tag type for a tab (prefers `tag`, falls back to legacy `required`) */ - const resolveTabTag = (tab: TabItem): TabTagType | null => { - if (tab.tag) return tab.tag; - - if (tab.required) return "required"; - return null; - }; - /** Render a single tab button */ const renderTabButton = (tab: TabItem, globalIndex: number) => { const isActive = tab.id === activeTab; - const effectiveTag = resolveTabTag(tab); - const tagConfig = effectiveTag ? TAG_CONFIG[effectiveTag] : null; - const tagLabel = effectiveTag ? tagLabels[effectiveTag] : null; return ( ); @@ -697,9 +656,6 @@ export function Tabs({ {groupTabs.map((tab) => { const isActive = tab.id === activeTab; const flatIndex = flattenedMenuTabs.findIndex((t) => t.id === tab.id); - const effectiveTag = resolveTabTag(tab); - const tagConfig = effectiveTag ? TAG_CONFIG[effectiveTag] : null; - const tagLabel = effectiveTag ? tagLabels[effectiveTag] : null; return ( ); diff --git a/src/components/Tabs/index.ts b/src/components/Tabs/index.ts index 4aad076..66d087d 100644 --- a/src/components/Tabs/index.ts +++ b/src/components/Tabs/index.ts @@ -1,10 +1,8 @@ -export { Tabs, TAG_CONFIG } from "./Tabs"; +export { Tabs } from "./Tabs"; export type { TabsProps, TabItem, TabGroupMeta, TabOverflowMode, - TabTagType, - TabTagLabels, TabVariant, } from "./Tabs"; diff --git a/src/components/Tabs/tabTags.test.ts b/src/components/Tabs/tabTags.test.ts deleted file mode 100644 index 8e6e297..0000000 --- a/src/components/Tabs/tabTags.test.ts +++ /dev/null @@ -1,65 +0,0 @@ -/** - * Tests for tabTags shared module - * - * Tests cover: - * - TabTagType type completeness (all four variants are defined) - * - TAG_CONFIG covers every TabTagType key - * - Each tag config entry has the expected badge variant - */ - -import { describe, it, expect } from "vitest"; -import { TAG_CONFIG } from "./tabTags"; -import type { TabTagType } from "./tabTags"; - -describe("tabTags", () => { - describe("TAG_CONFIG completeness", () => { - it("should define an entry for every TabTagType variant", () => { - const expectedKeys: TabTagType[] = [ - "required", - "recommended", - "beta", - "experimental", - ]; - - for (const key of expectedKeys) { - expect(TAG_CONFIG).toHaveProperty(key); - } - }); - - it("should not contain unexpected keys", () => { - const keys = Object.keys(TAG_CONFIG); - expect(keys).toHaveLength(4); - expect(keys.sort()).toEqual( - ["beta", "experimental", "recommended", "required"].sort(), - ); - }); - }); - - describe("TAG_CONFIG entries — badge variants", () => { - it("should map 'required' to warning variant", () => { - expect(TAG_CONFIG.required.variant).toBe("warning"); - }); - - it("should map 'recommended' to primary variant", () => { - expect(TAG_CONFIG.recommended.variant).toBe("primary"); - }); - - it("should map 'beta' to info variant", () => { - expect(TAG_CONFIG.beta.variant).toBe("info"); - }); - - it("should map 'experimental' to warning variant", () => { - expect(TAG_CONFIG.experimental.variant).toBe("warning"); - }); - }); - - describe("TAG_CONFIG shape", () => { - it("should have only a variant field on every entry", () => { - for (const [, config] of Object.entries(TAG_CONFIG)) { - expect(config).toHaveProperty("variant"); - expect(typeof config.variant).toBe("string"); - expect(Object.keys(config)).toEqual(["variant"]); - } - }); - }); -}); diff --git a/src/components/Tabs/tabTags.ts b/src/components/Tabs/tabTags.ts deleted file mode 100644 index 99cde17..0000000 --- a/src/components/Tabs/tabTags.ts +++ /dev/null @@ -1,24 +0,0 @@ -/** - * Tag label types and badge-variant configuration for the Tabs component. - * - * Used to display guide badge labels (Beta, Experimental, Required, Recommended) - * next to tab labels. The label text itself is supplied by the consumer via - * `Tabs`' `tagLabels` prop (see `TabTagLabels`) — this module only carries the - * presentation mapping from tag type to badge variant. - */ - -import type { BadgeProps } from "../Badge"; - -/** Guide tag types — displayed as a small badge after the label */ -export type TabTagType = "required" | "recommended" | "beta" | "experimental"; - -/** English default label for every {@link TabTagType}. */ -export type TabTagLabels = Record; - -/** Mapping of tag type to badge variant */ -export const TAG_CONFIG: Record = { - required: { variant: "warning" }, - recommended: { variant: "primary" }, - beta: { variant: "info" }, - experimental: { variant: "warning" }, -};