Skip to content
Closed
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
54 changes: 54 additions & 0 deletions src/app/baseball/(dashboard)/_components/hub-sub-nav.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// =============================================================================
// src/app/baseball/(dashboard)/_components/hub-sub-nav.test.tsx
//
// #905 — pins the fix for "Operations"/"Postgame Review" tabs clipping past
// the viewport edge at 320/390px. `shrink-0` gave every tab the flex item's
// default `min-width: auto` floor (its full `whitespace-nowrap` label width),
// so even a hub capped at ≤3 tabs (Ruling 2) could exceed the viewport once
// icon + padding + a longer label were summed — and because
// `getBoundingClientRect` reflects LAYOUT position, not ancestor `overflow`
// clipping, the strip's own `overflow-x-auto` didn't hide the defect from a
// geometry-based clip check. This test locks in that tabs are shrinkable
// (`min-w-0`, not `shrink-0`) with a truncating label — same fix class as the
// FairwayBottomNav min-w-0 fix (#899).
// =============================================================================

import { describe, it, expect, vi } from 'vitest';
import { render, screen } from '@testing-library/react';
import { HubSubNav } from './hub-sub-nav';
import type { HubSubNavTab } from './hub-sub-nav';
import { IconUsers } from '@/components/icons';

vi.mock('next/navigation', () => ({
usePathname: vi.fn(() => '/baseball/dashboard/roster'),
}));

const TABS: HubSubNavTab[] = [
{ id: 'roster', label: 'Roster', href: '/baseball/dashboard/roster', icon: IconUsers },
{ id: 'calendar', label: 'Calendar', href: '/baseball/dashboard/calendar', icon: IconUsers },
{ id: 'operations', label: 'Operations', href: '/baseball/dashboard/operations', icon: IconUsers },
];

