feat(packages/core,ui): auto-generate section landing pages and fix sidebar#38
Merged
zrosenbauer merged 7 commits intomainfrom Mar 16, 2026
Merged
feat(packages/core,ui): auto-generate section landing pages and fix sidebar#38zrosenbauer merged 7 commits intomainfrom
zrosenbauer merged 7 commits intomainfrom
Conversation
- Split Getting Started into Introduction (with feature cards) and Quick Start - Rename Guides section to Concepts, update all cross-references - Add top-level `actions` config for home page hero buttons - Add `sidebar` config with `above`/`below` link slots (SidebarConfig, SidebarLink types) - Remove stubbed OpenAPI config from types, schema, and sidebar plumbing - Promote HeroConfig.actions to flat `actions` field on ZpressConfig - Scope feature-grid padding to home page only (no horizontal padding in doc pages) - Move Contributing from nav to sidebar.below, use explicit nav - Add documentation-framework section, templates package, and draft command Co-Authored-By: Claude <noreply@anthropic.com>
…bar, remove && from scripts - Add auto-derived links from prefix/children common prefix for section groups - Generate landing pages with SectionCard grids for all sections with children - Fix section group names falling through to "Section" by deriving from prefix - Fix duplicate sidebar entries when first child link matches parent - Preserve explicitly configured pages when parent link is auto-derived - Redesign SectionCard with compact icon+title row and optional description - Remove all && chains from package.json scripts, use turbo tasks and pnpm lifecycle hooks - Add per-package turbo.json for config schema generation dependency Co-Authored-By: Claude <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 375f769 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (11)
📝 WalkthroughWalkthroughAdds a documentation framework and templates package, a CLI Changes
Sequence Diagram(s)sequenceDiagram
participant User as Developer
participant CLI as zpress CLI
participant Registry as Template Registry
participant Render as Renderer
participant FS as File System
participant Logger as Console
User->>CLI: run `zpress draft [type] [title] [out]`
CLI->>Registry: get(templateType)
Registry-->>CLI: Template
CLI->>Render: render(template, { title })
Render-->>CLI: markdown
CLI->>FS: ensure out dir & write file
FS-->>CLI: file written
CLI->>Logger: log success
Logger-->>User: success message
sequenceDiagram
participant Browser as User Agent
participant App as Zpress App
participant Config as Config Parser
participant Core as Core resolver
participant UI as Theme builder
participant Layout as Layout component
participant Sidebar as SidebarLinks component
Browser->>App: request page
App->>Config: load zpress.config.ts
Config-->>Core: parsed sections, sidebar, actions
Core->>UI: build themeConfig (sidebarAbove/below)
UI-->>Layout: themeConfig
Layout->>Sidebar: render(sidebarAbove)
Sidebar-->>Layout: above links markup
Layout->>Sidebar: render(sidebarBelow)
Sidebar-->>Layout: below links markup
Layout-->>Browser: final page
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/config/schemas/schema.json (1)
616-634:⚠️ Potential issue | 🟠 MajorUpdate configuration files to use
titleinstead oftextfor nav items.The nav item field was renamed from
texttotitlein the schema. The rootzpress.config.ts(lines 9, 12–13) andexamples/kitchen-sink/zpress.config.ts(lines 133–134) still use the old field name and will not match the schema. Update alltext:properties totitle:in these files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/config/schemas/schema.json` around lines 616 - 634, The schema renamed nav item field from text to title; update the config objects that define nav entries to use title instead of text — locate the nav arrays in the root zpress.config.ts (the exported config object) and in the examples/kitchen-sink zpress.config.ts (the kitchen-sink exported config) and replace each occurrence of the property name "text:" with "title:" for all nav item objects (preserve values and other fields like link/items/activeMatch).
🧹 Nitpick comments (12)
packages/config/turbo.json (1)
9-13: Consider addingtsconfig.jsonto build inputs.TypeScript compilation depends on
tsconfig.json. If it changes (e.g., compiler options), the build should re-run. Currently onlysrc/**,rslib.config.ts, andpackage.jsonare tracked.♻️ Suggested change
"build": { "dependsOn": ["generate:schema", "^build"], - "inputs": ["src/**", "rslib.config.ts", "package.json"], + "inputs": ["src/**", "rslib.config.ts", "package.json", "tsconfig.json"], "outputs": ["dist/**"] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/config/turbo.json` around lines 9 - 13, The "build" target's inputs array currently lists ["src/**", "rslib.config.ts", "package.json"] but omits the TypeScript configuration; update the "build" target (the "build" object and its "inputs" property) to include "tsconfig.json" so changes to compiler settings trigger a rebuild.packages/ui/src/config.ts (1)
100-111: Consider addingreadonlyto nested icon object properties for type consistency.The return type's icon object
{ id: string; color: string }lacksreadonlymodifiers, whileZpressSidebarLinkinuse-zpress.tsdefines it as{ readonly id: string; readonly color: string }. This inconsistency could cause type widening issues.♻️ Proposed fix
function resolveSidebarLinks( config: ZpressConfig, position: 'above' | 'below' -): readonly { text: string; link: string; icon?: string | { id: string; color: string } }[] { +): readonly { text: string; link: string; icon?: string | { readonly id: string; readonly color: string } }[] { if (config.sidebar && config.sidebar[position]) { return config.sidebar[position] } return [] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/config.ts` around lines 100 - 111, The return type of resolveSidebarLinks is inconsistent with ZpressSidebarLink because the nested icon object lacks readonly modifiers; change the function's return signature (for the icon shape in the union) from { id: string; color: string } to { readonly id: string; readonly color: string } so the resolved type matches ZpressSidebarLink and avoids type widening (update the signature on resolveSidebarLinks accordingly).packages/templates/test/built-in.test.ts (1)
6-6: Replace.map()with.forEach()and fix describe title case.Lines 6 and 14–35 have lint violations:
describetitle should use lowercase (ESLint:prefer-describe-function-title).map()is used for side-effects; use.forEach()instead (ESLint:arrow-body-style)Proposed fix
-describe('BUILT_IN_TEMPLATES', () => { +describe('built-in templates', () => { it('should have an entry for every template type', () => { const keys = Object.keys(BUILT_IN_TEMPLATES) expect(keys).toEqual(expect.arrayContaining([...TEMPLATE_TYPES])) expect(keys.length).toBe(TEMPLATE_TYPES.length) }) it('should have matching type field on every template', () => { - Object.entries(BUILT_IN_TEMPLATES).map(([key, template]) => { - return expect(template.type).toBe(key) - }) + Object.entries(BUILT_IN_TEMPLATES).forEach(([key, template]) => expect(template.type).toBe(key)) }) it('should have non-empty label and hint on every template', () => { - Object.values(BUILT_IN_TEMPLATES).map((template) => { + Object.values(BUILT_IN_TEMPLATES).forEach((template) => { expect(template.label.length).toBeGreaterThan(0) - return expect(template.hint.length).toBeGreaterThan(0) + expect(template.hint.length).toBeGreaterThan(0) }) }) it('should include {{title}} placeholder in every template body', () => { - Object.values(BUILT_IN_TEMPLATES).map((template) => { - return expect(template.body).toContain('{{title}}') - }) + Object.values(BUILT_IN_TEMPLATES).forEach((template) => expect(template.body).toContain('{{title}}')) }) it('should have a markdown heading in every template body', () => { - Object.values(BUILT_IN_TEMPLATES).map((template) => { - return expect(template.body).toMatch(/^# /m) - }) + Object.values(BUILT_IN_TEMPLATES).forEach((template) => expect(template.body).toMatch(/^# /m)) }) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/templates/test/built-in.test.ts` at line 6, Update the test to use a lowercase describe title and replace map calls used for side-effects with forEach: change describe('BUILT_IN_TEMPLATES', ...) to a lowercase string (e.g., 'built_in_templates') to satisfy prefer-describe-function-title, and in the test body where BUILT_IN_TEMPLATES.map(...) is iterating only for side effects, replace that .map(...) with .forEach(...) (referencing the BUILT_IN_TEMPLATES usage and the anonymous callback currently passed to .map).packages/ui/src/theme/components/sidebar/sidebar-links.css (1)
36-39: Add a:focus-visiblestate for keyboard navigation parity.Only hover is styled right now; adding explicit focus styling improves sidebar usability.
♿ Suggested CSS addition
.zp-sidebar-link:hover { color: var(--rp-c-text-1); background-color: var(--rp-c-bg-soft); } + +.zp-sidebar-link:focus-visible { + color: var(--rp-c-text-1); + background-color: var(--rp-c-bg-soft); + outline: 2px solid var(--rp-c-text-1); + outline-offset: 2px; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/theme/components/sidebar/sidebar-links.css` around lines 36 - 39, Add a keyboard focus style matching the hover state for the sidebar links: update the stylesheet to add a .zp-sidebar-link:focus-visible rule that sets the same color and background-color as .zp-sidebar-link:hover and provides an accessible focus indicator (e.g., outline or focus ring using a contrast-safe variable) while removing default browser outline if you provide a visible replacement; target the .zp-sidebar-link selector and use :focus-visible for keyboard-only focus.packages/ui/src/theme/components/nav/layout.tsx (1)
81-90: Prefer declarative slot composition over mutatingslots.The local object mutation at lines 82–88 violates the immutable/declarative TypeScript guideline. Use conditional spreads instead for clearer intent:
♻️ Suggested refactor
- const sidebarSlots = (() => { - const slots: { beforeSidebar?: React.ReactNode; afterSidebar?: React.ReactNode } = {} - if (aboveItems.length > 0) { - slots.beforeSidebar = <SidebarLinks items={aboveItems} position="above" /> - } - if (belowItems.length > 0) { - slots.afterSidebar = <SidebarLinks items={belowItems} position="below" /> - } - return slots - })() + const sidebarSlots: { beforeSidebar?: React.ReactNode; afterSidebar?: React.ReactNode } = { + ...(aboveItems.length > 0 + ? { beforeSidebar: <SidebarLinks items={aboveItems} position="above" /> } + : {}), + ...(belowItems.length > 0 + ? { afterSidebar: <SidebarLinks items={belowItems} position="below" /> } + : {}), + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/theme/components/nav/layout.tsx` around lines 81 - 90, The sidebarSlots object is built by mutating a local slots variable; replace that imperative mutation with a declarative object expression that conditionally includes properties using spreads or conditional property syntax: construct sidebarSlots directly (e.g., const sidebarSlots = { ...(aboveItems.length > 0 && { beforeSidebar: <SidebarLinks items={aboveItems} position="above" /> }), ...(belowItems.length > 0 && { afterSidebar: <SidebarLinks items={belowItems} position="below" /> }) } ) so you no longer mutate slots and keep the same behavior for SidebarLinks, aboveItems, and belowItems.docs/documentation-framework/recommended.md (1)
159-182: Add language specifier to fenced code block.The file structure code block is missing a language specifier. Use
textorplaintextfor directory trees.Proposed fix
-``` +```text docs/ ├── getting-started/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/documentation-framework/recommended.md` around lines 159 - 182, The fenced code block showing the docs directory tree is missing a language specifier; edit the block in recommended.md to start with ```text (or ```plaintext) instead of just ``` so the directory tree renders as plain text — update the existing fenced block that begins with the docs/ tree to use ```text.docs/documentation-framework/scaling.md (1)
14-21: Add language specifier to fenced code blocks for file structures.Multiple file structure code blocks (lines 14, 47, 128, 230) are missing language specifiers. Use
textorplaintextfor directory trees.Example fix for line 14
-``` +```text docs/ ├── getting-started.mdApply the same fix to lines 47, 128, and 230.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/documentation-framework/scaling.md` around lines 14 - 21, The fenced code blocks showing directory trees (the block starting with "docs/" and the tree lines like "├── getting-started.md" etc.) are missing a language specifier; update each of those fenced blocks (the ones beginning with the "docs/" tree at the instances referenced) to use a plaintext/text specifier by changing the opening fence from ``` to ```text (or ```plaintext) for the blocks containing the directory trees so the trees at the "docs/" snippet (and the other similar blocks at the other positions) are rendered correctly.packages/cli/src/commands/draft.ts (2)
11-16: Convert comment to JSDoc format for the exported command.The comment describes the function well but should use JSDoc format for consistency with the coding guidelines.
Proposed fix
-/** - * Scaffold a new documentation file from a template. - * - * Prompts for the doc type and title when not provided via args, - * then writes the rendered template to the specified output directory. - */ +/** + * Scaffold a new documentation file from a template. + * + * Prompts for the doc type and title when not provided via args, + * then writes the rendered template to the specified output directory. + * + * `@returns` A CLI command definition for the `draft` subcommand + */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/draft.ts` around lines 11 - 16, Replace the leading plain comment with a JSDoc block for the exported command in this module: convert the existing comment into /** ... */ JSDoc and include brief tags describing the command (e.g., `@description` "Scaffold a new documentation file from a template"), and add `@param` tags for the CLI arguments (doc type, title, output directory) and an `@returns` or `@async` tag if the exported command is async; apply this to the exported command in this file so the module-level comment follows the project JSDoc convention.
39-43: This error-handling pattern is consistent across all CLI commands in the project. While usingprocess.exit(1)deviates from the Result tuple guideline, it's the established convention throughout the codebase. If Result-based error handling is preferred, it should be refactored consistently across all command handlers and potentially within the@kidd-cli/corecommand abstraction itself.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/draft.ts` around lines 39 - 43, The current handler in draft.ts logs an error then calls process.exit(1) (at the registry.get(selectedType) failure path using ctx.logger.error and process.exit(1)), which conflicts with the Result-tuple guideline; replace the immediate process.exit call with returning a Result-style error from this command handler (i.e., return the error variant the command abstraction expects) and propagate that change through callers (or, if you intentionally keep the existing exit behavior, document and preserve it consistently across all CLI handlers and update `@kidd-cli/core` to reflect that convention); ensure the change targets the registry.get(selectedType) branch where ctx.logger.error is invoked.packages/ui/src/theme/components/sidebar/sidebar-links.tsx (2)
56-56: Add JSDoc to exportedSidebarLinks.The exported function is missing JSDoc, which this repo requires for public API surface.
📝 Suggested patch
+/** + * Render configurable sidebar links in either the above or below slot. + */ export function SidebarLinks(props: SidebarLinksProps): React.ReactElement | null {As per coding guidelines
Include JSDoc comments on exported functions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/theme/components/sidebar/sidebar-links.tsx` at line 56, Add a JSDoc block above the exported function SidebarLinks describing its purpose, the props parameter type SidebarLinksProps, and the return type (React.ReactElement | null); include tags like `@param` {SidebarLinksProps} props and `@returns` {React.ReactElement | null} and a short description of what the component renders so the public API has required documentation.
10-10:icon.coloris defined but never used.Line [10] allows
{ id, color }, but rendering only consumesid. Either wirecolorthrough to the icon renderer or remove it from this type to avoid misleading config.Also applies to: 18-26, 39-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/theme/components/sidebar/sidebar-links.tsx` at line 10, The declared prop type readonly icon?: string | { readonly id: string; readonly color: string } exposes icon.color but the render path only uses icon.id; update the implementation so the color is actually applied (e.g., pass icon.color into the icon renderer/Icon component or inline style/class where the icon is created inside SidebarLinks/SidebarLinkItem), or if color is not needed remove color from the union type to keep the type accurate; ensure the change touches the icon type declaration and the render usage where id is consumed (look for references to icon, id, and any Icon/RenderIcon call) so types and runtime usage stay consistent.packages/templates/src/registry.ts (1)
9-27: Use object parameters for multi-argument helpers.Line [9] and Line [22] use positional args with 2 parameters; this conflicts with the project TS style rule and makes call sites less self-descriptive.
♻️ Suggested refactor
-function resolveBody(base: string, override: ExtendTemplateOptions['body']): string { +function resolveBody(params: { + readonly base: string + readonly override: ExtendTemplateOptions['body'] +}): string { + const { base, override } = params if (override === undefined) { return base } if (isFunction(override)) { return override(base) } return override } @@ -function applyExtension(base: Template, options: ExtendTemplateOptions): Template { +function applyExtension(params: { + readonly base: Template + readonly options: ExtendTemplateOptions +}): Template { + const { base, options } = params return { type: base.type, label: options.label ?? base.label, hint: options.hint ?? base.hint, - body: resolveBody(base.body, options.body), + body: resolveBody({ base: base.body, override: options.body }), } } @@ - const extended = applyExtension(base, options) + const extended = applyExtension({ base, options })As per coding guidelines
Use object parameters for functions with 2+ parameters and include explicit return types.Also applies to: 47-47
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/templates/src/registry.ts` around lines 9 - 27, resolveBody and applyExtension use positional arguments and lack explicit return types; change resolveBody(base: string, override: ExtendTemplateOptions['body']) to resolveBody({ base, override }: { base: string; override: ExtendTemplateOptions['body'] }): string and change applyExtension(base: Template, options: ExtendTemplateOptions) to applyExtension({ base, options }: { base: Template; options: ExtendTemplateOptions }): Template, update their call sites to pass an object instead of positional args, and ensure you add the explicit return types (string and Template); also update any other multi-argument helper in this file to the same object-parameter + explicit return type pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/sync/resolve/index.ts`:
- Around line 333-371: The JSDoc/behavior mismatch in deriveCommonPrefix must be
fixed: change the early return check from links.length === 0 to links.length < 2
so the function returns undefined when fewer than two children have links (per
the doc), and replace the Array.from(...).reduce usage that mutates `common`
with a simple deterministic loop that compares segments left-to-right and
accumulates matching segments without side-effecting inside a reducer (e.g., a
for loop over index i up to `shortest` that breaks on first divergence and
pushes into `common`), keeping the final return `/${common.join('/')}` or
undefined when common is empty; reference: function deriveCommonPrefix and
variable names links, segmentArrays, shortest, common.
In `@packages/templates/test/registry.test.ts`:
- Line 18: The tests use optional chaining (e.g., template?.type) in multiple
assertions which violates OXLint; replace each use by explicitly narrowing the
value before accessing properties: assert or guard that the variable (for
example template) is defined (e.g., expect(template).toBeDefined() or if
(!template) throw) and then access template.type normally. Apply this pattern to
all flagged symbols/lines (all occurrences of template?.<prop> and similar
optional chains on the registry test variables) so property access is
non-optional and CI-compliant.
---
Outside diff comments:
In `@packages/config/schemas/schema.json`:
- Around line 616-634: The schema renamed nav item field from text to title;
update the config objects that define nav entries to use title instead of text —
locate the nav arrays in the root zpress.config.ts (the exported config object)
and in the examples/kitchen-sink zpress.config.ts (the kitchen-sink exported
config) and replace each occurrence of the property name "text:" with "title:"
for all nav item objects (preserve values and other fields like
link/items/activeMatch).
---
Nitpick comments:
In `@docs/documentation-framework/recommended.md`:
- Around line 159-182: The fenced code block showing the docs directory tree is
missing a language specifier; edit the block in recommended.md to start with
```text (or ```plaintext) instead of just ``` so the directory tree renders as
plain text — update the existing fenced block that begins with the docs/ tree to
use ```text.
In `@docs/documentation-framework/scaling.md`:
- Around line 14-21: The fenced code blocks showing directory trees (the block
starting with "docs/" and the tree lines like "├── getting-started.md" etc.) are
missing a language specifier; update each of those fenced blocks (the ones
beginning with the "docs/" tree at the instances referenced) to use a
plaintext/text specifier by changing the opening fence from ``` to ```text (or
```plaintext) for the blocks containing the directory trees so the trees at the
"docs/" snippet (and the other similar blocks at the other positions) are
rendered correctly.
In `@packages/cli/src/commands/draft.ts`:
- Around line 11-16: Replace the leading plain comment with a JSDoc block for
the exported command in this module: convert the existing comment into /** ...
*/ JSDoc and include brief tags describing the command (e.g., `@description`
"Scaffold a new documentation file from a template"), and add `@param` tags for
the CLI arguments (doc type, title, output directory) and an `@returns` or `@async`
tag if the exported command is async; apply this to the exported command in this
file so the module-level comment follows the project JSDoc convention.
- Around line 39-43: The current handler in draft.ts logs an error then calls
process.exit(1) (at the registry.get(selectedType) failure path using
ctx.logger.error and process.exit(1)), which conflicts with the Result-tuple
guideline; replace the immediate process.exit call with returning a Result-style
error from this command handler (i.e., return the error variant the command
abstraction expects) and propagate that change through callers (or, if you
intentionally keep the existing exit behavior, document and preserve it
consistently across all CLI handlers and update `@kidd-cli/core` to reflect that
convention); ensure the change targets the registry.get(selectedType) branch
where ctx.logger.error is invoked.
In `@packages/config/turbo.json`:
- Around line 9-13: The "build" target's inputs array currently lists ["src/**",
"rslib.config.ts", "package.json"] but omits the TypeScript configuration;
update the "build" target (the "build" object and its "inputs" property) to
include "tsconfig.json" so changes to compiler settings trigger a rebuild.
In `@packages/templates/src/registry.ts`:
- Around line 9-27: resolveBody and applyExtension use positional arguments and
lack explicit return types; change resolveBody(base: string, override:
ExtendTemplateOptions['body']) to resolveBody({ base, override }: { base:
string; override: ExtendTemplateOptions['body'] }): string and change
applyExtension(base: Template, options: ExtendTemplateOptions) to
applyExtension({ base, options }: { base: Template; options:
ExtendTemplateOptions }): Template, update their call sites to pass an object
instead of positional args, and ensure you add the explicit return types (string
and Template); also update any other multi-argument helper in this file to the
same object-parameter + explicit return type pattern.
In `@packages/templates/test/built-in.test.ts`:
- Line 6: Update the test to use a lowercase describe title and replace map
calls used for side-effects with forEach: change describe('BUILT_IN_TEMPLATES',
...) to a lowercase string (e.g., 'built_in_templates') to satisfy
prefer-describe-function-title, and in the test body where
BUILT_IN_TEMPLATES.map(...) is iterating only for side effects, replace that
.map(...) with .forEach(...) (referencing the BUILT_IN_TEMPLATES usage and the
anonymous callback currently passed to .map).
In `@packages/ui/src/config.ts`:
- Around line 100-111: The return type of resolveSidebarLinks is inconsistent
with ZpressSidebarLink because the nested icon object lacks readonly modifiers;
change the function's return signature (for the icon shape in the union) from {
id: string; color: string } to { readonly id: string; readonly color: string }
so the resolved type matches ZpressSidebarLink and avoids type widening (update
the signature on resolveSidebarLinks accordingly).
In `@packages/ui/src/theme/components/nav/layout.tsx`:
- Around line 81-90: The sidebarSlots object is built by mutating a local slots
variable; replace that imperative mutation with a declarative object expression
that conditionally includes properties using spreads or conditional property
syntax: construct sidebarSlots directly (e.g., const sidebarSlots = {
...(aboveItems.length > 0 && { beforeSidebar: <SidebarLinks items={aboveItems}
position="above" /> }), ...(belowItems.length > 0 && { afterSidebar:
<SidebarLinks items={belowItems} position="below" /> }) } ) so you no longer
mutate slots and keep the same behavior for SidebarLinks, aboveItems, and
belowItems.
In `@packages/ui/src/theme/components/sidebar/sidebar-links.css`:
- Around line 36-39: Add a keyboard focus style matching the hover state for the
sidebar links: update the stylesheet to add a .zp-sidebar-link:focus-visible
rule that sets the same color and background-color as .zp-sidebar-link:hover and
provides an accessible focus indicator (e.g., outline or focus ring using a
contrast-safe variable) while removing default browser outline if you provide a
visible replacement; target the .zp-sidebar-link selector and use :focus-visible
for keyboard-only focus.
In `@packages/ui/src/theme/components/sidebar/sidebar-links.tsx`:
- Line 56: Add a JSDoc block above the exported function SidebarLinks describing
its purpose, the props parameter type SidebarLinksProps, and the return type
(React.ReactElement | null); include tags like `@param` {SidebarLinksProps} props
and `@returns` {React.ReactElement | null} and a short description of what the
component renders so the public API has required documentation.
- Line 10: The declared prop type readonly icon?: string | { readonly id:
string; readonly color: string } exposes icon.color but the render path only
uses icon.id; update the implementation so the color is actually applied (e.g.,
pass icon.color into the icon renderer/Icon component or inline style/class
where the icon is created inside SidebarLinks/SidebarLinkItem), or if color is
not needed remove color from the union type to keep the type accurate; ensure
the change touches the icon type declaration and the render usage where id is
consumed (look for references to icon, id, and any Icon/RenderIcon call) so
types and runtime usage stay consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9de90737-e802-43ea-8d36-03cd2087ed24
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (77)
docs/concepts/auto-discovery.mddocs/concepts/deployment.mddocs/concepts/dynamic-content.mddocs/concepts/frontmatter.mddocs/concepts/landing-pages.mddocs/concepts/navigation.mddocs/concepts/sections-and-pages.mddocs/concepts/themes.mdxdocs/concepts/workspaces.mddocs/documentation-framework/overview.mddocs/documentation-framework/recommended.mddocs/documentation-framework/scaling.mddocs/documentation-framework/templates.mddocs/documentation-framework/types.mddocs/getting-started/introduction.mdxdocs/getting-started/quick-start.mddocs/references/frontmatter.mddocs/references/icons/colors.mdxdocs/references/icons/overview.mdxdocs/references/icons/technology/databases.mdxdocs/references/icons/technology/frameworks.mdxdocs/references/icons/technology/infrastructure.mdxdocs/references/icons/technology/integrations.mdxdocs/references/icons/technology/languages.mdxdocs/references/icons/technology/overview.mdxdocs/references/icons/technology/tooling.mdxexamples/kitchen-sink/zpress.config.tspackage.jsonpackages/cli/package.jsonpackages/cli/src/commands/draft.tspackages/cli/src/index.tspackages/config/README.mdpackages/config/package.jsonpackages/config/schemas/schema.jsonpackages/config/src/index.tspackages/config/src/schema.tspackages/config/src/types.tspackages/config/turbo.jsonpackages/core/src/define-config.tspackages/core/src/index.tspackages/core/src/sync/home.tspackages/core/src/sync/index.tspackages/core/src/sync/resolve/index.tspackages/core/src/sync/resolve/recursive.tspackages/core/src/sync/resolve/text.tspackages/core/src/sync/sidebar/inject.tspackages/core/src/sync/sidebar/multi.tspackages/core/src/sync/types.tspackages/core/src/sync/workspace.tspackages/core/src/types.tspackages/templates/package.jsonpackages/templates/rslib.config.tspackages/templates/src/built-in.tspackages/templates/src/define.tspackages/templates/src/index.tspackages/templates/src/registry.tspackages/templates/src/render.tspackages/templates/src/types.tspackages/templates/test/built-in.test.tspackages/templates/test/define.test.tspackages/templates/test/registry.test.tspackages/templates/test/render.test.tspackages/templates/tsconfig.jsonpackages/templates/vitest.config.tspackages/ui/src/config.tspackages/ui/src/theme/components/home/feature-card.csspackages/ui/src/theme/components/nav/layout.tsxpackages/ui/src/theme/components/shared/section-card.tsxpackages/ui/src/theme/components/sidebar/sidebar-links.csspackages/ui/src/theme/components/sidebar/sidebar-links.tsxpackages/ui/src/theme/hooks/use-zpress.tspackages/ui/src/theme/styles/overrides/home.csspackages/ui/src/theme/styles/overrides/section-card.csspackages/zpress/src/config.tspackages/zpress/src/index.tsturbo.jsonzpress.config.ts
💤 Files with no reviewable changes (2)
- packages/config/README.md
- packages/core/src/sync/workspace.ts
…geset - Fix JSDoc in deriveCommonPrefix to match implementation - Remove all optional chaining from registry tests (OXLint violation) - Add changeset for core/ui minor and config patch Co-Authored-By: Claude <noreply@anthropic.com>
- Remove array mutation in deriveCommonPrefix reduce (use immutable accumulator) - Use match expressions instead of nested if chains in resolveSectionTitle - Replace ternary in draft command with match expressions - Simplify defineTemplate to use spread - Simplify createRegistry to use Object.entries directly - Build sidebar slots immutably with spread instead of post-creation mutation - Use match for icon resolution and null guards in sidebar-links Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/sync/resolve/text.ts`:
- Line 56: The .otherwise(() => 'Section') fallback in resolve/text.ts
reintroduces the reserved name "Section"; change the fallback to a non-reserved
value or omit generating the label here (e.g., return undefined/null or an empty
string) so consumers can supply a proper title later; update the .otherwise(...)
call (the chain that currently returns 'Section') to return a safe sentinel (or
nothing) and ensure the caller that consumes this resolver handles the
undefined/null case appropriately.
- Around line 51-53: The lastSegment extraction should avoid using
filter(...).pop() and returning explicit undefined; replace the current matcher
block so the P.string branch returns the last non-empty path segment via
Array.prototype.findLast (e.g. .with(P.string, p =>
p.split('/').findLast(Boolean))) and keep the .otherwise branch to return
undefined only if prefix isn't a string; update the code to use lastSegment from
that matcher so it no longer triggers prefer-array-find or no-useless-undefined
lint rules.
In `@packages/templates/test/registry.test.ts`:
- Around line 13-23: The test uses TEMPLATE_TYPES.map for side-effect assertions
which triggers the useIterableCallbackReturn lint rule; change the iteration to
TEMPLATE_TYPES.forEach in the test 'should return correct template for each
type' so you iterate for side effects only, call createRegistry(), then for each
type call registry.get(type) and run the expect assertions
(expect(template).toBeDefined() and expect(template.type).toBe(type)) while
preserving the existing early-return guard when template is undefined.
- Around line 30-34: Replace direct equality boolean assertions using toBe(true)
and toBe(false) with the truthy/falsy matchers toBeTruthy() and toBeFalsy();
specifically, update test assertions that call registry.has(...) (and any other
boolean-returning calls in the same file, e.g., results of createRegistry()
lookups) so expect(registry.has('...')).toBe(true) becomes
expect(registry.has('...')).toBeTruthy() and expect(...).toBe(false) becomes
expect(...).toBeFalsy(), ensuring all occurrences in the registry.test.ts file
are changed to satisfy the vitest/prefer-to-be-truthy and
vitest/prefer-to-be-falsy rules.
In `@packages/ui/src/theme/components/nav/layout.tsx`:
- Around line 82-87: The construction using ternaries when building the sidebar
slots causes an eslint no-ternary error; replace the ternary expressions with
explicit conditional assignment: create the slots object (or the props object
used in layout rendering) then, if aboveItems.length > 0 assign
slots.beforeSidebar = <SidebarLinks items={aboveItems} position="above" /> and
if belowItems.length > 0 assign slots.afterSidebar = <SidebarLinks
items={belowItems} position="below" />; update the code paths using
beforeSidebar/afterSidebar to use this slots object instead of the
ternary-spread pattern so no ternary operator is used.
In `@packages/ui/src/theme/components/sidebar/sidebar-links.tsx`:
- Line 44: The nav element in the SidebarLinks component renders an unlabeled
navigation landmark; update the JSX in the SidebarLinks (sidebar-links.tsx) so
the <nav> includes an aria-label that distinguishes the two instances (use
props.position to generate the label, e.g. "Sidebar links above" vs "Sidebar
links below" or a similar clear string) to ensure screen readers can
disambiguate the navigation regions.
- Line 41: Add a JSDoc block above the exported function SidebarLinks describing
its purpose, input props and return type: document the SidebarLinksProps
parameter (mentioning the shape/important fields if applicable) and that it
returns a React.ReactElement or null; include tags like `@param`
{SidebarLinksProps} props and `@returns` {React.ReactElement | null} and mark it
as exported/public according to project conventions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a5a4dd9f-ccfd-4ba6-8700-49cd8b8b6882
📒 Files selected for processing (9)
.changeset/feat-section-landing-pages.mdpackages/cli/src/commands/draft.tspackages/core/src/sync/resolve/index.tspackages/core/src/sync/resolve/text.tspackages/templates/src/define.tspackages/templates/src/registry.tspackages/templates/test/registry.test.tspackages/ui/src/theme/components/nav/layout.tsxpackages/ui/src/theme/components/sidebar/sidebar-links.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/cli/src/commands/draft.ts
- packages/templates/src/registry.ts
- packages/core/src/sync/resolve/index.ts
- packages/templates/src/define.ts
- Replace filter(Boolean).pop() with findLast(Boolean) in resolveSectionTitle - Remove ternaries in layout.tsx sidebar slots, use match expressions - Replace toBe(true/false) with toBeTruthy/toBeFalsy in registry tests - Fix map callback returns and remove forEach usage in test files - Replace Array.from() with spread operator in registry - Add JSDoc and aria-label to SidebarLinks component - Remove useless undefined returns Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/templates/test/built-in.test.ts (1)
19-24: Consider removing unnecessary array wrapper.The array literal wrapping the two
expectcalls is unused. This works but adds visual noise without benefit.♻️ Optional simplification
it('should have non-empty label and hint on every template', () => { - Object.values(BUILT_IN_TEMPLATES).map((template) => [ - expect(template.label.length).toBeGreaterThan(0), - expect(template.hint.length).toBeGreaterThan(0), - ]) + Object.values(BUILT_IN_TEMPLATES).map((template) => { + expect(template.label.length).toBeGreaterThan(0) + return expect(template.hint.length).toBeGreaterThan(0) + }) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/templates/test/built-in.test.ts` around lines 19 - 24, The test iterates BUILT_IN_TEMPLATES but wraps the two expect() calls in an unnecessary array literal which creates visual noise and leaves map unused; change the iteration in the "should have non-empty label and hint on every template" test to use Object.values(BUILT_IN_TEMPLATES).forEach and call expect(template.label.length).toBeGreaterThan(0) and expect(template.hint.length).toBeGreaterThan(0) directly (remove the surrounding [ ... ] array wrapper).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/commands/draft.ts`:
- Around line 47-50: Replace the hard process exit with a Result-tuple style
error return: instead of calling process.exit(1) in the command handler when
template is falsy, log the error with ctx.logger.error(`Unknown template type:
${selectedType}`) and return an Err-result (e.g., a tuple like [null, new
Error(`Unknown template type: ${selectedType}`)] or your codebase's Result type)
from the enclosing command function; remove the process.exit(1) so the caller
can handle the error and compose/test flows (use the existing template,
selectedType and ctx.logger symbols to locate the block).
- Around line 58-67: The current prompt validator only checks title non-empty
but not whether the slug will be usable; update the validate function used when
prompting for title to compute the slug (call toSlug(title.trim()) or similar)
and return an error if that slug is empty or only punctuation so users can't
submit titles like "!!!"; reference the validate callback and the toSlug/title
variables (and ensure toSlug is imported/in-scope) and keep the existing "Title
is required" style message or replace it with something like "Please provide a
title that generates a valid filename".
- Around line 71-72: Before calling fs.writeFile, guard against clobbering by
checking whether filePath already exists (e.g., using fs.access or fs.stat) and
fail fast or prompt instead of overwriting; update the draft command flow around
the fs.mkdir(...) and await fs.writeFile(filePath, content, 'utf8') to first
verify non-existence of filePath and throw an explicit error (or call the
existing prompt flow) if it exists, otherwise proceed to create directories and
write the file.
---
Nitpick comments:
In `@packages/templates/test/built-in.test.ts`:
- Around line 19-24: The test iterates BUILT_IN_TEMPLATES but wraps the two
expect() calls in an unnecessary array literal which creates visual noise and
leaves map unused; change the iteration in the "should have non-empty label and
hint on every template" test to use Object.values(BUILT_IN_TEMPLATES).forEach
and call expect(template.label.length).toBeGreaterThan(0) and
expect(template.hint.length).toBeGreaterThan(0) directly (remove the surrounding
[ ... ] array wrapper).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 654e90b9-44ce-4721-b7f1-a14d0e277dcd
📒 Files selected for processing (7)
packages/cli/src/commands/draft.tspackages/core/src/sync/resolve/text.tspackages/templates/src/registry.tspackages/templates/test/built-in.test.tspackages/templates/test/registry.test.tspackages/ui/src/theme/components/nav/layout.tsxpackages/ui/src/theme/components/sidebar/sidebar-links.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/ui/src/theme/components/sidebar/sidebar-links.tsx
- packages/templates/src/registry.ts
- packages/core/src/sync/resolve/text.ts
…PI support Co-Authored-By: Claude <noreply@anthropic.com>
This was referenced Mar 18, 2026
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.
Summary
&&chains from package.json scripts — replaced with turbo task dependencies and pnpm lifecycle hooksturbo.jsonfor@zpress/configschema generation dependencyChanges
Core (
packages/core)autoLinkflag toResolvedEntryto distinguish explicit vs auto-derived linksderiveCommonPrefix()to auto-derive section links from children's common path prefixresolveSectionTitle()shared helper to derive group names from prefix segmentspromoteOverviewChildto skip entries with auto-derived links (preserves explicitly configured pages)UI (
packages/ui)SectionCard: icon + title inline on one row, optional description on a second rowConfig (
packages/config)buildscript: schema generation is now a separate turbo task (generate:schema)turbo.jsonwithbuilddepending ongenerate:schemaRoot
&&script chains with turbo tasks and pnpm lifecycle hooks (predev,postversion,prerelease)//#lint,//#format,//#docs:generate,generate:schemaTest plan
pnpm devstarts correctly (predev runs build, then turbo dev)pnpm checkruns typecheck, lint, and format via turbo/documentation-framework/overviewpage renderspnpm buildworks with config schema generation via turboSummary by CodeRabbit
New Features
Documentation
Configuration
Tests
Chores