trevato/deal-clock

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

More
Forks1g fMapg aRunsg rEventsFlows

Every page is checked to fit a phone screen without sideways scrolling #316

Merged masoncrew opened this

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

ConversationFiles
Merged

The six pages each carry the viewport tag, and the CSS no longer has any wide fixed width or no-wrap component that can push a page sideways at 360px. `src/phone.test.ts` passes. `bun scripts/design-lint.ts`, `bun scripts/gate.ts` and `bun x tsc --noEmit` could not be run here. The two scripts aren't in this clone, and tsc only printed its help text because there is no tsconfig. I did not render t…

shipped production is serving it deploy d2eada7

masoncrew opened this

The six pages each carry the viewport tag, and the CSS no longer has any wide fixed width or no-wrap component that can push a page sideways at 360px. src/phone.test.ts passes. bun scripts/design-lint.ts, bun scripts/gate.ts and bun x tsc --noEmit could not be run here. The two scripts aren't in this clone, and tsc only printed its help text because there is no tsconfig. I did not render the pages in a browser at 360px, so the findings come from reading the CSS.

Findings

  • Every page fetched (/, a deal, /people, /reminders/today, /week, /activity) already has the viewport meta tag from the shared layout.
  • No rule fixes a width above 360px outside a scroller. The client timeline's fixed-width columns already collapse to one column below 40rem.
  • The clock's no-wrap parts (ticks, today label, markers) sit inside its horizontal scroller, so they can't widen the page.
  • The no-wrap labels outside the scroller could not wrap at 360px:

- .button, .you and .side stayed on one line when long, such as a long button label, a long "you" line or the side label beside an address. - .pill and .badge had no way to shrink or break a long word.

Fix

  • Inside the existing @media (max-width: 30rem) block in static/app.css, .button, .you and .side now wrap, and .pill and .badge are capped at 100% width and break long words.
  • It is CSS only, with no new class, so nothing is added to docs/design/areas/. Desktop layout and the contract are untouched.

Tests (src/phone.test.ts)

  • The six pages return 200 and carry the viewport tag. /activity is fetched as an admin because it is admin-only, and Review is at /reminders/today, since /review 404s.
  • No rule fixes a width above 360px unless it scrolls or wraps.
  • Every no-wrap component either lives in the clock's scroller or wraps again at phone width.
  • The timeline's columns collapse below 40rem.

Left alone

  • There are no <table> elements, so there was nothing to change there.
  • I added no JavaScript.

Closes #305

masoncrew commented

The six pages each carry the viewport tag, and the CSS no longer has any wide fixed width or no-wrap component that can push a page sideways at 360px. src/phone.test.ts passes. bun scripts/design-lint.ts, bun scripts/gate.ts and bun x tsc --noEmit could not be run here. The two scripts aren't in this clone, and tsc only printed its help text because there is no tsconfig. I did not render the pages in a browser at 360px, so the findings come from reading the CSS.

Findings

  • Every page fetched (/, a deal, /people, /reminders/today, /week, /activity) already has the viewport meta tag from the shared layout.
  • No rule fixes a width above 360px outside a scroller. The client timeline's fixed-width columns already collapse to one column below 40rem.
  • The clock's no-wrap parts (ticks, today label, markers) sit inside its horizontal scroller, so they can't widen the page.
  • The no-wrap labels outside the scroller could not wrap at 360px:

- .button, .you and .side stayed on one line when long, such as a long button label, a long "you" line or the side label beside an address. - .pill and .badge had no way to shrink or break a long word.

Fix

  • Inside the existing @media (max-width: 30rem) block in static/app.css, .button, .you and .side now wrap, and .pill and .badge are capped at 100% width and break long words.
  • It is CSS only, with no new class, so nothing is added to docs/design/areas/. Desktop layout and the contract are untouched.

Tests (src/phone.test.ts)

  • The six pages return 200 and carry the viewport tag. /activity is fetched as an admin because it is admin-only, and Review is at /reminders/today, since /review 404s.
  • No rule fixes a width above 360px unless it scrolls or wraps.
  • Every no-wrap component either lives in the clock's scroller or wraps again at phone width.
  • The timeline's columns collapse below 40rem.

Left alone

  • There are no <table> elements, so there was nothing to change there.
  • I added no JavaScript.
sentinelcrew Changes requested — before the latest push

One fix first: the .side reset does not work. .side { white-space: nowrap } is declared at static/app.css ~line 1055, after the @media (max-width: 30rem) block (line 993). Same specificity, later source order, so nowrap still wins at 360px and a long side label still overflows. Move the .side base rule above the media block, or put the reset in a media block after it. The CSS test misses this because it only checks that the selector appears in the phone block, not the cascade order; it should also assert the phone block comes after every base rule for those selectors (or the .side base rule precedes it).

Also:

  • The .pill/.badge fix has no test. Nothing asserts the phone block gives them max-width: 100% and overflow-wrap. Add one that fails without it.
  • Test hygiene: the new imports and serve() sit mid-file after existing tests. Hoist the imports to the top. The new rules regex duplicates the one in the first test; reuse it.
  • Box 1 asks for the check on the review page and a deal page; those are covered. Box 3 asks for the findings in the pull and for design-lint to pass. The builder says design-lint, gate and tsc were not run, and the clone has scripts/lint-design.ts rather than the scripts/design-lint.ts the task names. Run the design lint (bun scripts/lint-design.ts) before pushing. The preview was not rendered at 360px either.
masoncrew commented

The .side reset now takes effect at 360px, and src/phone.test.ts pins the cascade order and the pill/badge rule. bun test src/phone.test.ts src/design.test.ts passes and bun scripts/lint-design.ts exits 0. I did not run scripts/gate.ts (no such file in the clone) or bun x tsc --noEmit (there is no tsconfig, so it only prints help). I still haven't rendered the preview at 360px.

  • .side cascade: the .side { white-space: nowrap } base rule was declared after the @media (max-width: 30rem) block, so it overrode the reset. It now sits above the block in static/app.css.
  • Cascade test: a new test checks that no .you, .side, .button or .state nowrap rule appears after the phone block. I did not stash the CSS change to confirm it fails on the old order.
  • Pill/badge test: it asserts the phone block gives .pill, .badge both max-width: 100% and overflow-wrap: anywhere.
  • Test hygiene: all imports are hoisted to the top of the file, and one shared RULE regex replaces the duplicated ones.
  • Design lint: the task named scripts/design-lint.ts, but this clone has scripts/lint-design.ts, so I ran that.
sentinelcrew Approved

Approved. The .side cascade bug is fixed (base rule moved above the phone block) and pinned by a test. The pill/badge cap and break are asserted. The change is CSS plus tests only, with no new JS, no contract change and no dependency. The viewport test covers all six pages, using an admin for /activity. Non-blocking: the cascade test hardcodes the you|side|button|state selector list. I read the diff only, because this clone does not hold the head commit.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs