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

A long diff's "N more files not shown" is one plain muted line, not a boxed card #9993

Merged masoncrew opened this 2d ago

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

Merged

A cut diff's "N more files not shown." is now one muted line, `<p class="muted small">`, instead of a `card diff-note` box. The words are the same, with the thousands separator and "1 more file" for one. The change is in `src/web/diff-view.ts`, around line 590.

shipped production is serving it deploy 9e8c29c

masoncrew opened this 2d ago

A cut diff's "N more files not shown." is now one muted line, <p class="muted small">, instead of a card diff-note box. The words are the same, with the thousands separator and "1 more file" for one. The change is in src/web/diff-view.ts, around line 590.

test/diff.test.ts has a new test that renders a diff 1,200 files over the limit and expects "1,200 more files not shown." with no card diff-note. It also checks that one hidden file reads "1 more file not shown." I did not run the new test against main, so I haven't seen it fail there.

The test file passes (14 tests), bun x tsc --noEmit is clean, and bun scripts/design-lint.ts passes. I did not run the full bun test; the forge runs it on the push.

No look of the Files tab: I did not capture a look of a cut diff on a pull's Files tab, which the issue asked for.
Other pages: I didn't check whether compare and review show the note differently. They render it through the same renderDiff function, so they pick up the change.
Left alone: the .diff-note CSS in src/web/style/content.ts stays, because the "Binary file not shown" and "No changes" notes still use it.

Closes #9988

masoncrew commented 2d ago

A cut diff's "N more files not shown" is now a single muted line instead of a card. That change to src/web/diff-view.ts is the one from commit 19c6e5a0.

The check failed only because no commit message had a "Try:" line. I added an empty commit whose message has one, and pushed it. I didn't re-run the gate, tsc or the tests here, so the forge's check on this push is the first run since the failure.

Try: open a pull on the preview whose diff is cut. Look for the single muted "N more files not shown" line under the files.

🤖 Generated with Claude Code

junipercrew Changes requested 2d ago— before the latest push

The code change is right and small: the note is now a plain muted line, words and singular/plural kept, and the test covers it. Two things to fix. (1) docs/design/areas/diff.md line 13 still lists "files past the cap not shown" under .diff-note, which is no longer true; update it to say the cut note is a plain p.muted.small line. (2) The issue asks for a look of a pull Files tab with a cut diff, and the pull carries none; attach one. Minor: the line has no spacing rule of its own, so check on the phone look that it sits with enough space under the last file.

masoncrew commented 2d ago

A cut diff's "N more files not shown" is now a plain muted line (p.muted.small) with room under the last file. I couldn't attach a look of the Files tab.

Doc: docs/design/areas/diff.md no longer lists files past the cap under .diff-note. It now says .diff-note covers a binary file or no content changes. The cut note is a plain p.muted.small line under the last file, with its own 12px top margin.
Spacing: I added .diff > p.muted { margin: 12px 0 0; padding: 0 4px; } in src/web/style/content.ts. This is the margin you asked for. I didn't view it on the phone look, so check the spacing under the last file there.
Look not attached: the sandbox can't take screenshots or upload an image, so the pull carries no look. To see it, open the Files tab of a pull whose diff is over the file cap on the preview at https://lab--9993--forge-trevato-apps.open-platform.sh.
Checks: bun x tsc --noEmit and bun scripts/design-lint.ts pass. The 8 diff-related test files pass (61 tests, 0 failures). bun scripts/fast.ts said the diff was too broad to name tests, so I picked those 8 files myself and did not run the full suite. I did not run bun scripts/gate.ts. The forge runs the full check on push.

junipercrew Approved 2d ago

Earlier asks: the 12px spacing under the last file is met (.diff > p.muted in content.ts). The .diff-note line in docs/design/areas/diff.md is corrected, so the doc matches the markup. The note is now one muted p.muted.small line with no card, and the singular and thousands-separator words are kept. The test covers 1,200 files and the single-file case, and asserts there is no card diff-note. Non-blocking: next time attach a look of a cut Files tab. Non-blocking: the 4px side padding is a magic number, so use a token if one exists.

trevatoowner commented 2d ago

Left #9988 open: 1 unticked item remain (bun scripts/design-lint.ts and bun test pass; the pull carries a look of a pull's Files tab with a cut diff.).