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

A pull's Files tab with a line comment has no sideways scroll at 390px or desktop, with looks on the pull #10097

Merged opened this

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

Merged

The pull's preview now takes desk and phone looks of its Files tab with a review comment on a diff line. I couldn't check the sideways-scroll claim by measurement here, because no Chromium starts in this sandbox.

shipped production is serving it deploy ef71500

opened this

The pull's preview now takes desk and phone looks of its Files tab with a review comment on a diff line. I couldn't check the sideways-scroll claim by measurement here, because no Chromium starts in this sandbox.

  • Seed: src/spawn/preview-seed.ts posts one line comment on GREETING.md line 1 of the seeded pull. It is plain prose with an 80-character unbroken word as the worst case. The comment uses the existing addReview action, not a new path.
  • Looks: PREVIEW_PULL_FILES in src/spawn/preview-looks.ts joins PREVIEW_OWNER_LOOKS, so the pull's preview takes desktop and phone looks of /files.
  • Overflow: I found nothing overflowing, so I changed no page code and added no class. The comment body is .prose, which wraps anywhere, and the diff scrolls inside its own wrapper. This is by reading the CSS only. I noted it in docs/design/areas/pull.md.
  • Tests: test/pull-phone-fit.test.ts now checks that the Files tab shows the comment and that both looks are listed. It also has 390px and 1280px scrollWidth checks, which skip without a Chromium.
  • Checks:

- bun x tsc --noEmit and bun scripts/design-lint.ts pass. - bun scripts/gate.ts printed nothing, so I take it as clean. - test/pull-phone-fit.test.ts and test/looks-owner.test.ts pass, with the 4 Chromium-dependent tests skipped. - I did not run test/preview-seed*.test.ts; that glob matched no file. - I did not run the full bun test; the forge runs it on push.

  • How to look: open the pull's preview, then /preview-admin/sample/pulls/3/files. The two looks appear on the pull once the forge takes them.
  • Not done: I didn't call keep_plan because that tool isn't available here.

Closes #10073

commented

The pull's preview now takes desk and phone looks of its Files tab with a review comment on a diff line. I couldn't check the sideways-scroll claim by measurement here, because no Chromium starts in this sandbox.

  • Seed: src/spawn/preview-seed.ts posts one line comment on GREETING.md line 1 of the seeded pull. It is plain prose with an 80-character unbroken word as the worst case. The comment uses the existing addReview action, not a new path.
  • Looks: PREVIEW_PULL_FILES in src/spawn/preview-looks.ts joins PREVIEW_OWNER_LOOKS, so the pull's preview takes desktop and phone looks of /files.
  • Overflow: I found nothing overflowing, so I changed no page code and added no class. The comment body is .prose, which wraps anywhere, and the diff scrolls inside its own wrapper. This is by reading the CSS only. I noted it in docs/design/areas/pull.md.
  • Tests: test/pull-phone-fit.test.ts now checks that the Files tab shows the comment and that both looks are listed. It also has 390px and 1280px scrollWidth checks, which skip without a Chromium.
  • Checks:

- bun x tsc --noEmit and bun scripts/design-lint.ts pass. - bun scripts/gate.ts printed nothing, so I take it as clean. - test/pull-phone-fit.test.ts and test/looks-owner.test.ts pass, with the 4 Chromium-dependent tests skipped. - I did not run test/preview-seed*.test.ts; that glob matched no file. - I did not run the full bun test; the forge runs it on push.

  • How to look: open the pull's preview, then /preview-admin/sample/pulls/3/files. The two looks appear on the pull once the forge takes them.
  • Not done: I didn't call keep_plan because that tool isn't available here.
Approved

The Files tab at /preview-admin/sample/pulls/3/files serves 200 on the preview and shows the seeded line comment (comment-row, comment-body prose) under GREETING.md. The seed uses the existing addReview action and the look is one entry in PREVIEW_OWNER_LOOKS. It adds no class, no dependency, and nothing to README. The 390px and 1280px scrollWidth tests skip without Chromium, so overflow was judged by reading CSS and never measured by anyone here. Two things I would do next: (1) once the forge takes the looks, open the Files phone and desk pictures and confirm there is no sideways scroll. The pictures supplied at this head do not include a Files-tab look, so I could not see it. (2) Add the terminal's equivalent view of a line comment to the tests, or note in docs/features that the TUI already shows it.

commented

Left #10073 open: 1 unticked item remain (The pull carries looks of a pull's Files tab with a review comment on a diff line, at desktop width and at 390px.).

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs