Repository navigation
web UI: style press/events/newsletter content screens - #60
Conversation
Restyles the three surviving content screens (FAQ/Staff cut in #50/#51, not touched) with #43's shared component classes -- .page-title, .label-rule, .card, .empty-state, .badge -- sharing one list/card pattern instead of bare <ul><li>. Events' featured-image box uses object-contain + a wider box (banners are ~1024x432, not square like press logos) so a fixed square object-cover crop doesn't slice the banner. New e2e/content-screens.spec.ts covers rendering, graceful image degradation, empty states, and the shared +error.svelte path for all three screens.
…solvable "default" image host
Reviewer-verified: GET /uapp/get_press returns image URLs with a literal host of `default`
(e.g. https://default/sites/default/files/press/images.png) for all releases -- present, so
`{#if image}` passes, but unresolvable, so PR #60's fixed h-16 box now renders a visible
broken-image glyph on every card (pre-#60 it collapsed to 0 height).
Fix, per review: normalize at the source in shared/src/content.ts. New sanitizeImageUrl()
blanks any image URL without a plausible hostname (must contain a dot, or be localhost);
applied in fetchPressReleases and mapEvent, covering both press and events for web and
mobile in one place. Web's press/events +page.ts load functions apply the same sanitizer a
second time on the /api/* response, since Playwright's page.route (used throughout this
suite's e2e mocks, per vertical-slice.spec.ts's own comment) can only intercept
browser-originated requests, not the SvelteKit server process's own outbound fetch to
umassdining.com -- without this second pass the new e2e case couldn't exercise the fix at all.
Also takes the reviewer's nit: removed the three `main li.card` count-0 assertions in the
empty-state specs (press/events/newsletter) -- vacuous, since they'd pass equally whether
the card markup is correct or entirely broken, given there's no data to render either way.
`.empty-state` visibility is the assertion that actually matters.
|
Fixed the blocking finding: unresolvable Normalized at the source in Red/green at both levels — details, exact failure output, and full lane results are in the updated PR body. Summary: Also took the nit on the vacuous Commit: c2c7d7f. Not merging, left for review. |
Closes #41
Scope change acknowledged
#41's original body listed five screens (press/events/faq/staff/newsletter). Per the issue's 2026-08-19 comment, FAQ and Staff Directory are cut from the product entirely (#50/#51, already deleted from the codebase before this branch started). This PR styles only press, events, newsletter. Repo-wide grep for remnants before starting, zero hits:
Acceptance criteria (as descoped)
.page-title,.label-rule,.card,.empty-state,.badge), sharing one list/card pattern instead of independent bare<ul><li>markup on each screen.Staff directory photos display well if present, degrade gracefully if absent— void, staff directory is cut. Generalized the same principle to the two screens that do carry remote images: press release images and event featured images. Both wrapped in{#if}(absent → no<img>at all, no broken-image icon) and, for present images,object-contain(notobject-cover) so nothing gets cropped. Verified against live upstream data during the check below that UMass Dining's event images are ~1024×432 banners, not square thumbnails — a fixed squareobject-coverbox would have sliced most of the banner off, so events get a widerh-16 w-28box than press'sh-16 w-16.pnpm --filter web checkclean (0 errors, 0 warnings, 396 files).web/, so the CI lanes it can affect areweb-checkande2e(shared/mobile/supabase-* jobs are untouched and unaffected). Both run clean locally, see below; I don't have a way to trigger the GitHub Actions run itself from here, so the PR checks are the live confirmation.Anonymous-first / no auth gates
Zero session/auth code in any of the three
+page.ts/+page.sveltefiles, unchanged from before this PR — these are public content screens fetched via/api/{press,events,newsletter}, noauth.uid()or session check anywhere in the diff.What changed
web/src/routes/press/+page.svelteweb/src/routes/events/+page.svelteweb/src/routes/newsletter/+page.svelteweb/e2e/content-screens.spec.ts(new)Not touched:
web/src/app.css,web/src/routes/+layout.svelte, any other route, any other e2e spec.Red/green TDD evidence
Wrote
content-screens.spec.tsfirst, against the unstyled baseline pages. 6 of 9 tests are new UI behavior (styling classes that didn't exist yet) and went red naturally:The 3 error-path tests exercise pre-existing behavior (the
throw error(...)in each+page.ts, and the shared+error.sveltefrom #43), so per red-green TDD they needed a one-line mutation to prove the test actually catches a regression, not just a false positive. For each of press/events/newsletter, temporarily replacedif (!res.ok) throw error(...)withif (!res.ok) return { X: [] }(silently swallow the failure instead of erroring), ran just that test, confirmed red, then reverted:Reverted all three
+page.tsfiles immediately after each probe (git statusconfirmed clean before implementing the styling).Implemented the three
+page.sveltefiles, re-ran the full new spec: 9/9 green. Then the full suite (26 tests across all specs, including the sibling-agent-ownedshell.spec.ts,rank.spec.ts,filters.spec.ts,favorites.spec.ts,halls-menu.spec.ts,vertical-slice.spec.ts) onPORT=4304: 26/26 green, confirming no regressions to shell nav, error page, or other screens.Verification against live data
Ran the dev server (
PORT=4304) and curled/press,/events,/newsletterdirectly — SvelteKit's SSR load hit the realumassdining.comupstream (not mocked for this check) and rendered real press releases, events, and newsletter issues through the new.card/.page-title/.empty-statemarkup. Confirmed the event-image aspect-ratio fix against a real banner (Summer Hours Banner_0.jpg, downloaded and measured at 1024×432 with PIL) —object-containwas the right call overobject-coverfor that aspect ratio.Lanes run (original)
PORT=4304 npx playwright test(all 26 specs) — passpnpm --filter web check— clean, 0 errors/warningspnpm --filter web build— succeeds (pre-existingadapter-auto"could not detect platform" notice only, unrelated to this diff)shared/mobile/supabase-*CI jobs — no files under those trees changed, so they're unaffected by this diff.Not merging
Per dispatch instructions, this PR is left open for review, not merged.
Update (2026-08-19): fixed the one blocking review finding
Defect (reviewer-verified against live data):
GET https://www.umassdining.com/uapp/get_pressreturns image URLs with a literal host ofdefault(e.g.https://default/sites/default/files/press/images.png) for all 10 live releases — present, so{#if release.image}inpress/+page.svelte:22passes, but unresolvable, so this PR's fixedh-16box (which replaced the old collapse-to-0-height behavior) rendered a visible broken-image glyph on every card.h-16was the load-bearing utility that made the pre-existing bug visible.GET /uapp/get_beacons_events'sfeatured_imagefield carries the same risk and shares the same guard pattern inevents/+page.svelte.Fix: normalized at the source, per the review's preferred direction. New
sanitizeImageUrl()inshared/src/content.tsblanks any image URL without a plausible hostname (must contain a dot, or belocalhost); wired intofetchPressReleasesandmapEvent, covering press and events for both web and mobile in one place — mobile callsfetchPressReleases/fetchEventsdirectly with no proxy in between, so it needed no further change. Web's/pressand/events+page.tsload functions apply the same sanitizer a second time on the/api/*JSON response — not redundant paranoia, but necessary for the new e2e case to actually exercise the fix: this suite's own established pattern (seevertical-slice.spec.ts's comment) is that Playwright'spage.routeonly intercepts browser-originated requests, never the SvelteKit server process's own outbound fetch toumassdining.com, so mocking**/api/press**bypassesfetchPressReleasesentirely. Without the second pass, a red→green e2e test for this fix would have been structurally impossible to write within this codebase's existing mocking boundary.Red/green evidence:
Unit,
shared/src/content.test.ts(written first, run red against unfixedcontent.ts):After adding
sanitizeImageUrland wiring it intofetchPressReleases/mapEvent:pnpm --filter @udine/shared test→ 49/49 pass (was 47/49 before the fix, i.e. the 2 new tests, no other regressions).E2E,
web/e2e/content-screens.spec.ts(written first, run red against unfixed+page.ts):After sanitizing in
press/+page.tsandevents/+page.ts:PORT=4304 npx playwright test(full suite) → 37/37 pass.Reviewer nit taken: the three
no releases/no events/no issuesempty-state specs also assertedmain li.cardcount 0 — vacuous, since it'd pass equally whether the card markup is correct or entirely broken (there's no data to render either way once the empty-state check already confirms the right panel showed). Removed those three assertions;.empty-statevisibility is the assertion that actually matters.Lanes run for this fix:
pnpm --filter @udine/shared test— 49/49 passPORT=4304 npx playwright test(full suite, now 37 specs after web UX: Today/macros view #49/web UI: style filters and favorites screens #57/Remove accidentally committed agent scratch, gitignore .claude/ and .scratch/ #58/web UI: style ranking page (comparisons, Favorite Foods, Dining Hall Ranking) #59/web UI: style friends, pings, and notifications screens #61 landed on main) — 37/37 passpnpm --filter web check— clean, 0 errors/0 warnings, 396 filespnpm --filter web build— succeeds (same pre-existingadapter-autonotice, unrelated)Files changed for this fix:
shared/src/content.ts,shared/src/content.test.ts,web/src/routes/press/+page.ts,web/src/routes/events/+page.ts,web/e2e/content-screens.spec.ts. No+page.sveltechanges needed — the guards were already correct, they just needed non-empty-but-invalid data to never reach them.Merged
origin/maininto this branch first (#49, #57, #58, #59, #61 had landed since) — no conflicts, disjoint files. Still not merging this PR, left open for review per dispatch instructions.