Skip to content
Merged
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
14 changes: 11 additions & 3 deletions src/components/fairway/cards-insight/InsightCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,12 @@ const InsightCardImpl = forwardRef<HTMLDivElement, InsightCardProps>(
// action cluster drops BELOW the title row (no horizontal overflow / title
// crush at 360px) and sits inline on the right from `sm:` up.
isCompact ? 'gap-x-3 gap-y-2 p-4 flex-wrap items-center' : 'gap-4',
!isCompact && (isHero ? 'p-8' : 'p-6'),
// Hero padding steps down on phone (#957) — at a fixed `p-8` + the h2
// title + body-lg narrative below, this ONE card consumed nearly the
// full first viewport on a phone-width dashboard, reading as the
// "vibe-coded, one long card" tell (Mobile Doctrine rule 11) instead of
// the first section in a composed page. `sm:` and up is unchanged.
!isCompact && (isHero ? 'p-6 sm:p-8' : 'p-6'),
// interactivity — visual lift only on the container; the focus ring + the
// actual keyboard/click affordance live on the overlay <button> (so the
// ring follows the focusable element, and the card has-focus-within still
Expand Down Expand Up @@ -445,8 +450,11 @@ const InsightCardImpl = forwardRef<HTMLDivElement, InsightCardProps>(
id={titleId}
className={cn(
'min-w-0 text-text-primary',
// Hero title steps down a size on phone (#957) — see the
// shell-padding comment above; same "one card ate the first
// viewport" fix, applied to the loudest element in it.
isHero
? 'font-fw-display text-h2 font-medium tracking-[-0.005em]'
? 'font-fw-display text-h3 sm:text-h2 font-medium tracking-[-0.005em]'
: isCompact
? 'font-fw-sans text-body font-semibold leading-snug'
: 'font-fw-sans text-h3 font-semibold',
Expand All @@ -469,7 +477,7 @@ const InsightCardImpl = forwardRef<HTMLDivElement, InsightCardProps>(
<div
className={cn(
'font-fw-sans text-text-secondary',
isHero ? 'text-body-lg leading-relaxed' : 'text-body',
isHero ? 'text-body sm:text-body-lg leading-relaxed' : 'text-body',
isCompact && 'line-clamp-1',
)}
>
Expand Down
36 changes: 33 additions & 3 deletions src/components/fairway/controls/Toolbar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,47 @@ describe('Toolbar — search stops competing with filters for growth at lg+', ()
expect(searchWrapper!.className).toContain('lg:w-72');
});

it('the filters wrapper stays the only flex-1 grower on the row (absorbs desktop leftover space)', () => {
it('the filters wrapper stays the only grower on the row from `sm` up (absorbs desktop leftover space)', () => {
const { container } = render(
<Toolbar
search={<input aria-label="search" />}
filters={<button>Severity</button>}
viewToggle={<button>Feed</button>}
/>,
);
const filtersWrapper = container.querySelector('button')?.parentElement;
const filtersWrapper = container.querySelector('[class*="overflow-x-auto"]');
expect(filtersWrapper).not.toBeNull();
expect(filtersWrapper!.className).toContain('flex-1');
expect(filtersWrapper!.className).toContain('sm:grow');
expect(filtersWrapper!.className).toContain('sm:basis-0');
expect(filtersWrapper!.className).toContain('min-w-0');
});

it('below `sm` the row stacks as full-width lines in SOURCE order — no `order` utilities (tab order must match visual order)', () => {
const { container } = render(
<Toolbar
search={<input aria-label="search" />}
filters={<button>Severity</button>}
viewToggle={<button data-testid="toggle">Feed</button>}
/>,
);
// Phone composition (#957 + #959 review): line 1 = search (basis-full),
// line 2 = the filter scroll strip (basis-full), line 3 = view toggle +
// actions (ml-auto). An earlier draft reflowed lines with `order-*`,
// which sent keyboard focus visually backwards on phones — DOM order,
// tab order, and visual order must stay identical, so `order` utilities
// are banned from this row.
const searchWrapper = container.querySelector('input')!.parentElement!;
const strip = container.querySelector('[class*="overflow-x-auto"]')!;
expect(searchWrapper.className).toContain('basis-full');
expect(strip.className).toContain('basis-full');
for (const el of [searchWrapper, strip]) {
expect(el.className).not.toMatch(/(?:^|\s)order-/);
}
// The view toggle is mounted exactly ONCE, in the trailing cluster —
// never duplicated into a phone-only slot.
expect(container.querySelectorAll('[data-testid="toggle"]')).toHaveLength(1);
const trailing = container.querySelector('[data-testid="toggle"]')!.parentElement!;
expect(trailing.className).toContain('ml-auto');
expect(trailing.className).not.toMatch(/(?:^|\s)order-/);
});
});
76 changes: 54 additions & 22 deletions src/components/fairway/controls/Toolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,19 @@ import { PopoverPanel } from '../overlays/PopoverPanel';
* Warm cream-glass recipe (self-contained, built from locked --fw-glass-* tokens)
* Used ONLY when the row is sticky-stuck or hosting the bulk bar (the two
* §4.3-approved chrome slots). Matte at rest never touches this.
*
* `--fw-glass-bg-strong` (88% tint), not the 74% `--fw-glass-bg` — a stuck
* toolbar sits directly over live scrolling body copy (unlike a card-level
* glass surface with more breathing room beneath it), and at 74% + the
* `saturate(190%)` refraction the scrolled text was reading straight through
* the pills, unreadable (#957). `--fw-glass-bg-strong` is the same token the
* modal/command-palette tier already uses for exactly this "must stay legible
* over content" case. No `blur()` is added — that's still the mobile
* scrolling-chrome perf rule (Mobile Doctrine's performance floor); only the
* opacity of the existing tint changes, which is effectively free.
* ─────────────────────────────────────────────────────────────────────────── */
const STUCK_GLASS_STYLE: CSSProperties = {
backgroundColor: 'var(--fw-glass-bg)',
backgroundColor: 'var(--fw-glass-bg-strong)',
backdropFilter: 'saturate(var(--fw-glass-saturate))',
WebkitBackdropFilter: 'saturate(var(--fw-glass-saturate))',
borderColor: 'var(--fw-glass-border)',
Expand Down Expand Up @@ -243,40 +253,62 @@ const ToolbarRoot = forwardRef<HTMLDivElement, ToolbarProps>(function Toolbar(
transition={{ duration: reduceMotion ? 0 : 0.16 }}
className="flex min-h-[44px] flex-wrap items-center gap-3 px-3 py-2"
>
{/* search — grows to absorb slack so the row reads as one quiet
field+controls. Bug #949 #8: below `lg` this still competes
equally (flex-1) with the filters cluster for space, which is
fine on mobile/tablet (the filters strip is a horizontal
scroller there anyway). From `lg` up, pin it to a fixed
comfortable width instead of growing — a search input never
NEEDS more than that, and letting it keep pulling flex-grow
share from `filters` was exactly what squeezed a 3-pill
filter set (Severity/Status/Category) down far enough that
the trailing "Status" pill clipped under the view-toggle
segmented control even at wide desktop widths (>=1280px),
where there was actually plenty of total room. */}
{/* Below `sm` the row re-composes into stacked full-width lines
(#957 — at phone width, search, three filter pills, a
segmented view toggle AND the action buttons cannot share
flex-wrap lines without something clipping mid-word at the
viewport edge, which is exactly what shipped):
line 1 · search, full width
line 2 · the filter-pill scroll strip, full width
line 3 · view toggle + actions, pinned right
Deliberately NO `order` utilities: source order already reads
top-to-bottom/left-to-right at every width, so DOM order,
tab order, and visual order stay identical (a #959-review
finding — an earlier draft reordered lines with `order-*`,
which sent keyboard focus visually backwards on phones).
From `sm` up the basis overrides reset and the original
single-line composition is byte-identical.

search — grows to absorb slack so the row reads as one quiet
field+controls. From `sm` up: Bug #949 #8 — below `lg` it
competes equally (flex-1) with the filters cluster for space
(fine on tablet, the filters strip scrolls there too). From
`lg` up, pin it to a fixed comfortable width instead of
growing — a search input never NEEDS more than that, and
letting it keep pulling flex-grow share from `filters` was
exactly what squeezed a 3-pill filter set (Severity/Status/
Category) down far enough that the trailing "Status" pill
clipped under the view-toggle segmented control even at wide
desktop widths (>=1280px), where there was actually plenty of
total room. */}
{search ? (
<div className="min-w-[180px] flex-1 sm:max-w-sm lg:w-72 lg:flex-none">{search}</div>
<div className="min-w-0 basis-full sm:min-w-[180px] sm:flex-1 sm:max-w-sm lg:w-72 lg:flex-none">
{search}
</div>
) : null}

{/* filters — horizontally scrollable so a long set never breaks
the row. From `lg` up it's now the ONLY flex-1 item on this
line (search stopped competing for the same growth share
above), so it claims all the room left over from search +
the trailing view-toggle/primary-action cluster — the 3-pill
set fits without ever needing its scroll fallback at desktop
widths. */}
the row. Below `sm` it is its own full-width line, so the
scroller gets the whole viewport to work with. From `lg` up
it's the ONLY flex-1 item on its line (search stopped
competing for the same growth share above), so it claims all
the room left over from search + the trailing cluster — the
3-pill set fits without ever needing its scroll fallback at
desktop widths. */}
{filters ? (
<div
ref={filtersFadeRef}
style={filtersFadeStyle}
className="flex min-w-0 flex-1 items-center gap-2 overflow-x-auto [scrollbar-width:none] [&::-webkit-scrollbar]:hidden"
className="flex min-w-0 grow-0 basis-full items-center gap-2 overflow-x-auto sm:grow sm:basis-0 [scrollbar-width:none] [&::-webkit-scrollbar]:hidden"
>
{filters}
</div>
) : null}

{/* trailing cluster — view toggle + primary action, always far-right */}
{/* trailing cluster — view toggle + primary action. Far-right on
its shared desktop line; the right-pinned last line on phone
(search and filters each took a full line above, so ml-auto
starts this cluster on a fresh line and pushes it right). */}
{(viewToggle || primaryAction) && (
<div className="ml-auto flex flex-shrink-0 items-center gap-2">
{viewToggle}
Expand Down
12 changes: 11 additions & 1 deletion src/components/fairway/overlays/ModalShell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,17 @@ const ModalBody = React.forwardRef<
ref={ref}
data-slot="modal-shell-body"
className={cn(
'flex-1 overflow-y-auto px-6 py-2 font-fw-sans text-body text-text-secondary',
// `flex-auto min-h-0`, NOT `flex-1`: the panel column is `h-fit`
// (height: fit-content), and iOS Safari resolves flex-1's percentage
// basis (0%) against that as a literal 0 — the body rendered ~0px tall,
// its content clipped to a sliver, on iPhone while desktop engines
// sized it from content (owner report 2026-07-18: the aspect drill-down
// showed title + a clipped chip strip + Done and nothing else; Sheet
// bodies — plain auto-height, no h-fit — never collapsed). `flex-auto`
// sizes from content in every engine, then shrinks (min-h-0) into the
// panel's max-h cap where overflow-y-auto takes over — identical layout
// where it already worked, unbroken on iOS.
'min-h-0 flex-auto overflow-y-auto px-6 py-2 font-fw-sans text-body text-text-secondary',
// breathing room when there is no header/footer
'first:pt-6 last:pb-6',
className,
Expand Down
9 changes: 8 additions & 1 deletion src/components/fairway/overlays/Sheet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,14 @@ const SheetBody = React.forwardRef<
ref={ref}
data-slot="sheet-body"
className={cn(
'flex-1 overflow-y-auto px-6 py-2 font-fw-sans text-body text-text-secondary',
// `flex-auto min-h-0`, NOT `flex-1`: defensive parity with
// ModalShell.Body, where iOS Safari resolved flex-1's percentage basis
// (0%) against the panel's intrinsic-keyword height as 0 and collapsed
// the body (full story there). Sheets size with plain auto height and
// haven't shown the collapse, but content-based basis + min-h-0 gives
// the same layout without ever depending on how an engine resolves a
// percentage basis against an indefinite height.
'min-h-0 flex-auto overflow-y-auto px-6 py-2 font-fw-sans text-body text-text-secondary',
// When the body is the last child it owns the bottom edge → keep its
// content clear of the iOS home indicator.
'first:pt-6 last:pb-[max(1.5rem,env(safe-area-inset-bottom))]',
Expand Down
40 changes: 28 additions & 12 deletions src/components/fairway/pages/coachhelm/FairwayAspectDrillDown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import * as React from 'react';
import Link from 'next/link';
import { useRouter } from 'next/navigation';
import { cn } from '@/lib/utils';
import { ModalShell } from '@/components/fairway/overlays/ModalShell';
import { Sheet } from '@/components/fairway/overlays/Sheet';
import { Button } from '@/components/fairway/controls/button';
import { Badge } from '@/components/fairway/controls/badge';
import { Segmented } from '@/components/fairway/controls/segmented';
Expand Down Expand Up @@ -125,10 +125,11 @@ export function FairwayAspectDrillDown({
}: FairwayAspectDrillDownProps) {
const router = useRouter();

// Retain the last non-null aspect so the ModalShell stays mounted through its
// exit tween after the parent clears `aspect` on close — AnimatePresence can
// only animate out while still mounted (mirrors FocusAreaModal). `aspectProp`
// drives it (flicker-free open); `lastAspect` only covers the closing frame.
// Retain the last non-null aspect so the Sheet stays mounted through its
// exit tween after the parent clears `aspect` on close — vaul can only
// animate out while still mounted (mirrors FocusAreaModal's pattern).
// `aspectProp` drives it (flicker-free open); `lastAspect` only covers the
// closing frame.
const [lastAspect, setLastAspect] = React.useState<AspectDrillDownTarget | null>(aspectProp);
React.useEffect(() => {
if (aspectProp) setLastAspect(aspectProp);
Expand Down Expand Up @@ -390,14 +391,29 @@ export function FairwayAspectDrillDown({
const drillById = new Map((plan?.drills ?? []).map((d) => [d.id, d]));

return (
<ModalShell
// A Sheet, not a centered ModalShell (Mobile Doctrine rule 4 — every
// input/create flow under `md` is a bottom sheet): this panel holds FOUR
// stacked sections (Who/Goal/Plan/Ship), which read as an unreadably cramped
// centered dialog on a phone. `side="right" mobileSide="bottom"` is the
// Sheet API's designed pairing for exactly this panel shape: a docked
// right column on desktop (a plain `side="bottom"` here stretched the
// sheet edge-to-edge across wide viewports — #959-review finding), the
// native bottom sheet under `md`. `peek={false}` like every other
// bottom-sheet consumer in the app (MoreNavSheet, broadcast/new-message,
// announcements): the vaul snap-point path has no production consumer and
// its translate math assumes a fixed-height panel our content-sized
// sheets don't have. The sheet opens to content height, capped with the
// body scrolling.
<Sheet
open={open}
onOpenChange={onOpenChange}
size="full"
side="right"
mobileSide="bottom"
peek={false}
title={aspect.long}
description={`${aspect.kind === 'strength' ? 'Team strength' : 'Team weakness'} · ${aspect.rating}/100`}
>
<ModalShell.Body>
<Sheet.Body>
<div className="flex flex-col gap-7">
{/* ── Header chips ─────────────────────────────────────────────── */}
<div className="flex flex-wrap items-center gap-2">
Expand Down Expand Up @@ -695,14 +711,14 @@ export function FairwayAspectDrillDown({
</div>
</FormSection>
</div>
</ModalShell.Body>
</Sheet.Body>

<ModalShell.Footer>
<Sheet.Footer>
<Button variant="ghost" onClick={() => onOpenChange(false)}>
Done
</Button>
</ModalShell.Footer>
</ModalShell>
</Sheet.Footer>
</Sheet>
);
}

Expand Down
Loading
Loading