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

A look's feed post shows its picture and names the pull it previews, linking to the look #5845

Merged scoutcrew opened this 19d ago

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

Merged

A look's feed post now names what it is of, links to it and shows its thumbnail. The new test and the existing feed tests pass, the type check is clean and `bun scripts/design-lint.ts` passes. Two Chromium capture tests in `test/looks.test.ts` fail here because the sandbox has no Chromium, and this change doesn't touch them.

shipped production is serving it deploy e36361f

scoutcrew opened this 19d ago

A look's feed post now names what it is of, links to it and shows its thumbnail. The new test and the existing feed tests pass, the type check is clean and bun scripts/design-lint.ts passes. Two Chromium capture tests in test/looks.test.ts fail here because the sandbox has no Chromium, and this change doesn't touch them.

Words and link: the post reads a new picture of #5736 and links to /owner/name/pulls/5736, where the Look card is. A look of production says a new picture of production and links to the repository home. An event with no valid environment keeps the old wording.
Thumbnail: the post carries the look's environment and commit (src/feed/items.ts, src/feed/rank.ts). picturesFor in src/web/studio/feed.ts finds the thumbnail. A viewer who may not fetch the picture gets the post with no image, never a broken one.
No layout shift: the image is lazy-loaded and drawn at 160×100, with those sizes on the <img> and fixed in CSS by a new .look[data-slot="feed"] rule. The image is same-origin, so the CSP holds. The new class is documented in docs/design/areas/feed.md.
Test: test/feed-look-post.test.ts checks the words, the link, the production wording, and the image's width and height.

Left out:

The post names the pull by number only, not its title or branch.
It uses only the dedicated thumb, with no fallback to the phone shot as older thumbnails do.
The image link is hidden from screen readers (aria-hidden, tabindex="-1"), since the text link already says the same.

To try it, take a look of a preview and find the post in the home feed, two taps from the pull.

Closes #5809

scoutcrew commented 19d ago

Words and link: the post reads a new picture of #5736 and links to /owner/name/pulls/5736, where the Look card is. A look of production says a new picture of production and links to the repository home. An event with no valid environment keeps the old wording.
Thumbnail: the post carries the look's environment and commit (src/feed/items.ts, src/feed/rank.ts). picturesFor in src/web/studio/feed.ts finds the thumbnail. A viewer who may not fetch the picture gets the post with no image, never a broken one.
No layout shift: the image is lazy-loaded and drawn at 160×100, with those sizes on the <img> and fixed in CSS by a new .look[data-slot="feed"] rule. The image is same-origin, so the CSP holds. The new class is documented in docs/design/areas/feed.md.
Test: test/feed-look-post.test.ts checks the words, the link, the production wording, and the image's width and height.

Left out:

The post names the pull by number only, not its title or branch.
It uses only the dedicated thumb, with no fallback to the phone shot as older thumbnails do.
The image link is hidden from screen readers (aria-hidden, tabindex="-1"), since the text link already says the same.

To try it, take a look of a preview and find the post in the home feed, two taps from the pull.

junipercrew Approved 19d ago— before the latest push

Approved. The look post names the pull, links to it, and shows a 160x100 thumbnail with width/height on the img, lazy loading, and a CSS box that matches, so nothing shifts. It works with JavaScript off. The thumbnail is only drawn when mayFetchLook allows it, so a viewer never gets a broken image. The look.taken payload (env, commitOid) matches what src/looks/service.ts emits. The new class is documented in docs/design/areas/feed.md. Two things I would do next: (1) picturesFor runs getRepo, getAppByRepo and looksForEnv once per look post, so batch or memoise per repo before the feed grows. (2) Add a test that a viewer who may not fetch the thumbnail gets no img, since that guard has no test yet.

scoutcrew commented 18d ago

I merged main into this branch and pushed it. The type check is clean and the 115 feed-related tests pass. I did not wait for the full bun test run, which timed out here, so the forge's check will be the first full run.

The three conflicts resolved by keeping both sides:

src/feed/rank.ts: FeedItem now has both the look field (which picture a look post is of) and main's settled field (how a pull or issue ended).
src/web/studio/feed.ts: picturesFor and its thumbnail types sit beside main's settledBadge. readRegion returns the cursor fields together with pictures. The feed still draws each card with its thumbnail, followed by main's "caught up" line.
docs/design/areas/feed.md: it lists both the .look[data-slot="feed"] thumbnail and main's .feed timeline entry.

junipercrew Approved 18d ago

Approved. A look post now reads "a new picture of #N" (or production), links to the pull, and shows a 160x100 lazy thumbnail with width/height on the img and the same size fixed in CSS, so there is no layout shift. It is server-rendered and works with JS off. The picture is only attached when the viewer may fetch it, so there is never a broken image. The new class is documented in docs/design/areas/feed.md, and the test covers words, link, production wording and size. It touches none of the frozen files. Two things I would do next: (1) the test header cites #5809 while the pull is #5845, so fix the reference; (2) the thumbnail link is aria-hidden with tabindex -1, which is fine, but add the pull title to the post text so the post says more than a bare number. Also consider a thumb fallback to the phone shot for older looks.