From 29bb914a95c3cb257fa0a42f834f3ae9bdf2c51e Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sat, 5 Sep 2026 18:31:00 +0200 Subject: [PATCH] feat(e10): re-aim the Button variants at the paint the app actually uses, and migrate the first 15 sites Counting the un-migrated call sites showed the primitive and the app had drifted into two different looks: the majority of hand-rolled buttons hover to accent (outline) and fill with card (ghost), while Button hovered to fg and filled with surface. That split is the inconsistency this epic exists to remove, so the primitive follows the majority rather than the other way round -- 16 existing call sites shift slightly, ~90 hand-rolled ones stop being special. Adds the variants the call sites prove are missing (outline-accent, outline-danger, outline-warn, link), an icon-sm size for the p-1.5 icon family, and a size=none so a link owns no button padding; renames danger-outline to danger-soft so it cannot be confused with outline-danger (red at rest vs red on hover). Two new tests lock the link-size default and the rule that a variant never adds a second font-weight utility -- without tailwind-merge, two font-* utilities are decided by stylesheet order, not class order, so that is a real hazard. Migrates AdminUsers, AuditLog, Channels, TagManager, Playlists, PlaylistsRail and SettingsPanel (Button call sites 37 -> 52). Left alone deliberately: conditional-paint sites, video-overlay white buttons, drag handles, chip removers and card-click targets -- the families Button's own docstring excludes. 3x tsc, eslint, knip, prettier and 689 vitest green; visual UAT on Users, Access requests and Playlists at 1280x900, no console errors. --- frontend/src/components/AdminUsers.tsx | 76 ++++++++++--------- frontend/src/components/AuditLog.tsx | 10 ++- frontend/src/components/Channels.tsx | 18 ++--- frontend/src/components/Playlists.tsx | 35 ++++----- frontend/src/components/PlaylistsRail.tsx | 9 +-- frontend/src/components/SettingsPanel.tsx | 9 +-- frontend/src/components/TagManager.tsx | 9 ++- .../src/components/ui/Button.dom.test.tsx | 27 +++++++ frontend/src/components/ui/Button.tsx | 47 ++++++++++-- 9 files changed, 154 insertions(+), 86 deletions(-) diff --git a/frontend/src/components/AdminUsers.tsx b/frontend/src/components/AdminUsers.tsx index ab70e155..9d6b9b0b 100644 --- a/frontend/src/components/AdminUsers.tsx +++ b/frontend/src/components/AdminUsers.tsx @@ -26,6 +26,7 @@ import { import { notify } from "../lib/notifications"; import { invalidateUserAccess, isEmailOff } from "../lib/adminUsers"; import { ADMIN_USERS_TAB_KEY } from "../lib/adminUsersTab"; +import { Button } from "./ui/Button"; import Tooltip from "./ui/Tooltip"; import InlineNotice from "./ui/InlineNotice"; import { Section } from "./ui/form"; @@ -244,14 +245,16 @@ function UsersRoles({ me }: { me: Me }) { : t("users.roles.makeAdmin") } > - + - + icon={ + u.is_suspended ? ( + + ) : ( + + ) + } + /> - + icon={} + /> ); @@ -435,25 +438,28 @@ function PendingAccessRowView({ {!row.email_verified && ( - + icon={} + /> )} {row.can_activate && ( - + )} @@ -673,12 +679,14 @@ function AdminInvites() { placeholder={t("settings.invites.addPlaceholder")} className="flex-1 min-w-0 bg-surface border border-border rounded-xl px-3 py-2 text-sm focus:outline-hidden focus:ring-2 focus:ring-accent/40" /> - + {t("settings.invites.add")} + {decided.length > 0 && ( diff --git a/frontend/src/components/AuditLog.tsx b/frontend/src/components/AuditLog.tsx index 0631b678..e382a24a 100644 --- a/frontend/src/components/AuditLog.tsx +++ b/frontend/src/components/AuditLog.tsx @@ -6,6 +6,7 @@ import { Trash2 } from "lucide-react"; import { api } from "../lib/api"; import { notify } from "../lib/notifications"; import { LS } from "../lib/storage"; +import { Button } from "./ui/Button"; import { Section } from "./ui/form"; import { useConfirm } from "./ConfirmProvider"; import DataTable from "./ui/DataTable"; @@ -72,14 +73,15 @@ export default function AuditLog() { persistKey={LS.auditTable} controlsPosition="top" controlsLeading={ - + } emptyText={t("audit.empty")} /> diff --git a/frontend/src/components/Channels.tsx b/frontend/src/components/Channels.tsx index 2edc6817..1fff9b9c 100644 --- a/frontend/src/components/Channels.tsx +++ b/frontend/src/components/Channels.tsx @@ -10,6 +10,7 @@ import { notifyYouTubeActionError } from "../lib/youtubeErrors"; import { accountKey, LS } from "../lib/storage"; import { buildChannelColumns, applySyncFilter } from "./channelColumns"; import { notify } from "../lib/notifications"; +import { Button } from "./ui/Button"; import Tooltip from "./ui/Tooltip"; import DataTable from "./ui/DataTable"; import Tabs, { type TabDef } from "./ui/Tabs"; @@ -489,13 +490,15 @@ export default function Channels() { {/* The "TAGS" label IS the tags-editor trigger (pencil), replacing the separate "Manage tags" button; the tooltip explains what tags are + that it's clickable. */} - + {tagFilter.length > 0 && ( @@ -518,12 +521,9 @@ export default function Channels() { {highConfidence.length > 0 && ( - + )} )} diff --git a/frontend/src/components/Playlists.tsx b/frontend/src/components/Playlists.tsx index 2d82d550..2dc648a0 100644 --- a/frontend/src/components/Playlists.tsx +++ b/frontend/src/components/Playlists.tsx @@ -42,6 +42,7 @@ import { cycleSort, type SortState } from "../lib/columnSort"; import { formatRuntime } from "../lib/format"; import { applyView, sameOrder } from "../lib/playlistView"; const PlayerModal = lazy(() => import("./PlayerModal")); +import { Button } from "./ui/Button"; import UndoToolbar from "./ui/UndoToolbar"; import { PageToolbar } from "./ui/PageShell"; import { useChoice, useConfirm } from "./ConfirmProvider"; @@ -585,12 +586,9 @@ export default function Playlists() { {t("playlists.loadFailed")} - + ); } @@ -703,25 +701,26 @@ export default function Playlists() { )} {linked && detail.dirty && ( - + )} {editable && ( - + icon={} + /> )} @@ -755,12 +754,14 @@ export default function Playlists() { )} {canSaveOrder && ( - + {t("playlists.saveOrder")} + )} {/* ONE sentence that names the state. Three loose texts used to pile up here (reset link + save hint + "reordering is off"), and none of them answered the diff --git a/frontend/src/components/PlaylistsRail.tsx b/frontend/src/components/PlaylistsRail.tsx index a23aab7f..918ab859 100644 --- a/frontend/src/components/PlaylistsRail.tsx +++ b/frontend/src/components/PlaylistsRail.tsx @@ -10,6 +10,7 @@ import { notifyYouTubeActionError } from "../lib/youtubeErrors"; import { LS, readAccountMerged, writeAccount } from "../lib/storage"; import { type PanelLayout } from "../lib/panelLayout"; import FilterRail from "./FilterRail"; +import { Button } from "./ui/Button"; import { type PanelItem } from "./ui/PanelGroups"; import SortControl from "./ui/SortControl"; import { type SortOption } from "../lib/columnSort"; @@ -147,14 +148,10 @@ export default function PlaylistsRail({ title: t("playlists.manageGroup"), body: (
- + {playlists.length > 1 && ( ) : ( - + )}
@@ -676,7 +673,7 @@ export function DownloadAuth() {
{h.configured && ( +
diff --git a/frontend/src/components/ui/Button.dom.test.tsx b/frontend/src/components/ui/Button.dom.test.tsx index 6db53608..dc20d45a 100644 --- a/frontend/src/components/ui/Button.dom.test.tsx +++ b/frontend/src/components/ui/Button.dom.test.tsx @@ -71,6 +71,33 @@ describe("Button primitive (S5)", () => { expect(onClick).not.toHaveBeenCalled(); }); + it("a link gets no button padding, and an explicit size still wins", () => { + // The size default is variant-dependent (link → "none"). Locking it because the failure is + // silent: a link that quietly inherits px-3 py-2 reads as a button, and nothing errors. + const { rerender } = render(); + let btn = screen.getByRole("button", { name: "Show all" }); + expect(btn.className).toContain("text-accent"); + expect(btn.className).not.toContain("px-3"); + rerender( + , + ); + btn = screen.getByRole("button", { name: "Show all" }); + expect(btn.className).toContain("px-3"); + }); + + it("keeps one font-weight utility whatever the variant", () => { + // Two font-* utilities of equal specificity are decided by stylesheet order, not class order, + // so a variant must never add a second one (there is no tailwind-merge here). + for (const variant of ["primary", "link", "glass", "outline"] as const) { + const { unmount } = render(); + const cls = screen.getByRole("button", { name: "X" }).className; + expect(cls.split(/\s+/).filter((c) => c.startsWith("font-")).length).toBe(1); + unmount(); + } + }); + it("fullWidth adds the w-full utility", () => { render(); expect(screen.getByRole("button", { name: "Wide" }).className).toContain("w-full"); diff --git a/frontend/src/components/ui/Button.tsx b/frontend/src/components/ui/Button.tsx index 12b44960..31baf14b 100644 --- a/frontend/src/components/ui/Button.tsx +++ b/frontend/src/components/ui/Button.tsx @@ -17,25 +17,45 @@ type ButtonVariant = | "primary" // solid accent — the main affirmative action | "danger" // solid danger — destructive confirm | "outline" // bordered, quiet — secondary action (Full history, Subscribe, Cancel-with-border) + | "outline-accent" // bordered + accent-tinted — an affirmative secondary (Activate, Approve) + | "outline-danger" // neutral at rest, danger on hover — the row-level Delete icon + | "outline-warn" // bordered + warn-tinted — a reversible restriction that is currently ON | "ghost" // borderless, hover-fill — tertiary/dismiss (dialog Cancel, close) | "glass" // glass card — the Plex/overlay action buttons - | "danger-outline"; // bordered danger — a destructive action that shouldn't shout + | "danger-soft" // bordered AND danger-coloured at rest — a destructive action that shouldn't shout + | "link"; // inline text link — "Show all", "Learn more", a resend-mail prompt inside a sentence type ButtonSize = | "xs" // dense inline action | "sm" // the btnCls geometry (px-3 py-1.5) | "md" // default action button (px-3 py-2) | "lg" // prominent CTA - | "icon"; // square icon-only (w-9 h-9) + | "icon" // square icon-only (w-9 h-9) + | "icon-sm" // square icon-only, dense (w-8 h-8) — toolbars and table rows + | "none"; // no padding/radius of its own — the `link` variant's default const VARIANTS: Record = { // .btn-primary / .btn-danger own the fill + one hover-dim (index.css @layer components). primary: "btn-primary", danger: "btn-danger", - outline: "border border-border text-muted hover:text-fg hover:border-fg/40", - ghost: "text-muted hover:text-fg hover:bg-surface", + // E10 S1b: the neutral variants were RE-AIMED at the paint the app actually uses. Counting the + // un-migrated call sites showed the majority hovering to `accent` (outline) and filling with `card` + // (ghost), while this primitive hovered to `fg` and filled with `surface` — so the 37 migrated sites + // and the ~100 hand-rolled ones had drifted into two different looks. That split IS the + // inconsistency this epic exists to remove; the primitive follows the majority, not the reverse. + outline: "border border-border text-muted hover:text-accent hover:border-accent", + "outline-accent": "border border-accent/40 text-accent hover:bg-accent/10", + // Neutral until you reach for it. NOT `danger-soft`, which is already red at rest — the difference + // is whether the button should advertise the destruction before the pointer arrives. + "outline-danger": "border border-border text-muted hover:text-danger hover:border-danger/40", + "outline-warn": "border border-warn/40 text-warn hover:bg-warn/10", + ghost: "text-muted hover:text-fg hover:bg-card", glass: "glass-card glass-hover hover:text-accent", - "danger-outline": "border border-danger/60 text-danger hover:bg-danger/10 hover:border-danger", + "danger-soft": "border border-danger/60 text-danger hover:bg-danger/10 hover:border-danger", + // No font-weight here on purpose: BASE already sets font-medium, and a second font-* utility would + // be a coin-flip (both are single-class utilities of equal specificity, and without tailwind-merge + // the winner is decided by the generated stylesheet order, not by the class list order). + link: "text-accent hover:underline", }; const SIZES: Record = { @@ -44,6 +64,8 @@ const SIZES: Record = { md: "px-3 py-2 text-sm rounded-lg gap-1.5", lg: "px-4 py-2.5 text-base rounded-lg gap-2", icon: "w-9 h-9 rounded-lg justify-center", + "icon-sm": "w-8 h-8 rounded-lg justify-center", + none: "gap-1.5", }; const BASE = @@ -51,6 +73,7 @@ const BASE = export interface ButtonProps extends ButtonHTMLAttributes { variant?: ButtonVariant; + /** Defaults to "md", except for `variant="link"`, whose default is "none" (a link owns no padding). */ size?: ButtonSize; /** Leading icon (an lucide element). Replaced by a spinner while `loading`. */ icon?: ReactNode; @@ -67,7 +90,7 @@ export interface ButtonProps extends ButtonHTMLAttributes { export const Button = forwardRef(function Button( { variant = "primary", - size = "md", + size, icon, iconOnly = false, label, @@ -81,6 +104,10 @@ export const Button = forwardRef(function Button }, ref, ) { + // A link must not inherit the action-button padding/radius, so its size default differs. Resolved + // here rather than by asking every call site to pass `size="none"` alongside `variant="link"` — + // a pairing everyone would eventually forget, which is how a "link" grows a button's box. + const resolvedSize: ButtonSize = size ?? (variant === "link" ? "none" : "md"); const leading = loading ? : icon; return (