describe('HubSubNav — #905 shrinkable tabs', () => {
it('does not pin tabs to shrink-0 (the min-width:auto floor that clipped past the viewport)', () => {
const { container } = render(<HubSubNav tabs={TABS} ariaLabel="Team sections" />);
const items = container.querySelectorAll('li');
expect(items.length).toBeGreaterThan(0);
for (const el of Array.from(items)) {
expect((el as HTMLElement).className).not.toContain('shrink-0');
expect((el as HTMLElement).className).toContain('min-w-0');
}
});

it('truncates each tab label so it can shrink to fit instead of overflowing', () => {
render(<HubSubNav tabs={TABS} ariaLabel="Team sections" />);
const label = screen.getByText('Operations');
expect(label.className).toContain('truncate');
expect(label.className).toContain('min-w-0');
});

it('still renders every tab as a real link', () => {
render(<HubSubNav tabs={TABS} ariaLabel="Team sections" />);
expect(screen.getByRole('link', { name: /Operations/ })).toBeInTheDocument();
});
});
39 changes: 36 additions & 3 deletions src/app/baseball/(dashboard)/_components/hub-sub-nav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,23 @@ export function HubSubNav({ tabs, ariaLabel, className }: HubSubNavProps) {
const isActive = t.id === resolvedId;
const Icon = t.icon;
return (
<li key={t.id} className="relative shrink-0">
// min-w-0 (not shrink-0): #905 — at 320/390px a hub capped at
// ≤3 tabs (Ruling 2) can still exceed the viewport once icon +
// padding + a longer label ("Postgame Review", "Operations")
// are summed (e.g. Stats & Performance's 3 tabs measure to
// ~424px unshrunk at a 390px viewport). `shrink-0` gave every
// tab the flex item's default `min-width: auto` floor (its
// full unbreakable `whitespace-nowrap` label width), so the
// LAST tab's own bounding rect — not just the scrollable
// strip's content — bled past the viewport edge even though
// `overflow-x-auto` visually clipped it (getBoundingClientRect
// reflects layout position, not ancestor clipping). `min-w-0`
// restores the default `flex-shrink: 1` floor to zero so the
// label's `truncate` (below) can actually engage — tabs now
// shrink-to-fit inside the viewport first, falling back to the
// strip's horizontal scroll only once even truncated tabs
// don't fit (mirrors the FairwayBottomNav min-w-0 fix, #899).
<li key={t.id} className="relative min-w-0">
<Link
href={t.href}
ref={(node) => {
Expand All @@ -268,7 +284,18 @@ export function HubSubNav({ tabs, ariaLabel, className }: HubSubNavProps) {
onKeyDown={onKeyDown}
data-active={isActive ? '' : undefined}
className={cn(
'group relative inline-flex select-none items-center gap-2 whitespace-nowrap',
// `m-0` (#927): globals.css's blanket `li a` "inline link
// touch target" rule (meant for prose body text) matches
// this `<a>` too — a plain anchor inside an `<li>` — and,
// with no margin utility here to out-specificity it,
// applied its `margin: -0.375rem -0.125rem` unchallenged:
// +4px width and a 2px left shift on every tab, on top of
// whatever this tab's own content needed. `m-0` (a class
// selector) beats the rule's `li a` (two type selectors)
// on specificity and neutralizes it; the global rule is
// now also scoped to exclude `nav`-shaped anchors (this
// strip is a `<nav>`) so this can't silently recur here.
'group relative inline-flex select-none items-center gap-2 whitespace-nowrap m-0',
'rounded-t-lg px-3.5 pb-3 pt-3 text-sm font-medium min-h-[44px]',
'transition-colors duration-150 ease-out',
'focus:outline-none focus-visible:ring-2 focus-visible:ring-grade-plus/50 focus-visible:ring-offset-1 focus-visible:ring-offset-[color:var(--paper)]',
Expand All @@ -289,7 +316,13 @@ export function HubSubNav({ tabs, ariaLabel, className }: HubSubNavProps) {
)}
/>
)}
<span className="relative">{t.label}</span>
{/* min-w-0 + truncate: the tab's actual shrink mechanism —
the label is the flex item whose default min-content
floor (full nowrap text width) must be zeroed for the
`<li>`'s min-w-0 above to have anywhere to give the
space back to. Icon stays full-size (fixed 16px SVG,
never the thing that should disappear first). */}
<span className="relative min-w-0 truncate">{t.label}</span>

{/* The gliding active underline (layoutId; honors reduced-motion).
Kit cinematic settle curve — a glide, never a bouncy spring
Expand Down
25 changes: 23 additions & 2 deletions src/app/globals.css
Original file line number Diff line number Diff line change
Expand Up @@ -2089,9 +2089,30 @@
}
}

/* Inline link touch targets on mobile */
/* Inline link touch targets on mobile.
*
* ROOT CAUSE of #905/#927 (fixed 2026-07-17): the bare `li a` clause below
* matches ANY `<a>` inside ANY `<li>` app-wide, including structural
* flex/grid-divided nav rows like FairwayBottomNav and HubSubNav — not
* just inline links inside rendered prose. Every anchor those components
* render is a plain `<a>`/`<Link>` inside an `<li>` with NO margin utility
* of its own, so this rule's `margin: -0.375rem -0.125rem` (-6px/-2px)
* applied unchallenged (no competing Tailwind class = no specificity
* contest to win): a flex column computed to an honest 1/5-of-320px share
* had its rendered `<a>` bleed 2px past each edge (+4px total width, -2px
* left shift) purely from this rule — nothing to do with the row's own
* flex/justify-content classes. For the FIRST column that shift crosses
* the viewport's left edge, which is exactly the "Home" [left -2, right
* 66] failure e2e/mobile-viewports.spec.ts caught. `:not(...)` scopes the
* touch-target trick back to actual inline body-text links (its original
* intent) and leaves nav/tab/toolbar anchors — which already own precise
* touch-target sizing via their own component classes — alone.
*/
@media (max-width: 1023px) {
.prose a, p a, li a, span a {
.prose a:not(nav a, [role="navigation"] a, [role="tablist"] a, [role="toolbar"] a),
p a:not(nav a, [role="navigation"] a, [role="tablist"] a, [role="toolbar"] a),
li a:not(nav a, [role="navigation"] a, [role="tablist"] a, [role="toolbar"] a),
span a:not(nav a, [role="navigation"] a, [role="tablist"] a, [role="toolbar"] a) {
display: inline-block;
padding: 0.375rem 0.125rem;
margin: -0.375rem -0.125rem;
Expand Down
54 changes: 54 additions & 0 deletions src/components/fairway/app-shell/FairwayBottomNav.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// =============================================================================
// src/components/fairway/app-shell/FairwayBottomNav.test.tsx
//
// #905 — pins the fix for the "Home" tab clipping past the left viewport edge
// at 320/390px. #899 already removed the `min-width: auto` floor (`min-w-0`
// on every column), but the row's OWN `justify-around` remained: per the CSS
// Box Alignment spec, `justify-content: space-around` falls back to `center`
// whenever the line's free space goes negative, and centering an overflowing
// row shifts its start point negative — regardless of how small the overflow
// is. This test locks in that `justify-around` is gone (so any residual
// sub-pixel overflow degrades to an ordinary right-edge overflow on the LAST
// column, never a negative left-shift on the FIRST) while every column keeps
// its `min-w-0` floor-removal.
// =============================================================================

import { describe, it, expect } from 'vitest';
import { render, screen } from '@testing-library/react';
import { FairwayBottomNav } from './FairwayBottomNav';
import type { NavItem } from './types';
import { IconHome, IconUsers, IconChartBar, IconMessage } from '@/components/icons';

const ITEMS: NavItem[] = [
{ label: 'Home', href: '/baseball/dashboard/command-center', icon: IconHome },
{ label: 'Team', href: '/baseball/dashboard/roster', icon: IconUsers },
{ label: 'Stats', href: '/baseball/dashboard/stats-center', icon: IconChartBar },
{ label: 'Messages', href: '/baseball/dashboard/messages', icon: IconMessage },
];

describe('FairwayBottomNav — #905 negative-shift fix', () => {
it('never applies justify-around/justify-center to the row (fallback-to-center-on-overflow hazard)', () => {
const { container } = render(
<FairwayBottomNav items={ITEMS} pathname="/baseball/dashboard/command-center" onMoreOpen={() => {}} />,
);
const list = container.querySelector('ul')!;
expect(list.className).not.toMatch(/justify-around|justify-center/);
});

it('keeps min-w-0 on every destination column (the #899 floor-removal fix)', () => {
const { container } = render(
<FairwayBottomNav items={ITEMS} pathname="/baseball/dashboard/command-center" onMoreOpen={() => {}} />,
);
const columns = container.querySelectorAll('ul > li');
// 4 destinations + the More column.
expect(columns).toHaveLength(5);
for (const column of Array.from(columns)) {
expect((column as HTMLElement).className).toContain('min-w-0');
}
});

it('renders the first destination ("Home") as a real, unclipped tab', () => {
render(<FairwayBottomNav items={ITEMS} pathname="/baseball/dashboard/command-center" onMoreOpen={() => {}} />);
expect(screen.getByRole('link', { name: 'Home' })).toBeInTheDocument();
});
});
47 changes: 37 additions & 10 deletions src/components/fairway/app-shell/FairwayBottomNav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,24 @@ export const FairwayBottomNav = memo(function FairwayBottomNav({
className,
)}
>
<ul className="flex items-stretch justify-around">
{/* #905: no `justify-around`. Every column below is `flex: 1 1 0%`
(min-w-0 `flex-1`), so flex-grow already consumes 100% of the row's
width — `justify-content` only matters when there's leftover OR
negative free space, and with 5 honest `flex-1` columns dividing
an exact 320/390/430px row there never legitimately is any.
CORRECTION (#927): the first pass here theorized `justify-around`
(space-around) was falling back to `center` on overflow and
shifting the row's start negative — plausible-sounding, but
removing it left the measured "Home" [left -2, right 66] geometry
byte-for-byte unchanged in CI, which means it was never the actual
mechanism. The real cause was a global `li a` CSS rule (see
globals.css's "Inline link touch targets" block) unconditionally
margining every `<a>`-in-`<li>` app-wide, including these tabs —
see the `m-0` comment on each `<Link>` below for the full
writeup. Left here (harmless, and arguably the more predictable
default) rather than reverted, now that it's known NOT to be the
fix. */}
<ul className="flex items-stretch">
{items.map((item) => {
const active =
item.active ??
Expand All @@ -147,15 +164,25 @@ export const FairwayBottomNav = memo(function FairwayBottomNav({
// `flex: 1 1 0%` on the parent `<li>`) — without it, a long
// label (e.g. "Development", "Messages", or a mode's
// exposureNoun) can force this column past its 1/5 share of
// a 320/390px bar, overflowing the row by a few px. Tailwind's
// `justify-around` (space-around) falls back to `center` per
// the CSS Box Alignment spec whenever the line's free space
// is negative, and centering an overflowing row shifts its
// start point negative — the exact -2px left overhang on the
// first ("Home") tab this fixes. `min-w-0` here (mirroring
// the `min-w-0` already on the More button below) lets the
// label's own `truncate` class actually engage instead.
'group relative flex min-h-[56px] min-w-0 flex-col items-center justify-center gap-0.5 px-1 py-1.5',
// a 320/390px bar, overflowing the row by a few px.
//
// `m-0` (#927 real fix): globals.css's `li a` "inline link
// touch target" rule (meant for prose body text) matches
// this `<a>` too — it's a plain anchor inside an `<li>` —
// and applies `margin: -0.375rem -0.125rem` with NO
// Tailwind margin class here to out-specificity it (no
// rule = no contest to win). That silently added 4px width
// and shifted every tab 2px left of its true flex-computed
// position; on the FIRST column that pushed the box past
// the viewport's left edge — the exact "Home" [left -2,
// right 66] failure, and why the earlier justify-around /
// min-w-0 pass here (which never touched margin) left the
// measured geometry byte-for-byte unchanged. `m-0` beats
// `li a` on specificity (class > two type selectors) and
// neutralizes the leak at the component level; the global
// rule itself is now also scoped to exclude nav/tablist/
// toolbar anchors so this class of bug can't recur here.
'group relative flex min-h-[56px] min-w-0 m-0 flex-col items-center justify-center gap-0.5 px-1 py-1.5',
'outline-none transition-colors [transition-duration:var(--fw-dur-fast)] motion-reduce:transition-none',
'focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-border-focus',
active ? 'text-accent-700' : 'text-text-tertiary hover:text-text-secondary',
Expand Down
Loading