test(trip): add AE4 no-banner coverage + record deferred review findings

- trip-header.spec.js: new U4/R9/AE4 test — a trip with no cover_image and no
  entry images renders its text header with no .trip-header-banner strip.
- Makefile: demo-load/demo-reset now also (un)load the no-photos-demo fixture,
  guarded with `|| true` so runs whose user/ predates the fixture skip it
  instead of failing globalSetup.
- plan: post-review follow-up notes the applied fixes and records the two
  intentionally-deferred findings (macro-owned resolution per KTD2; inline
  toggle script per U4's Approach) so they are not re-flagged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RDS6t8wcpbwKvvrxykVQ5K
This commit is contained in:
2026-07-07 09:13:59 +02:00
co-authored by Claude Opus 4.8
parent 6a73be3e49
commit af07ef403c
3 changed files with 57 additions and 1 deletions
@@ -259,3 +259,35 @@ No lint/build step applies — the edited `css/style.css` and templates are serv
| U3 | List cards show one-liners where set and sharp retina covers with correct fallback; U3 specs pass. |
| U4 | Trip-page header shows one-liner, expandable description, and gated banner in HTD order; home view unchanged; U4 specs pass. |
| U5 | Header/banner styled; description expands; split stays above the fold on mobile. |
---
## Post-review follow-up (2026-07-07)
A structured code review of the finished diff produced fixes and two
intentionally-deferred findings.
**Applied**
- Cover picker restricted to images (`accept:` on the `cover_image`
`pagemediaselect` field) + macro resolves against `media.images`, so a
non-image selection (e.g. a `.gpx` from the trip page media) can no longer
route a non-image Medium into `cropResize`. Also hardens R11.
- Test quality: replaced a vacuous `toContainText` in the description-clamp
spec with real clamp/un-clamp assertions; added the missing **AE4** coverage
(trip with no cover + no entry images → text-only header, no banner) via a
`no-photos-demo` fixture; corrected an R11 over-claim in the trips-list spec
header comment.
**Intentionally deferred — explicit plan override (do not re-flag)**
- **Macro re-queries dailies/first-entry (reviewer: efficiency/maintainability).**
Deferred by design: **KTD2** puts cover resolution *inside* the shared macro
precisely so the list card and trip banner cannot drift. Moving resolution
out to callers reopens that drift; the extra `grav.pages.find()` is cached and
negligible.
- **Inline `<script>` for the description toggle should be bundled into
`js/src/main.js` (reviewer: convention).** Deferred by design: **U4's
Approach** explicitly specifies "a small inline `<script>` (alongside the
existing `initTripStats` script)." The inline placement is the plan's chosen
approach for a self-contained ~15-line toggle, not an oversight.