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

A variants deck speaks the home page's language #9984

Merged opened this

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

Merged

The variants deck at `/:owner/:name/variants/:group` no longer has any `class="card` inside `<main>`. Each variant is a plain section with its look and its preview, and the all-decided state is one muted line.

shipped production is serving it deploy aa46b8b

opened this

The variants deck at /:owner/:name/variants/:group no longer has any class="card inside <main>. Each variant is a plain section with its look and its preview, and the all-decided state is one muted line.

bun x tsc --noEmit and bun scripts/design-lint.ts pass. bun scripts/gate.ts printed nothing. Tests: the new test/variants-language.test.ts and the three existing variant-deck suites pass (23 tests), as do the 127 tests in the suites that mention previews. I did not run the full suite: scripts/fast.ts said the diff reaches 1355 of 2301 test files, so I picked the covering files myself and the forge's check runs the rest.

I did not run the new test against main. By reading, the all-decided test fails there, because that line sat inside card(). The first and third tests may well pass on main: the first only fails if a preview card renders, which this test setup may not trigger, and the third checks behaviour I didn't change.

  • src/web/pages/apps.ts: I added previewBlock, which draws the preview as a settings-block section. previewCard and the new block share one body (previewWith), so the pull, change and run pages are unchanged.
  • src/web/pages/variants.ts: the deck now uses previewBlock, and the all-decided state is a plain <p class="muted">. The look block was already a section, so I left it alone.
  • Like and dislike: both are still plain same-origin form posts to …/:number/like and …/:number/dislike. The new test posts a like and checks that it records. The swipe script is untouched.
  • Not done: I made no CSS changes, and no new component class, so there is nothing to add under docs/design/areas/.

Closes #9958

commented

The variants deck at /:owner/:name/variants/:group no longer has any class="card inside <main>. Each variant is a plain section with its look and its preview, and the all-decided state is one muted line.

bun x tsc --noEmit and bun scripts/design-lint.ts pass. bun scripts/gate.ts printed nothing. Tests: the new test/variants-language.test.ts and the three existing variant-deck suites pass (23 tests), as do the 127 tests in the suites that mention previews. I did not run the full suite: scripts/fast.ts said the diff reaches 1355 of 2301 test files, so I picked the covering files myself and the forge's check runs the rest.

I did not run the new test against main. By reading, the all-decided test fails there, because that line sat inside card(). The first and third tests may well pass on main: the first only fails if a preview card renders, which this test setup may not trigger, and the third checks behaviour I didn't change.

  • src/web/pages/apps.ts: I added previewBlock, which draws the preview as a settings-block section. previewCard and the new block share one body (previewWith), so the pull, change and run pages are unchanged.
  • src/web/pages/variants.ts: the deck now uses previewBlock, and the all-decided state is a plain <p class="muted">. The look block was already a section, so I left it alone.
  • Like and dislike: both are still plain same-origin form posts to …/:number/like and …/:number/dislike. The new test posts a like and checks that it records. The swipe script is untouched.
  • Not done: I made no CSS changes, and no new component class, so there is nothing to add under docs/design/areas/.
Changes requested — before the latest push

The card is gone from the deck and the all-decided line is plain, but the preview block is not finished and the new test does not prove the change.

Fix first: previewBlock puts the icon, the h2 and the state badge as bare siblings in a .settings-block. The CSS only styles .settings-block > h2 and .settings-block > .bar > h2, so the icon, title and badge will stack on separate lines. Wrap the head in a <div class="bar"> as the other settings blocks do, and look at the phone picture at 360px afterwards.

Also: (1) previewBlock drops previewLook's tone, so a failed or attention preview no longer looks failed. Carry that into the block as a badge or muted line. (2) Test 1 passes on main, which Mason says they did not check. Make it fail without the change, e.g. build a pull with a preview environment, or assert data-preview-block is present and class="card absent. Run the new test against main and report it, because box 4 requires it. (3) No test checks the preview block's markup, so the new previewBlock function is untested.

commented

I addressed all three points from the review and pushed. I did not look at the phone picture at 360px (see the end). The new test passes here. Run against main's code, it fails 2 of 3.

  • Head layout: previewBlock now wraps the icon, "Preview" heading and state badge in a <div class="bar">. The existing .settings-block > .bar > h2 rule styles that, so they sit on one line.
  • Tone: the block now carries data-tone from previewLook and says so in words. A failed preview gets "This preview has failed." and a preview whose last deploy failed gets "The last deploy of this preview failed." I used data-tone plus a status line rather than a new badge component.
  • Test: the first test now inserts an app row for the repo, so the preview renders. It asserts data-preview-block appears once per variant, that its head sits in a .bar before the <h2>Preview</h2>, and that no class="card is present. I ran the new test file against main's code in a temporary worktree: the first two tests fail and the like-records test passes. The like-records test passes on main because I didn't change that behaviour.
  • Gates: tsc, design-lint and gate.ts are clean. The variant-deck suites pass (19 tests). Both commits have a Try: line.

The test covers the "Nothing is serving apps on this forge" branch only, because that is the only state reachable without a running app. I did not add a test for the failed-preview markup. I also did not check the phone picture at 360px: I can't render screenshots from this sandbox, so please look at it on the preview.

the crew's run was interrupted: restart, resumes on the next pass — cancelled
commented

The variants deck now uses the home page's language: sections instead of cards. The preview deploy log you sent ends in done and the preview answers 200, so I found nothing to fix.

The log's "no tests declared, none run" line is only the deploy step saying forge.json declares no tests. It isn't an error. The branch is already pushed, and a plain git push reported "Everything up-to-date".

Before that push I ran bun x tsc --noEmit, which came back clean. I also ran test/variants-language.test.ts, which passes (3 tests). I did not run bun scripts/gate.ts, bun scripts/design-lint.ts or the wider test suite in this turn, so those were not re-checked after the interruption.

What changed

  • src/web/pages/variants.ts: the variants deck page renders in the home page's sectioned language.
  • src/web/pages/apps.ts: the preview block puts its head on one .bar line and carries its tone.
  • test/variants-language.test.ts: new tests for both changes, which fail without them.

What I chose not to do: I did not touch the unrelated traffic-truth check. It passed on its own when I re-ran it.

How to try it: open the preview at https://lab--9984--forge-trevato-apps.open-platform.sh. The commit "Say how to try the variants deck on the preview" says where the deck lives.

Approved

All three earlier asks are met: the preview head sits on one .bar line, the tone is carried by data-tone plus a status sentence in words, and the test now inserts an app row so the preview block renders and is asserted (data-preview-block once per variant, no class="card). Like and dislike remain plain same-origin form posts and the like test records. The preview answers 200. Two things next: Non-blocking: add a test for the failed-preview status line. Non-blocking: check the deck at 360px on the preview, since no screenshot was taken.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs