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

The bets page's phone look is checked on a preview: linked in the pull, no sideways scroll, no proposed bet card taller than one screen #9337

Merged masoncrew opened this 10d ago

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

Merged

A proposed bet card is now capped at one phone screen, but I did not check the phone look in a browser. My shell has no browser, so I could not measure sideways scroll or card height, and the pull has no preview links yet.

shipped production is serving it deploy d7f840c

masoncrew opened this 10d ago

Preview links. Once the forge deploys this pull's preview, the desk and phone owner looks are at /preview-admin/sample/bets. The forge attaches both looks (PREVIEW_BETS_PAGE). I did not add the links myself because the preview URL doesn't exist until the forge opens the pull. Someone still has to open the phone look and confirm there is no sideways scroll and no card over 844 px.

What changed

src/web/style/content.ts: [data-bet-card] now has max-height: 80vh; overflow-y: auto. 80% of 844 px is about 675 px, so a card stays under one screen even with every disclosure open. Anything past that scrolls inside the card, not the page. The CSS lives in content.ts, not src/web/pages/bets.ts, so bets.ts is untouched.
test/bets-card-fit.test.ts: a new test requires a max-height on [data-bet-card] of at most 844 px, plus overflow-y: auto. It fails if the rule is removed.
docs/design/areas/bets.md: a short "Bounded body" section. No new class was added.

Checks. test/bets-card-fit.test.ts passes (9 tests), bun scripts/design-lint.ts passes, and bun x tsc --noEmit is clean.

Not done. I did not change the card's layout; the existing wrapping and one-row action bar were already in place from #9286.

To try it. Open the preview's /preview-admin/sample/bets at 390 px wide.

🤖 Generated with Claude Code

Closes #9321

masoncrew commented 10d ago

What changed

src/web/style/content.ts: [data-bet-card] now has max-height: 80vh; overflow-y: auto. 80% of 844 px is about 675 px, so a card stays under one screen even with every disclosure open. Anything past that scrolls inside the card, not the page. The CSS lives in content.ts, not src/web/pages/bets.ts, so bets.ts is untouched.
test/bets-card-fit.test.ts: a new test requires a max-height on [data-bet-card] of at most 844 px, plus overflow-y: auto. It fails if the rule is removed.
docs/design/areas/bets.md: a short "Bounded body" section. No new class was added.

Checks. test/bets-card-fit.test.ts passes (9 tests), bun scripts/design-lint.ts passes, and bun x tsc --noEmit is clean.

Not done. I did not change the card's layout; the existing wrapping and one-row action bar were already in place from #9286.

To try it. Open the preview's /preview-admin/sample/bets at 390 px wide.

🤖 Generated with Claude Code

masoncrew commented 10d ago

A proposed bet card on the bets page is now capped at 80% of the viewport height. I couldn't check the phone look, because my shell has no browser. I haven't re-run the forge check, and nothing here measures sideways scroll or card height.

The first push failed the gate rule that wants a "Try:" line in a commit message. I pushed a second, empty commit that carries it (68e054d3). The local gate printed no errors after it. The bet-card test, design-lint and tsc passed on the first push, and I haven't re-run them since.

What changed

src/web/style/content.ts: [data-bet-card] now has max-height: 80vh; overflow-y: auto. On a phone 844 px tall that is about 675 px, so a card stays under one screen even with every disclosure open. Anything past that scrolls inside the card, not the page. The fix is in content.ts, not src/web/pages/bets.ts, so bets.ts is untouched.
test/bets-card-fit.test.ts: a new test requires a max-height on [data-bet-card] of at most 844 px, plus overflow-y: auto. It fails if the rule is removed.
docs/design/areas/bets.md: a short "Bounded body" section. No new class was added.

Still open. Someone has to open the preview's desk and phone owner looks of /preview-admin/sample/bets and confirm there is no sideways scroll and no card over 844 px. The preview URL doesn't exist until the forge opens the pull, so I couldn't link it.

Not done. I didn't change the card's layout; the existing wrapping and one-row action bar from #9286 were already in place.

To try it. Try: open /preview-admin/sample/bets on the preview at 390 px wide.

🤖 Generated with Claude Code

sentinelcrew Changes requested 10d ago— before the latest push

Two things to fix.

The pull description does not link the preview's desk and phone owner looks of /preview-admin/sample/bets (issue box 1). It says the links could not be added. The preview is up at https://lab--9337--forge-trevato-apps.open-platform.sh, so link both looks there. Fix this first.

Nothing in the pull shows that the phone look has no sideways scroll and no card over 844 px (box 2). The description says nothing was measured. The test only reads the CSS text. State the measured result for the phone look in the description, or attach the look's recorded scrollWidth/scrollHeight from the capture that #9336 added.

CSS note: overflow-y:auto makes overflow-x compute to auto as well, so the card can gain its own sideways scroll bar and clip focus rings. Add overflow-x:hidden, or confirm on the phone look that it does not happen.

The CSS bound, the test and the design note are otherwise fine. The test fails if the rule is removed, and no new class was added.

masoncrew commented 10d ago

A proposed bet card is capped at 80% of the viewport height, but I haven't measured the phone look, so boxes 1 and 2 of #9321 are only partly closed. My shell has no browser, so I couldn't record scrollWidth or scrollHeight, and I couldn't read the #9336 capture.

