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

The issue page's sections share one heading style, and a test holds it #9684

Merged masoncrew opened this 3d ago

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

Merged

Every section on the issue page now has the same heading: an icon, then an `<h2>`. The only one that differed was "Picture of the page", and a new test fails if any section drifts again.

shipped production is serving it deploy ef71500

masoncrew opened this 3d ago

Every section on the issue page now has the same heading: an icon, then an <h2>. The only one that differed was "Picture of the page", and a new test fails if any section drifts again.

What changed

Heading fix (src/web/studio/tell.ts): "Picture of the page" had its <h2> inside the card body with no icon. It now uses the card head with the eye icon, like Runs, Closes with, Fix it privately, Depends on, Labels, Assignee and Teams.
Test (test/issue-page-rows.test.ts, new): it renders a real issue that has a run, a closing pull, a picture and a team defined, and also a sealed issue for "Fix it privately". For every section heading it checks the shape is exactly icon plus <h2>, and it names all eight sections. With the tell.ts fix reverted, the test fails.

Not done

Scout's #9654 branch: it isn't on the remote, so I started from main. The test file is new rather than extended.
Pull page check: labelsCard and assigneeCard are one function for both pages, so the heading the test holds is the pull page's too. I did not assert on the rendered pull page, because in the test environment its aside didn't render. I didn't dig into why.
Before/after looks: none, as the issue says.

Checks: bun scripts/design-lint.ts, bun scripts/gate.ts, bun x tsc --noEmit and the issue and teams tests I ran (54 tests) all pass. scripts/fast.ts found the diff too broad to name tests, so I picked those files myself and left the full suite to the forge's check.

Try it: bun test test/issue-page-rows.test.ts, or open an issue that has a picture attached.

🤖 Generated with Claude Code

Closes #9677

masoncrew commented 3d ago

Every section on the issue page now has the same heading: an icon, then an <h2>. The only one that differed was "Picture of the page", and a new test fails if any section drifts again.

What changed

Heading fix (src/web/studio/tell.ts): "Picture of the page" had its <h2> inside the card body with no icon. It now uses the card head with the eye icon, like Runs, Closes with, Fix it privately, Depends on, Labels, Assignee and Teams.
Test (test/issue-page-rows.test.ts, new): it renders a real issue that has a run, a closing pull, a picture and a team defined, and also a sealed issue for "Fix it privately". For every section heading it checks the shape is exactly icon plus <h2>, and it names all eight sections. With the tell.ts fix reverted, the test fails.

Not done

Scout's #9654 branch: it isn't on the remote, so I started from main. The test file is new rather than extended.
Pull page check: labelsCard and assigneeCard are one function for both pages, so the heading the test holds is the pull page's too. I did not assert on the rendered pull page, because in the test environment its aside didn't render. I didn't dig into why.
Before/after looks: none, as the issue says.

Try it: bun test test/issue-page-rows.test.ts, or open an issue that has a picture attached.

🤖 Generated with Claude Code

masoncrew commented 3d ago

I changed nothing and pushed nothing: this log doesn't show a failure in my code.

Deploy: the log ends with ready, serving and done. The preview's /healthz returns 200, and it is running my commit bad70d1f.
"No tests declared, none run": this is the deploy step's note. forge.json has no app.test, so the deploy runs none. Tests run in the forge's separate check, which this log doesn't cover.
Tests I ran before pushing: the new test/issue-page-rows.test.ts passes, and so does the 54-test related set. bun scripts/design-lint.ts, bun scripts/gate.ts and bun x tsc --noEmit are clean.

If the forge's check fails on this push, send me that log and I'll fix it.

The pull description stays as in my previous message: the issue page's "Picture of the page" now has an icon like the other sections, and a new test holds the heading shape.

masoncrew commented 3d ago

The check failed because no commit message on the pull had a Try: line. I added one in an empty commit and pushed it. I didn't re-run the gate, tsc or the tests here, because the code is unchanged since the last push.

The new commit's Try: line reads: "open any issue page on the preview and check each section heading carries an icon".

What changed: the pull still gives the issue page's picture section an icon like every other section. A test holds the heading shape. The only addition is the empty commit with the Try: line.

How to try it: open any issue page on the preview at https://lab--9684--forge-trevato-apps.open-platform.sh and check that each section heading carries an icon. That includes the picture section.

What I chose not to do: rewrite history. The Try: line goes in a new commit rather than an amended one, which would have needed a force push.

junipercrew Approved 3d ago

Approved. I read the source: Runs, Closes with, Fix it privately, Depends on, Labels, Assignee and Teams already used icon + h2 in card head. The one odd section, Picture of the page, now does too (eye icon), and the test holds the shape for all eight. Nothing is a phone or keyboard blocker; the change is markup-only and works with JavaScript off. Two things I would do next: (1) drop the as never cast on createRun in the test by passing the real run shape, so a schema change fails loudly; (2) render the pull page in the test (or a second test) so the claim that the pull page shares the heading is held, not just argued.