refactor(coach): retire the redundant settings-sheet close-X

The primitive `<SheetContent>` close-X is an absolutely positioned
`opacity-70 rounded-xs` button that clashes with the rest of the
sheet's in-header chrome. Hide it via `showCloseButton={false}` and
render a matching ghost-icon `<SheetClose>` inline in the header
so the close affordance shares the same baseline, ghost variant,
and 44 px tap floor as the coach-drawer header pattern. The
settings-sheet test mock picks up a `SheetClose` passthrough so the
existing skeleton + form assertions keep working.
This commit is contained in:
Marc-André Bombeck committed 2026-05-15 18:35:02 +02:00
1 parent beb2b40f7d
commit 48261b6797
2 files changed
+38 -7

No files matched your search

@@ -55,6 +55,11 @@ vi.mock("@/components/ui/sheet", () => ({
SheetDescription: ({ children }: { children: React.ReactNode }) => (
<p>{children}</p>
),
// v1.4.27 R3d MB1 — the settings sheet now renders an inline
// `<SheetClose>` in the header instead of the primitive's
// absolutely-positioned close-X. Mock it as a passthrough so the
// existing skeleton + form snapshot assertions keep working.
SheetClose: ({ children }: { children: React.ReactNode }) => <>{children}</>,
}));
import { CoachSettingsSheet } from "../coach-settings-sheet";
@@ -5,8 +5,11 @@ import { useMutation, useQueryClient } from "@tanstack/react-query";
import { Loader2 } from "lucide-react";
import { toast } from "sonner";
import { X } from "lucide-react";
import {
Sheet,
SheetClose,
SheetContent,
SheetDescription,
SheetHeader,
@@ -140,16 +143,39 @@ export function CoachSettingsSheet({
<Sheet open={open} onOpenChange={onOpenChange}>
<SheetContent
side="right"
// v1.4.27 R3d MB1 — the default `<SheetContent>` close-X is an
// absolutely positioned `opacity-70 rounded-xs` button that
// clashed with the in-header layout. Match the coach-drawer
// pattern instead: hide the primitive's close-X and render a
// matching ghost-icon `<SheetClose>` inline in the header so
// the close affordance sits on the same baseline as the rest
// of the sheet chrome and clears the 44 px tap target.
showCloseButton={false}
data-slot="coach-settings-sheet"
className="flex h-[100dvh] w-full flex-col gap-0 p-0 sm:max-w-[420px]"
>
<SheetHeader className="border-border/70 border-b p-4 pr-12">
<SheetTitle className="text-sm font-semibold">
{t("insights.coach.settingsTitle")}
</SheetTitle>
<SheetDescription className="text-muted-foreground text-xs">
{t("insights.coach.settingsDescription")}
</SheetDescription>
<SheetHeader className="border-border/70 flex-row items-start gap-2 border-b p-4">
<div className="min-w-0 flex-1">
<SheetTitle className="text-sm font-semibold">
{t("insights.coach.settingsTitle")}
</SheetTitle>
<SheetDescription className="text-muted-foreground text-xs">
{t("insights.coach.settingsDescription")}
</SheetDescription>
</div>
<SheetClose asChild>
<Button
type="button"
variant="ghost"
size="icon"
data-slot="coach-settings-sheet-close"
aria-label={t("common.close")}
title={t("common.close")}
className="text-muted-foreground hover:text-foreground -mt-1 shrink-0"
>
<X className="size-4" aria-hidden="true" />
</Button>
</SheetClose>
</SheetHeader>
{isLoading ? (