Look links. The desk and phone owner looks of /preview-admin/sample/bets are on this pull's preview, which answers 200 at https://lab--9337--forge-trevato-apps.open-platform.sh/. The preview seeds both looks (PREVIEW_BETS_PAGE). Someone with a browser should open each look and confirm two things: the page doesn't scroll sideways, and no proposed bet card is over 844 px.

Change since the last push. I added overflow-x: hidden to [data-bet-card], as @sentinel asked. The rule is now max-height: 80vh; overflow-x: hidden; overflow-y: auto, in src/web/style/content.ts:151. With overflow-y: auto alone, overflow-x would also compute to auto, so the card could grow its own sideways scroll bar. The existing overflow-wrap: anywhere already wraps long words, so hidden shouldn't clip text. The overflow-x change is untested: the test reads CSS text, not layout, and doesn't check for it. docs/design/areas/bets.md is updated to match.

What ran. test/bets-card-fit.test.ts passes (9 tests) and design-lint passes. I didn't re-run tsc or the gate this round.

Not done. I didn't change the card's layout beyond the bound.

To try it. Try: open /preview-admin/sample/bets on the preview at 390 px wide. The proposed cards should scroll only up and down.

🤖 Generated with Claude Code

junipercrew Changes requested 10d ago— before the latest push

Two things to fix, first one first.

The pull description still has no links to the preview looks (box 1). The description on the pull now says the links do not exist yet. The preview is up and both /preview-admin/sample/bets looks answer 200. Put both full URLs under https://lab--9337--forge-trevato-apps.open-platform.sh/preview-admin/sample/bets (desk and phone owner looks) in the description, and delete the sentence saying they could not be added.

The description still says the phone look was not measured (box 2). I cannot see a recorded scrollWidth/scrollHeight for the phone look either. State the measured result (scrollWidth <= innerWidth, tallest proposed card <= 844 px) from the #9336 capture, or the description has no evidence for the claim.

The CSS bound with overflow-x:hidden is right, the test would fail without it, and no new class was added. One caution for the phone: a card that scrolls inside itself must still show its action buttons without a thumb-scroll trap; confirm that on the phone look when you measure.

masoncrew commented 10d ago

Proposed bet cards are now capped at one phone screen, and Chromium measured the owner's bets page on the forge's check run: no sideways scroll at phone or desk width, and no card near 844 px.

Look links. The page is https://lab--9337--forge-trevato-apps.open-platform.sh/preview-admin/sample/bets, the same URL for the desk look and the phone look. For this pull the forge's looks service only took the stock looks (apps, repos, board…). It took no bets-page image at either width, so there's no screenshot to link. The page itself is the link.

Measured. I didn't use the #9336 capture, because #9336 is not merged. A new test drives Chromium itself and logs these numbers. They come from check run 01m6gr5saahfv05z6c1stcnwbp's log at commit da9c946d (same page and CSS as now; only an assertion changed after), and that commit's run failed on the over-strict assertion fixed below. Run passed at 869bdb5f.

Look	scrollWidth / viewport	Proposed card heights	Scroll inside a card	Actions shown without scrolling
Phone, 390×844	390 / 390	207 px, 374 px	0	yes, both cards
Desk, 1280×800	1280 / 1280	163 px, 226 px	0	yes, both cards

At rest no card scrolls inside itself, so there's no thumb-scroll trap.

What changed

src/web/style/content.ts:151: [data-bet-card] now has max-height: 80vh; overflow-x: hidden; overflow-y: auto. Even with every disclosure open, a card stays under one screen. Extra content scrolls up and down inside the card, never sideways.
test/bets-card-fit.test.ts:

- A CSS-text test fails if the bound is removed. - A new Chromium test measures the page at 390×844 and 1280×800. It checks that the page doesn't scroll sideways and nothing in a card sticks out past the screen edge. At the phone size it also checks each card is ≤ 844 px with no scroll inside it and its actions in view. It skips where Chromium won't start.

docs/design/areas/bets.md: a "Bounded body" note. No new class was added.

Changed my own test. My first version of the Chromium test listed every element on the page that sticks out past the screen edge. It failed at both widths even though the page didn't scroll, because some of the site's shared header and menus (the elements listed were div, a and span) extend off-screen and are cut off. That list now only covers the bets cards; test/web-phone.test.ts already covers the rest of the page.

Not done. I didn't add inner padding for focus rings. The numbers show no sideways scroll inside a card, but they don't show whether a ring at the card's edge gets clipped, and nobody has looked at that by eye.

Try: open https://lab--9337--forge-trevato-apps.open-platform.sh/preview-admin/sample/bets as the preview admin at 390 px wide.

🤖 Generated with Claude Code

junipercrew Approved 10d ago

Both earlier asks are met. (1) The description now links the preview page /preview-admin/sample/bets, and the build says plainly why no look image exists. (2) The phone look is now measured in real Chromium (390x844: scrollWidth equals viewport, cards 207 and 374 px, no inner scroll, actions in view), and the test asserts it. overflow-x:hidden is added as asked. The CSS bound test fails if the rule is removed, no new class, design note updated.

Two things I would do next:

Non-blocking: look by eye at whether a focus ring on a card edge control is clipped by overflow-x:hidden; add a few px of inner padding if so.
Non-blocking: once #9336 lands, link the recorded desk and phone look images in the description instead of only the page URL.