style(DST-1726): make component categories distinguishable in the docs sidebar - #5748
Open
sebald wants to merge 1 commit into
Open
style(DST-1726): make component categories distinguishable in the docs sidebar#5748sebald wants to merge 1 commit into
sebald wants to merge 1 commit into
Conversation
…s sidebar Category labels differed from component links by only `font-bold` and a slight colour shift, so the sidebar read as one flat list. Give them their own type scale instead: 11px, uppercase, tracked, full contrast, with asymmetric space so a label belongs to the group below it. Fumadocs draws its folder rail at depth 1, which on a component page is `Components` — one line running past all ten category labels. Make the rail follow content instead of depth: trees whose children contain separators skip it, flat trees (Getting Started, Foundations) keep it, and with it the track the active-page marker sits on.
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
github-actions
Bot
added
the
type:docs
Contributor
|
Accessibility tests executed. Download the report here. |
Contributor
Coverage Report for Marigold Code Coverage
File CoverageNo changed files found. |
sarahgm
added a commit
that referenced
this pull request
Sep 4, 2026
… both key sources Section 1 was showing PRs other people are already reviewing. #5684 has two reviewers, every thread resolved, and nothing there for a third opinion, yet it ranked first. So the section now keeps exactly two kinds of PR: ones nobody has reviewed (no reviews, or only dismissed ones), and ones you have reviewed yourself. An unresolved preview comment does not count as somebody reviewing. It is feedback for the author, not a code review, so #5740 and #5748 stay in the queue: nobody has read either one. Cut rows are named in a `+N cut (others reviewing)` line rather than dropped, because a PR others commented on and nobody approved can otherwise sit forever without ever reaching you. Rendering real PR titles then exposed a worse bug. Step 5 copied `/create-pr`'s first-hit-wins cascade, branch before title, which silently picks the wrong ticket when the two disagree. #5776's branch is `dst-1745_fix-popover` and its title is `fix(DST-1754): keep popovers inside the body's clip box`: transposed digits. DST-1745 is a Core-only invoice-printing migration that is Done, DST-1754 is the popover bug the PR actually fixes and is In Review in the active sprint. One typo produced three wrong rows: the PR was demoted as "ticket is Done", it sank below a key-less PR, and step 8 reported DST-1754 as having no PR at all. So step 5 now reads both sources every time and prefers the title when they disagree, flagging the row. The title is prose, read by every reviewer, corrected when wrong, and lands in the changelog, while a branch name is typed once and never fixed. Checked across all 32 open PRs in the org: #5776 is the only disagreement, so the flag almost never fires. That also retires the Done edge case's example. The rule stands, but there is no live instance of an open PR on a Done ticket, and the one that looked like it was a typo wearing a costume.
sarahgm
added a commit
that referenced
this pull request
Sep 4, 2026
… the example Two rules could not be followed as written. The red-check table sent a `CheckRun` with an empty `workflowName` to the integration row, which files the bare `CodeQL` aggregate as somebody else's problem. That is the exact mislabelling the paragraph underneath it forbids, and the table is the part an implementer follows. On #5776 the two empty-name checks are `Vercel Preview Comments` and that `CodeQL` aggregate, sitting alongside the `Analyze (javascript)` and `Analyze (typescript)` runs that do carry `workflowName: "CodeQL"`. Rows are now matched in order, first hit wins, with the name test before the type test, and the CI row tests no `workflowName` at all. Applied to #5776's 27 checks that gives 1 preview-comments, 3 deploy, 23 our-CI. `StatusContext` carries the deploy row alone, because no live `CheckRun` is genuinely third-party. The `statusCheckRollup` fetch was scoped to the rows about to be printed, which section 2 cannot know beforehand: a CI failure and unresolved preview comments are two of its four qualifying conditions and exist nowhere but that rollup. Section 1 needs it on every candidate too, since #5740 and #5748 are in the queue only because of a red preview check. It now runs once over the whole shortlist, before either filter. `latestReviews` does not return an empty `commit.oid`. It returns the full oid on all three PRs the turn table cites, so the `reviews(last:30)` block that claim justified had no remaining consumer and is gone. The turn verdicts are unchanged: #5779 author (`5f38f3e` both sides), #5776 yours (`1165e01` against `63ea2b5`), #5761 yours. Three smaller corrections. The largest departure from the ticket, dropping `--review-requested=@me`, was the only one with no rationale written down, so it now carries the numbers: 2 of 15 open PRs on marigold have a review request of any kind and none of them is mine, so the ticket's query returns an empty queue that reads as a clear board. The archived-repo filter asked for `name` and compared it to `owner/repo`, which matches nothing and leaves every archived repo in the queue, so it asks for `nameWithOwner`. And the ranking tiers are named rather than numbered, which also fixes an off-by-one where the prose said "tier 5" while describing tier 6. Refreshing the example against a full run left the rows and their order alone, and changed three things. Five section-1 titles and all four tail titles were shortened past what step 9 describes, which only says to strip the Conventional Commits prefix, so they are the real titles now. The rank-instability figures were wrong and are measured: DST-1717 came back 3rd of 5 keys and 6th of 8. And 5761 carries a `reviews.totalCount` of 47 against 3 live threads, not the 30 a `last:30` window reports back. Running the skill again turned up three more. The tail was not in rank order. Its queries already carry `ORDER BY Rank ASC`, but DST-1529 is tenth of the fourteen tickets in review and DST-1759 fourteenth, so the example ending with 1529 had been sorted by something else. Step 8 now says that removing rows from a ranked list does not reorder the survivors, which is what goes wrong if you collect the leftovers into a set and render the set. Two shell traps join the `while read` rule, because all three are a metacharacter doing something other than what it looks like. `set -- $spec` inside a loop fails the same way `for r in $REPOS` does, so fields get read with `while IFS=' ' read -r`. And `.name?//""` does not compile at all, because jq parses `?//` as the destructuring-alternative operator. That one is quiet inside a loop: the rows still print and only the cells fed by the broken filter are wrong, which is how it produced a full digest with every preview-comment cell inverted. Checking that guard corrected a claim rather than adding one. `workflowName` is present on every `CheckRun` and merely empty on some, so the guard exists for `name`, which a `StatusContext` genuinely lacks. Both key sets are written down now, which also explains why a failing deploy is read off `state`: a `StatusContext` has no `conclusion`.
sarahgm
added a commit
that referenced
this pull request
Sep 4, 2026
… both key sources Section 1 was showing PRs other people are already reviewing. #5684 has two reviewers, every thread resolved, and nothing there for a third opinion, yet it ranked first. So the section now keeps exactly two kinds of PR: ones nobody has reviewed (no reviews, or only dismissed ones), and ones you have reviewed yourself. An unresolved preview comment does not count as somebody reviewing. It is feedback for the author, not a code review, so #5740 and #5748 stay in the queue: nobody has read either one. Cut rows are named in a `+N cut (others reviewing)` line rather than dropped, because a PR others commented on and nobody approved can otherwise sit forever without ever reaching you. Rendering real PR titles then exposed a worse bug. Step 5 copied `/create-pr`'s first-hit-wins cascade, branch before title, which silently picks the wrong ticket when the two disagree. #5776's branch is `dst-1745_fix-popover` and its title is `fix(DST-1754): keep popovers inside the body's clip box`: transposed digits. DST-1745 is a Core-only invoice-printing migration that is Done, DST-1754 is the popover bug the PR actually fixes and is In Review in the active sprint. One typo produced three wrong rows: the PR was demoted as "ticket is Done", it sank below a key-less PR, and step 8 reported DST-1754 as having no PR at all. So step 5 now reads both sources every time and prefers the title when they disagree, flagging the row. The title is prose, read by every reviewer, corrected when wrong, and lands in the changelog, while a branch name is typed once and never fixed. Checked across all 32 open PRs in the org: #5776 is the only disagreement, so the flag almost never fires. That also retires the Done edge case's example. The rule stands, but there is no live instance of an open PR on a Done ticket, and the one that looked like it was a typo wearing a costume.
sarahgm
added a commit
that referenced
this pull request
Sep 4, 2026
… the example Two rules could not be followed as written. The red-check table sent a `CheckRun` with an empty `workflowName` to the integration row, which files the bare `CodeQL` aggregate as somebody else's problem. That is the exact mislabelling the paragraph underneath it forbids, and the table is the part an implementer follows. On #5776 the two empty-name checks are `Vercel Preview Comments` and that `CodeQL` aggregate, sitting alongside the `Analyze (javascript)` and `Analyze (typescript)` runs that do carry `workflowName: "CodeQL"`. Rows are now matched in order, first hit wins, with the name test before the type test, and the CI row tests no `workflowName` at all. Applied to #5776's 27 checks that gives 1 preview-comments, 3 deploy, 23 our-CI. `StatusContext` carries the deploy row alone, because no live `CheckRun` is genuinely third-party. The `statusCheckRollup` fetch was scoped to the rows about to be printed, which section 2 cannot know beforehand: a CI failure and unresolved preview comments are two of its four qualifying conditions and exist nowhere but that rollup. Section 1 needs it on every candidate too, since #5740 and #5748 are in the queue only because of a red preview check. It now runs once over the whole shortlist, before either filter. `latestReviews` does not return an empty `commit.oid`. It returns the full oid on all three PRs the turn table cites, so the `reviews(last:30)` block that claim justified had no remaining consumer and is gone. The turn verdicts are unchanged: #5779 author (`5f38f3e` both sides), #5776 yours (`1165e01` against `63ea2b5`), #5761 yours. Three smaller corrections. The largest departure from the ticket, dropping `--review-requested=@me`, was the only one with no rationale written down, so it now carries the numbers: 2 of 15 open PRs on marigold have a review request of any kind and none of them is mine, so the ticket's query returns an empty queue that reads as a clear board. The archived-repo filter asked for `name` and compared it to `owner/repo`, which matches nothing and leaves every archived repo in the queue, so it asks for `nameWithOwner`. And the ranking tiers are named rather than numbered, which also fixes an off-by-one where the prose said "tier 5" while describing tier 6. Refreshing the example against a full run left the rows and their order alone, and changed three things. Five section-1 titles and all four tail titles were shortened past what step 9 describes, which only says to strip the Conventional Commits prefix, so they are the real titles now. The rank-instability figures were wrong and are measured: DST-1717 came back 3rd of 5 keys and 6th of 8. And 5761 carries a `reviews.totalCount` of 47 against 3 live threads, not the 30 a `last:30` window reports back. Running the skill again turned up three more. The tail was not in rank order. Its queries already carry `ORDER BY Rank ASC`, but DST-1529 is tenth of the fourteen tickets in review and DST-1759 fourteenth, so the example ending with 1529 had been sorted by something else. Step 8 now says that removing rows from a ranked list does not reorder the survivors, which is what goes wrong if you collect the leftovers into a set and render the set. Two shell traps join the `while read` rule, because all three are a metacharacter doing something other than what it looks like. `set -- $spec` inside a loop fails the same way `for r in $REPOS` does, so fields get read with `while IFS=' ' read -r`. And `.name?//""` does not compile at all, because jq parses `?//` as the destructuring-alternative operator. That one is quiet inside a loop: the rows still print and only the cells fed by the broken filter are wrong, which is how it produced a full digest with every preview-comment cell inverted. Checking that guard corrected a claim rather than adding one. `workflowName` is present on every `CheckRun` and merely empty on some, so the guard exists for `name`, which a `StatusContext` genuinely lacks. Both key sets are written down now, which also explains why a failing deploy is read off `state`: a `StatusContext` has no `conclusion`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
On component pages the docs sidebar read as one flat list of ~100 rows. Two causes, both addressed here:
Category labels looked like nav items. A label (
Application,Layout,Actions, …) differed from a component link by only two signals —font-boldand a slightly brighter colour. Same 14px size, same sentence case, same 20px indent, same rhythm; in dark mode the colour delta is barely visible. They now get their own type scale instead of a heavier weight: 11px, uppercase,0.09emtracking,font-semibold,text-fd-foreground, plus asymmetric spacing (mt-7 mb-1) so a label reads as belonging to the group below it.One rail ran through the whole tree. Fumadocs draws its folder rail in
SidebarFolderContentat depth 1 — on a component page that depth-1 folder isComponents, so a single line ran past all ten category labels and every component row, asserting "this is one branch" against exactly the grouping the categories communicate. The rail now follows content instead of depth:Trees that group their pages into categories (Components, Patterns, Releases) render no rail; flat trees (Getting Started, Foundations) keep Fumadocs' behaviour unchanged — and with it the track the active-page marker sits on.
Top-level section triggers also get
text-fd-foreground font-medium, so section → category → page are three distinct styles rather than two.Deliberately not in scope: the active-page marker (
itemVariants'highlightvariant) is untouched; no sticky category labels (tried, dropped — rows scrolling under a pinned label read as broken); no horizontal divider above labels (type and space carry it); no page-tree restructure — categories stay---Label---separators inmeta.json.Closes DST-1726
Screenshots / Preview
Sidebar only; easiest to see on the docs preview deployment — compare
/components/actions/button(categories, no rail) with/foundations/typography(flat, rail present).Before / after, on a component page:
ACTIONSandButtonsit at the same size and case, 4px apart, with a continuous vertical line to their left spanning every category.ACTIONSis a small tracked caps label with 28px of space above it and 4px below; no line except the 1pxfd-primarymarker on the active page.Test Instructions
pnpm start, open http://localhost:3000/components/actions/button.Breaking Changes
No
Checklist
Stories added/updated— docs app component, not a Marigold componentUnit tests added/updated— presentational classNames only; verified in the browserComponent documentation added/updated— n/aneutral-300tofd-foreground)Visual regression tests updated— the docs app is not covered by ChromaticChangeset added—@marigold/docsis private and no package underpackages/orthemes/changed