Self-hosted: give Magazine a hero-plus-rows archive - #1472
Conversation
Magazine was a colour and type treatment on the shared list, so its name promised a structure it did not have. The newest entry now renders as a hero (full-width image, display headline, roomier excerpt) with the rest as ordinary rows. The feed frame is extracted so this costs no duplication: fetching, paging and the four failure states move to ArchiveFrame, and both the default archive and Magazine's supply only the layout. Copying that logic was the alternative, and it is exactly the copy that drifts and then loses a reader's place in the feed on a failed page.
Code Review by Qodo
1.
|
|
Warning Review limit reached
Next review available in: 55 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Summary by QodoMagazine theme: hero+rows archive via shared ArchiveFrame
AI Description
Diagram
High-Level Assessment
Files changed (6)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd6174cf4c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const postSearch = { raw: undefined }; | ||
|
|
||
| return ( | ||
| <article className="magazine-hero pb-8 mb-2 border-b border-theme"> |
There was a problem hiding this comment.
Make the magazine hero span the configured grid
When Magazine uses the supported listType: 'grid'—or the value is absent and apply-config-dom.ts defaults it to grid—ArchiveFrame makes .blog-posts-list a two- or three-column grid, but this article has no full-column span. The hero therefore occupies only the first grid cell and later posts appear beside it, despite the image sizes and the new layout both assuming a full-width hero. Make the hero span all grid columns so the archive retains its hero-first structure under this configuration.
Useful? React with 👍 / 👎.
Code Review by Qodo
1.
|
…cks shrink In the grid feed the hero was an ordinary cell: one column wide with the second post beside it, which is the opposite of a hero. It now spans every column. That exposed an older bug underneath. A bare 1fr is minmax(auto, 1fr), so a track cannot shrink below its content's min-content width; the post cards are wider than a third of the reading measure, so three tracks blew out to 1179px inside a 768px column. Every template's grid feed had it. The tracks are now minmax(0, 1fr).
|
Fixed at 99bb3a9, and chasing it turned up an older bug underneath. The hero was an ordinary grid cell. Reproduced before fixing: with What that exposed. With the hero spanning the row, it came out 1221px wide inside a 768px container. The cause is not the hero: a bare
So every template's grid feed had this, and two columns hid it by fitting. Tracks are now
The hero image also stops under-fetching as a result: it was displaying at 1221px while the browser picked an 800px candidate, and now displays at 768px with the same 800px candidate. One thing I did not fix. A residual overflow remains inside the cards themselves ( 940 SPA tests and typecheck pass. |
|
Fixed at bc4e3ac, and there is a related discovery below that raises the stakes on the hero fix. The metadata row. Reproduced before fixing: in a 235px card the row wanted 386px and spilled 151px over its neighbour, so the Verified across both feed modes, since the row is shared by every template:
No card spill and no page-level horizontal scroll in any of them. Wrapping only engages where there is not enough room, so the list feed every tenant actually uses is byte-identical. The related discovery. Checking Codex's note on the same file, the two defaults for this setting disagree:
So a config that simply lacks the key renders as a grid, not a list. Every one of the twelve live tenants carries the key explicitly, so nobody is affected today, but a hand-edited The two bot findings on this file are the same hero-span issue and were already fixed at 99bb3a9, one commit before their review ran. 940 SPA tests and typecheck pass. |
Closes #1468. Independent of #1470; either can merge first.
Magazine was tokens on the shared list, so the name promised a structure it did not have. The newest entry is now a hero (full-width 16:9 image, display headline, roomier excerpt), with the rest as ordinary rows beneath a rule.
This upgrades an existing template into a real layout rather than adding a name to the picker, so the roster count does not move. It takes the roster to four layouts across eight templates.
The frame extraction
The default archive owned fetching, paging and four failure states (nothing yet, nothing at all, failed with nothing to show, failed with pages already read). A hero archive needs all of it and none of it is about layout, so it moves to
ArchiveFrame, which both archives now render inside. Each supplies only how entries are arranged.The alternative was copying that logic into the Magazine archive, and it is exactly the copy that drifts: the retry branch there is the one that decides whether a failed page loses the reader's place in the feed.
PostCarddeliberately stays the shared default, the same split Reader uses, because search results render through that seam and a hero has no meaning in a list of search hits.Behaviour worth stating
Verified by rendering it
Built the app and served the real
distthrough the published image's nginx against a real account:The sidebar stays, since Magazine is not one of the templates that drops it.
934 SPA tests and typecheck pass.
registry.test.tsmoves Magazine out of the CSS-only set and pins what it overrides; the failure-states source guard now points atArchiveFrame, which is where the outcome handling lives.