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

The pictures the forge took of an environment speak the home page's language on the pull page #9967

Merged opened this

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

Merged

The look under a pull's checks is now a block with an h2, not a card. The new test fails on main (the block assertion) and passes with the change.

shipped production is serving it deploy aa46b8b

opened this

The look under a pull's checks is now a block with an h2, not a card. The new test fails on main (the block assertion) and passes with the change.

  • Change: the Look section in src/web/pages/looks.ts is now <section class="settings-block" id="look"> with <h2>Look</h2>. settings-block is the class the Settings › Accounts page (#9948) already uses, so no new component class was needed. The "no picture yet" case uses the same block. The card and icon imports are gone from that file. The pictures, the main-versus-head pairing, the dark-mode disclosure and attached pictures are unchanged, and so are the image routes and caching headers. The block lost the eye icon the card header carried.
  • Test: test/looks-language.test.ts checks that the pull page shows the look under an h2 with its pictures and no card. It also checks that a private repository's picture URL returns 404 to a signed-out outsider.
  • Checks: tsc, scripts/design-lint.ts and scripts/gate.ts pass. The six look-related test files pass except three chromium capture tests in test/looks.test.ts, which fail the same way on main because the sandbox has no browser. I did not run the full suite; the forge runs it on the push.
  • Not done: I left the look's styling alone; the .looks layout is unchanged.

To try it, open any pull page whose preview has a picture, for example /alice/garden/pulls/1.

Closes #9959

commented

The look under a pull's checks is now a block with an h2, not a card. The new test fails on main (the block assertion) and passes with the change.

  • Change: the Look section in src/web/pages/looks.ts is now <section class="settings-block" id="look"> with <h2>Look</h2>. settings-block is the class the Settings › Accounts page (#9948) already uses, so no new component class was needed. The "no picture yet" case uses the same block. The card and icon imports are gone from that file. The pictures, the main-versus-head pairing, the dark-mode disclosure and attached pictures are unchanged, and so are the image routes and caching headers. The block lost the eye icon the card header carried.
  • Test: test/looks-language.test.ts checks that the pull page shows the look under an h2 with its pictures and no card. It also checks that a private repository's picture URL returns 404 to a signed-out outsider.
  • Checks: tsc, scripts/design-lint.ts and scripts/gate.ts pass. The six look-related test files pass except three chromium capture tests in test/looks.test.ts, which fail the same way on main because the sandbox has no browser. I did not run the full suite; the forge runs it on the push.
  • Not done: I left the look's styling alone; the .looks layout is unchanged.

To try it, open any pull page whose preview has a picture, for example /alice/garden/pulls/1.

commented

The check failed on a commit-message rule, not on code. The gate wants a "Try:" line in at least one commit message when a pull changes src/web/pages/looks.ts. I added an empty commit with that line and pushed it. I didn't re-run the tests or the gate.

The pull makes the look under a pull's checks a block under an h2 instead of a card (#9959). The new line says to open a pull's page on the preview and read the look block under the h2 below its checks.

Nothing in the code changed in this push. I'm relying on the forge's re-run of the check to confirm it passes.

Approved

The Look on the pull page is now a settings-block section under an h2 with no card class. Pictures, pairing and the image routes are unchanged, and the new test covers the block and the private 404. It uses an existing component class, one accent, and no JS. Two things I would do next: (1) update docs/APPS.md lines ~1272 and ~1294 and the comment names in test/looks.test.ts, which still say 'Look card'; (2) check the phone capture of a pull page, since the block lost its eye icon and the h2 is now the only anchor for the look, and consider giving the 'No picture yet' case a short link to why.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs