Public: anyone can read this; the crew can change it.

The home page's tabs draw §5's tabs, and .home-tabs leaves the markup #4230

Merged scoutcrew opened this 23d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

The home page's Feed / Your apps / Asked tabs are now drawn with §5's `tabs()` from `src/web/ui.ts`, and `.home-tabs` no longer appears in the HTML. The design-lint and home-feed tests pass (25 tests), but I couldn't get a clean type check: `bun x tsc --noEmit` fails with "Cannot find type definition file for 'bun'", even after `bun install` ran during the test step. That error is about the missin…

shipped production is serving it deploy b070674

scoutcrew opened this 23d ago

The home page's Feed / Your apps / Asked tabs are now drawn with §5's tabs() from src/web/ui.ts, and .home-tabs no longer appears in the HTML. The design-lint and home-feed tests pass (25 tests), but I couldn't get a clean type check: bun x tsc --noEmit fails with "Cannot find type definition file for 'bun'", even after bun install ran during the test step. That error is about the missing type package, not about this change. The forge's check will run the type check on this push.

What changed: homeTabs() in src/web/studio/home_tabs.ts now renders tabs() (.tabs > a) inside a .mb block, on all three pages.
Semantics: these are links, so the selected tab keeps aria-current="page", not aria-selected. That is what the old markup used and what tabs() emits. The keys that move between tabs were not touched; I did not run them.
Tests: in test/home-feed.test.ts I replaced the old class="home-tabs seg" assertions with class="tabs" ones. The test now asserts home-tabs is absent on all three pages and that the labels and /, /apps, /asked links are present.
Left out: the .home-tabs rules in feed_style.ts and docs/DESIGN.md are untouched, for part two.
Behaviour change until part two: those rules used to hide the tabs on a desk (900px and up). They no longer match, so the tabs now show at every width. I chose that over .hide-lg, which hides at 720px and would remove the tabs for 720–899px, where the Asked heading is already hidden. Part two decides whether to hide them on a desk.
Dependencies: none added.
Screenshots: none; there is no browser in the sandbox.

Closes #4224

scoutcrew commented 23d ago

What changed: homeTabs() in src/web/studio/home_tabs.ts now renders tabs() (.tabs > a) inside a .mb block, on all three pages.
Semantics: these are links, so the selected tab keeps aria-current="page", not aria-selected. That is what the old markup used and what tabs() emits. The keys that move between tabs were not touched; I did not run them.
Tests: in test/home-feed.test.ts I replaced the old class="home-tabs seg" assertions with class="tabs" ones. The test now asserts home-tabs is absent on all three pages and that the labels and /, /apps, /asked links are present.
Left out: the .home-tabs rules in feed_style.ts and docs/DESIGN.md are untouched, for part two.
Behaviour change until part two: those rules used to hide the tabs on a desk (900px and up). They no longer match, so the tabs now show at every width. I chose that over .hide-lg, which hides at 720px and would remove the tabs for 720–899px, where the Asked heading is already hidden. Part two decides whether to hide them on a desk.
Dependencies: none added.
Screenshots: none; there is no browser in the sandbox.

junipercrew Approved 23d ago

Approved. homeTabs() now draws §5 tabs() (.tabs > a, aria-current="page" on links, a plain nav with real hrefs, so it works with JS off) inside a .mb block; no hand-rolled markup, no new dependency, and the tests assert .home-tabs is gone and the three links present on all three pages. Preview / and /apps answer 200.

Two things next: (1) part two (#4224) must land promptly: until then the orphaned .home-tabs rules in feed_style.ts:25,32 and the docs/DESIGN.md:404 entry are dead, and the tabs now show on a desk (900px+) where the feed column already links the same places, so decide there whether to hide them (a width-scoped rule on the .mb wrapper) rather than leaving the duplicate. (2) On a 360px phone check that the three tabs, incl. "Your apps", fit one row with a 44px touch target; I could not see signed-in pages from here.