mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 21:10:59 +00:00
fix(settings): close the dead space above every row of the Appearance list
The Appearance hub paints its modules as one divide-y ledger inside a settings card, and the card dropped its padding without dropping its gap. The gap survived between the rows as unclickable space the first row does not have: 24 px on desktop, 16 px on a phone, measured. Rows read a step apart from the top of their own list. - SettingsCard grows a `flush` variant that drops padding AND gap, naming the shape `<InsightSectionCard flush>` already names on the Insights side - the geometry sweep gains a fourth claim: in a divided list card nothing sits between two rows but the divider, and the first row starts at the card's inner edge
This commit is contained in:
1 parent
01f94df60b
commit
ebf0c9d632
3 files changed
+137
-4
No files matched your search
@@ -21,6 +21,12 @@ import { settleBeforeMeasure } from "./utils/settle";
|
||||
* 3. **One reading edge.** Every card body starts at the card's inner edge.
|
||||
* The `pl-7` gutter put some bodies 28 px in and some at the edge, on the
|
||||
* same page, one card apart.
|
||||
* 4. **One row rhythm in a list card.** A card whose rows are a `divide-y`
|
||||
* ledger has nothing between two rows but the divider line, and its first
|
||||
* row starts at the card's inner top edge. The card contract carries a
|
||||
* `gap-4 md:gap-6` for a stack of blocks; dropping the padding without
|
||||
* dropping the gap leaves that gap standing between the rows as dead,
|
||||
* unclickable space the top of the list does not have.
|
||||
*
|
||||
* Runs in EN and DE, at desktop and phone widths. German runs 15-25% longer,
|
||||
* and an explainer that wraps in German where English fits is the same defect
|
||||
@@ -210,6 +216,63 @@ async function measureCards(page: import("@playwright/test").Page) {
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Row rhythm inside a `divide-y` list card, per card on the page.
|
||||
*
|
||||
* A list card is recognised by its own shape rather than by a class name:
|
||||
* three or more direct children, no card header among them, and a divider on
|
||||
* every seam between them. That is the ledger pattern wherever it is written,
|
||||
* and the three-row floor keeps a header-plus-one-bordered-block card — which
|
||||
* claim 2 already owns — out of this one.
|
||||
*
|
||||
* `topInset` is the space above the FIRST row, measured from the card's inner
|
||||
* top edge; `rowGaps` is the space between consecutive rows, over and above
|
||||
* the divider line itself. Both are zero in a correct ledger — which is the
|
||||
* whole point: the first row and every later row sit the same distance from
|
||||
* the line above them.
|
||||
*/
|
||||
async function measureListCards(page: import("@playwright/test").Page) {
|
||||
return page.evaluate(() => {
|
||||
const out: { slot: string; topInset: number; rowGaps: number[] }[] = [];
|
||||
for (const card of document.querySelectorAll('[data-slot="card"]')) {
|
||||
if (card.querySelector(":scope > header")) continue;
|
||||
const rows = Array.from(card.children) as HTMLElement[];
|
||||
if (rows.length < 3) continue;
|
||||
// Tailwind writes the divider on the top edge of every row but the
|
||||
// first, or on the bottom edge of every row but the last, depending on
|
||||
// the utility. Accept either — the pattern is the same ledger.
|
||||
const dividedFromTop = rows
|
||||
.slice(1)
|
||||
.every((row) => parseFloat(getComputedStyle(row).borderTopWidth) > 0);
|
||||
const dividedFromBottom = rows
|
||||
.slice(0, -1)
|
||||
.every(
|
||||
(row) => parseFloat(getComputedStyle(row).borderBottomWidth) > 0,
|
||||
);
|
||||
if (!dividedFromTop && !dividedFromBottom) continue;
|
||||
const cardStyle = getComputedStyle(card);
|
||||
const cardInnerTop =
|
||||
card.getBoundingClientRect().top +
|
||||
parseFloat(cardStyle.borderTopWidth) +
|
||||
parseFloat(cardStyle.paddingTop);
|
||||
out.push({
|
||||
slot: card.getAttribute("data-testid") ?? "card",
|
||||
topInset: rows[0].getBoundingClientRect().top - cardInnerTop,
|
||||
// The divider is drawn inside the row's border box, so it is already
|
||||
// inside the measured top edge — a correct ledger reports 0 here.
|
||||
rowGaps: rows
|
||||
.slice(1)
|
||||
.map(
|
||||
(row, index) =>
|
||||
row.getBoundingClientRect().top -
|
||||
rows[index].getBoundingClientRect().bottom,
|
||||
),
|
||||
});
|
||||
}
|
||||
return out;
|
||||
});
|
||||
}
|
||||
|
||||
for (const locale of ["en", "de"] as const) {
|
||||
test.describe(`one piece — ${locale}`, () => {
|
||||
test(`page headers land at one height across every route (${locale})`, async ({
|
||||
@@ -402,5 +465,58 @@ for (const locale of ["en", "de"] as const) {
|
||||
expect(offGap, "header→body gap is not the card's own gap").toEqual([]);
|
||||
expect(offEdge, "card body does not start at the card edge").toEqual([]);
|
||||
});
|
||||
|
||||
test(`list cards keep one row rhythm (${locale})`, async ({ page }) => {
|
||||
test.setTimeout(420_000);
|
||||
await page.context().addCookies([
|
||||
{
|
||||
name: "healthlog-locale",
|
||||
value: locale,
|
||||
url: process.env.E2E_BASE_URL ?? "http://localhost:3000",
|
||||
},
|
||||
]);
|
||||
|
||||
const offenders: string[] = [];
|
||||
let measured = 0;
|
||||
for (const route of ROUTES) {
|
||||
const response = await page.goto(route, {
|
||||
waitUntil: "domcontentloaded",
|
||||
});
|
||||
if (response && response.status() >= 400) continue;
|
||||
const heading = page
|
||||
.locator(
|
||||
'main h1:visible, main [role="heading"][aria-level="1"]:visible',
|
||||
)
|
||||
.first();
|
||||
if (!(await heading.isVisible().catch(() => false))) {
|
||||
await heading
|
||||
.waitFor({ state: "visible", timeout: 8_000 })
|
||||
.catch(() => {});
|
||||
}
|
||||
await settleBeforeMeasure(page, page.locator("main").first());
|
||||
for (const card of await measureListCards(page)) {
|
||||
measured += 1;
|
||||
const worstGap = Math.max(0, ...card.rowGaps);
|
||||
if (
|
||||
Math.abs(card.topInset) > TOLERANCE ||
|
||||
worstGap > TOLERANCE ||
|
||||
Math.abs(worstGap - Math.min(...card.rowGaps)) > TOLERANCE
|
||||
) {
|
||||
offenders.push(
|
||||
`${route} (${card.slot}): first row inset ${card.topInset.toFixed(1)}px, ` +
|
||||
`between rows ${[...new Set(card.rowGaps.map((g) => g.toFixed(1)))].join("/")}px`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
console.log(`[${locale}] list cards measured: ${measured}`);
|
||||
// The sweep has to actually find the ledgers it is about.
|
||||
expect(measured).toBeGreaterThan(0);
|
||||
expect(
|
||||
offenders,
|
||||
"a divided list card puts dead space above its rows that the first row does not have",
|
||||
).toEqual([]);
|
||||
});
|
||||
});
|
||||
}
|
||||
@@ -39,7 +39,7 @@ export function LayoutSection() {
|
||||
);
|
||||
|
||||
return (
|
||||
<SettingsCard className="divide-border divide-y overflow-hidden p-0 md:p-0">
|
||||
<SettingsCard flush className="divide-border divide-y">
|
||||
{visibleGroups.map((group) => (
|
||||
<Link
|
||||
key={group.id}
|
||||
|
||||
@@ -28,10 +28,22 @@ import { cn } from "@/lib/utils";
|
||||
*
|
||||
* `as="section"` keeps a card's semantic landmark element (a `<section>` with
|
||||
* an `aria-labelledby`) while painting the same shape, rhythm, and padding.
|
||||
*
|
||||
* `flush` is for the other body shape: an edge-to-edge `divide-y` ledger whose
|
||||
* rows carry their own inset. Such a card drops BOTH the padding and the gap.
|
||||
* Dropping only the padding is the trap — the gap survives as dead,
|
||||
* unclickable space between the rows that the first row does not have, so the
|
||||
* list reads a step off itself. `<InsightSectionCard flush>` names the same
|
||||
* shape on the Insights side.
|
||||
*/
|
||||
const SETTINGS_CARD_SHELL = "flex flex-col gap-4 p-4 md:p-6";
|
||||
// Both breakpoints are named explicitly: the `<Card>` primitive carries
|
||||
// `md:gap-6` / `md:py-6`, and a bare `gap-0` / `p-0` loses to a `md:` variant
|
||||
// from `md` up — which is exactly the width the dead space showed at.
|
||||
const SETTINGS_CARD_FLUSH_SHELL =
|
||||
"flex flex-col gap-0 overflow-hidden p-0 md:gap-0 md:p-0";
|
||||
|
||||
type SettingsCardOwnProps = { className?: string };
|
||||
type SettingsCardOwnProps = { className?: string; flush?: boolean };
|
||||
|
||||
type SettingsCardProps<E extends React.ElementType> = SettingsCardOwnProps & {
|
||||
as?: E;
|
||||
@@ -40,6 +52,7 @@ type SettingsCardProps<E extends React.ElementType> = SettingsCardOwnProps & {
|
||||
export function SettingsCard<E extends React.ElementType = typeof Card>({
|
||||
as,
|
||||
className,
|
||||
flush = false,
|
||||
...props
|
||||
}: SettingsCardProps<E>) {
|
||||
if (as) {
|
||||
@@ -50,7 +63,8 @@ export function SettingsCard<E extends React.ElementType = typeof Card>({
|
||||
// Mirror the ui Card shell so a semantic landmark card paints
|
||||
// identically to the default <Card>-backed one.
|
||||
className={cn(
|
||||
"bg-card text-card-foreground flex flex-col gap-4 rounded-xl border p-4 shadow-sm md:gap-6 md:p-6",
|
||||
"bg-card text-card-foreground flex flex-col rounded-xl border shadow-sm",
|
||||
flush ? SETTINGS_CARD_FLUSH_SHELL : "gap-4 p-4 md:gap-6 md:p-6",
|
||||
className,
|
||||
)}
|
||||
{...props}
|
||||
@@ -61,7 +75,10 @@ export function SettingsCard<E extends React.ElementType = typeof Card>({
|
||||
<Card
|
||||
// Reset the primitive's flex/gap layout and apply the shared
|
||||
// `p-4 md:p-6` padding; Settings bodies own their internal spacing.
|
||||
className={cn(SETTINGS_CARD_SHELL, className)}
|
||||
className={cn(
|
||||
flush ? SETTINGS_CARD_FLUSH_SHELL : SETTINGS_CARD_SHELL,
|
||||
className,
|
||||
)}
|
||||
{...(props as React.ComponentProps<typeof Card>)}
|
||||
/>
|
||||
);
|
||||
|
||||
Reference in new issue
Block a user