Repository navigation
feat(docsite): nest docs pages under a parent page in the sidebar - #9786
nynexman4464 wants to merge 6 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
github-actions
Bot
added
the
needs:design-review
meta-cla
Bot
added
the
CLA Signed
github-actions
Bot
added
the
needs:spec-owner-review
PR Analysis Report🔎 Preview ·
|
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on the spec:AST-046 FR14 amendment, and two fixes are needed whatever that decision is.
Spec decision: this head adds to FR14 and its verification row that the docsite MAY give a nested namespace it names a page of its own (its sidebar entry under the root page, its guide slugs redirecting into it), that the docsite MAY name a sidebar parent for any page and show it in the parent's group, and that nesting is one level deep; current FR14 keeps every guide on its root namespace's page and does not cover nesting a page under another or showing it in another group. An engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner.
Blocking:
- A child page reached by in-site navigation, such as the site search, keeps its parent collapsed and hides the current entry, because
DocsShell.tsxpasses the open state asdefaultIsCollapsed, which the sidebar reads only when it first mounts; make the open state follow the current path and add a test that moves from a parent page to a child page. - The description says Styling components "lists apart from" Styling overview, but on the base it is the item directly above it, so please reword that example to describe the missing nesting instead.
[Automated review]
| key={page.topic} | ||
| {...docLink(page)} | ||
| collapsible={{ | ||
| defaultIsCollapsed: !children.some( |
There was a problem hiding this comment.
defaultIsCollapsed is read once, when this item mounts, and DocsShell stays mounted across in-site navigation. Reaching a child page from the site search (or any soft navigation) leaves this parent collapsed, so the current entry is hidden. Consider a controlled isCollapsed that is open whenever a child is current and otherwise follows the reader's toggle, plus a test that moves from the parent path to a child path.
| when every guide below it is a foundations doc, and `guide` otherwise. A | ||
| when every guide below it is a foundations doc, and `guide` otherwise. The | ||
| docsite MAY name a sidebar parent page for any page: the page keeps its | ||
| slug and its own group, and its sidebar entry sits under the parent's |
There was a problem hiding this comment.
Non-blocking: "keeps its slug and its own group" reads against "in the parent's group" in the same sentence; Icon sets keeps category: 'guide' but shows in Foundations. "keeps its slug and its category" would say what the code does.
b12166a to
ef35b34
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on the spec:AST-046 FR14 amendment, and one fix is needed whatever that decision is.
Spec decision: this head adds to FR14 and its verification row that the docsite MAY name a sidebar parent for any page and show it in the parent's group while it keeps its slug and category, that sidebar nesting is one level deep, and that the docsite MAY give a nested namespace it names a page of its own (its sidebar entry under the root page, its guide slugs redirecting into it). Current FR14 keeps every guide on its root namespace's page and treats an address that lands in another sidebar group as a failure, so these claims need an owner's decision. An engineering owner records it by commenting /approve-spec <full head SHA> on this pull request; the author may do so as an engineering owner.
Blocking:
- On a child page such as Styling components or Icon sets, the parent's chevron is announced as "Collapse" but pressing it does nothing, because
hasCurrentChildoverrides the reader's toggle inDocsSidebarItems.tsx; let the reader's choice win on the page where they made it, and add a test that presses the toggle on a child page and expects it to collapse.
The earlier soft-navigation and description findings are fixed.
[Automated review]
| isCollapsed: !hasCurrentChild && isClosedByReader, | ||
| onCollapsedChange: setIsClosedByReader, |
There was a problem hiding this comment.
While a child is current, hasCurrentChild forces isCollapsed to false, so onCollapsedChange has no visible effect: on /docs/styling the chevron stays "Collapse Styling overview", aria-expanded="true", after any number of presses. Consider keeping the reader's choice with the path it was made on, for example {path, isCollapsed} in state, and using it while path === pathname, so the parent still opens on arrival at a child but the reader can fold it there. A test that presses the toggle on a child page would cover it.
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on this pull request's intentional spec:AST-046 FR14 amendment; no blocking code change is needed.
An engineering owner needs to approve the updated FR14 and its verification row, which add these claims that current FR14 does not state (today every guide stays on its root namespace's page, and an address that lands in another sidebar group is a failure):
- the docsite MAY name a sidebar parent for any page, which keeps its slug and
categoryand shows under the parent in the parent's group; - sidebar nesting is one level deep;
- the docsite MAY give a nested namespace it names a page of its own, under the root page's entry, with its guide slugs redirecting into it.
An engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. The earlier collapse-toggle finding is fixed.
- Regression (non-blocking):
mainnow passesDocsShella projected nav model whose topics carry onlytopic,titleandcategory, and this branch conflicts with it inDocsShell.tsx; when you resolve it, carryparentthroughDocsNavTopicandgetDocsNav, since no current test fails if it is dropped.
[Automated review]
| .sort(foundationsSort); | ||
| // Classify doc topics by category (from data). | ||
| const {guide: guideItems, foundations: foundationItems} = | ||
| docsSidebarGroups(docTopics); |
There was a problem hiding this comment.
Non-blocking: main now hands DocsShell a projected nav model (docsNav.ts) whose topics keep only topic, title and category, and this branch conflicts with it here. When resolving, add parent to DocsNavTopic and to the getDocsNav projection; otherwise every nesting disappears while typecheck and the current tests stay green. Running the real-site case in docs-sidebar-data.test.ts over getDocsNav().docTopics would catch it.
FR14 kept every nested namespace on its root namespace's page, and a page in its own sidebar group. The docsite may now give a nested namespace it names a page of its own, listed under the root page, and may name a sidebar parent for any page: the page keeps its slug and group, and shows under the parent in the parent's group. Nesting is one level deep.
A page with a sidebar parent shows as a collapsible child of the parent's item, in the parent's group, in Guide and Foundations alike. The parent stays a link with its own expand toggle, open when the child page is. SIDEBAR_PARENTS names the parents; a nested namespace that NESTED_NAMESPACE_PAGES gives its own page (none yet) gets its root page. Icon set files now shows under Icons, and Styling components under Styling overview. Their URLs and the CLI docs tree do not change.
Icon sets and Icon set files are the two halves of the icon set guide, so both show under Icons in Foundations. The URL is unchanged.
"Its own group" read against "in the parent's group". The page keeps its `category`; only its sidebar entry moves to the parent's group.
…s is current The docs shell stays mounted across links and site search, and the parent entry read its open state only on mount, so a move from the parent page to its child left the child hidden. The parent entry is now open whenever a child page is current, and otherwise follows the reader's toggle.
While a child page was current, the parent entry stayed open whatever the reader pressed, so its "Collapse" toggle did nothing there. The entry now holds its own open state: each move to one of its pages opens it, and the reader's toggle wins until the next move.
b822ccc to
8c84053
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on this pull request's intentional spec:AST-046 FR14 amendment; no blocking code change is needed.
An engineering owner needs to approve the updated FR14 and its verification row, which add these claims that current FR14 does not state (today every guide stays on its root namespace's page, and an address that lands in another sidebar group is a failure):
- the docsite MAY name a sidebar parent for any page, which keeps its slug and
categoryand shows under the parent in the parent's group; - sidebar nesting is one level deep;
- the docsite MAY give a nested namespace it names a page of its own, under the root page's entry, with its guide slugs redirecting into it.
An engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. The earlier conflict with the projected nav model is resolved: parent now reaches the sidebar through docsNav.ts. Two non-blocking notes on silent no-op map entries are inline.
[Automated review]
| const bySlug = new Map(pages.map(page => [page.topic, page])); | ||
| for (const [child, parent] of parents) { | ||
| const page = bySlug.get(child); | ||
| if (!page || !bySlug.has(parent) || child === parent) { |
There was a problem hiding this comment.
Non-blocking: this accepts any registry page on either side, but docsSidebarGroups nests only under Guide and Foundations pages (docsSidebarData.ts:61-71). An entry naming getting-started as parent, or a page with no category as child, passes the build and changes nothing. Checking both pages' sidebar groups here would make it fail like the other bad entries.
| const nested = []; | ||
| for (const route of nestedRoutes) { | ||
| const page = await namespacePage(readDocs, route, {slug: routeSlug(route)}); | ||
| if (!page) continue; |
There was a problem hiding this comment.
Non-blocking: a NESTED_NAMESPACE_PAGES route that is not a namespace with a guide is skipped here without an error, and entries keyed by cli are never read in docsTreePages. A mistyped entry therefore builds silently, unlike a bad SIDEBAR_PARENTS entry. The map is empty today, so this matters from its first entry.
User need
The docs sidebar lists every guide and foundations page flat, sorted by title, so a page that belongs to another page sits far from it. "Icon sets" and "Icon set files" list in Guide, away from Icons. "Styling components" lists as a peer directly above "Styling overview", though Styling overview is the page that chooses between the styling paths.
A reader who opens the parent page can't see its related pages. A reader who lands on a related page loses where it belongs.
Problem and solution fit
SIDEBAR_PARENTSinapps/docsite/scripts/docs-pages.mjsmaps child slug to parent slug. The first entries:NESTED_NAMESPACE_PAGESlets a root namespace give a nested docs-tree namespace a full page of its own at its route's slug. The root page leaves that subtree out, the nested guides' slugs redirect into the new page, and the root page becomes its sidebar parent. It starts empty; the calendars guide (docs(i18n): calendars guide, with Calendars nested under Internationalization #9625) is its first entry.DocsNavTopicindocsNav.tscarriesparent, so the nav modelDocsShellreceives still nests pages.docsSidebarData.tsgroups them, andDocsSidebarItems.tsxrenders the collapsible parent entry.Current specification
spec:AST-046FR14 owns docsite pages and sidebar groups for the docs tree. This PR amends FR14 and its verification row:category, and shows under the parent in the parent's group;Public delta and caller burden
None for package consumers: this is the docsite only. Docsite maintainers add one map entry to nest a page. The build fails when an entry names a missing page, a page under itself, or a second level.
Design and alternatives
The site keeps the parent map next to the existing
GUIDE_NAMESPACE_PAGESandMERGED_TOPICSmaps, so doc files and the CLI tree are unchanged.Rejected: a sidebar field in each doc file, which would add a docs-tree field for one site's navigation.
Evidence
docs-nav.test.tschecks that the nav model passesparentthrough.Screenshots
Local docsite build of this branch, taken before the rebase onto the new nav model; the rendered sidebar is unchanged.
Scope
Testing
pnpm -F @astryxdesign/core build,pnpm -F @astryxdesign/lab build, thenpnpm -F docsite generate. Docsitetscneeds current CLI API types (pnpm -F @astryxdesign/cli sync:api-types).cd apps/docsite && pnpm exec vitest run: 805 passed, 7 skipped.cd apps/docsite && pnpm exec tsc --noEmit -p .: clean.pnpm check:knowledge: clean.pnpm review:pre --base main: 737 authored lines, no findings.next devwith Chromium: parent links,aria-expanded, the child'saria-current, and the collapsed state on the parent page.Writing rubric results
spec:AST-046: