Uh oh!
There was an error while loading. Please reload this page.
feat(swingset): group sidebar entries under section labels - #8815
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 4fad9dc The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types 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 |
📝 WalkthroughWalkthroughThe PR refactors the sidebar component to organize component stories into hierarchical groups. Each group now gets a dedicated section container with a header label, and stories within that group render as collapsible sections. Link routing and active-state logic remain unchanged. ChangesSidebar grouping structure
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/swingset/src/components/app-sidebar.tsx`:
- Around line 70-72: The group label is rendered as a non-semantic div, hurting
screen-reader navigation; update the element that renders the section label in
the AppSidebar component (the block using {group}) to use a proper heading
element (e.g., h2/h3) with the existing classes or, if heading level constraints
prevent changing markup, add role="heading" and aria-level="3" to the element so
assistive tech recognizes it as a section heading while keeping the same styling
and variable {group}.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: cf28e533-e20a-45ce-bf02-e5837916f235
📒 Files selected for processing (2)
.changeset/two-level-sidebar-sorting.mdpackages/swingset/src/components/app-sidebar.tsx
| <div className='text-sidebar-foreground/50 px-4 pb-0 pt-4 text-[10px] font-semibold uppercase tracking-wider'> | ||
| {group} | ||
| </div> |
There was a problem hiding this comment.
Use semantic heading element for section labels.
The section group headers are rendered as styled divs without semantic meaning. Screen readers won't recognize these as section headings, degrading navigation for keyboard and assistive technology users.
♿ Proposed fix using semantic heading
- <div className='text-sidebar-foreground/50 px-4 pb-0 pt-4 text-[10px] font-semibold uppercase tracking-wider'>+ <h2 className='text-sidebar-foreground/50 px-4 pb-0 pt-4 text-[10px] font-semibold uppercase tracking-wider'>
{group}
- </div>+ </h2>Alternatively, if heading levels are a concern, use ARIA:
- <div className='text-sidebar-foreground/50 px-4 pb-0 pt-4 text-[10px] font-semibold uppercase tracking-wider'>+ <div role='heading' aria-level='2' className='text-sidebar-foreground/50 px-4 pb-0 pt-4 text-[10px] font-semibold uppercase tracking-wider'>
{group}
</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <divclassName='text-sidebar-foreground/50 px-4 pb-0 pt-4 text-[10px] font-semibold uppercase tracking-wider'> | |
| {group} | |
| </div> | |
| <h2className='text-sidebar-foreground/50 px-4 pb-0 pt-4 text-[10px] font-semibold uppercase tracking-wider'> | |
| {group} | |
| </h2> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/swingset/src/components/app-sidebar.tsx` around lines 70 - 72, The
group label is rendered as a non-semantic div, hurting screen-reader navigation;
update the element that renders the section label in the AppSidebar component
(the block using {group}) to use a proper heading element (e.g., h2/h3) with the
existing classes or, if heading level constraints prevent changing markup, add
role="heading" and aria-level="3" to the element so assistive tech recognizes it
as a section heading while keeping the same styling and variable {group}.
@clerk/astro@clerk/backend@clerk/chrome-extension@clerk/clerk-js@clerk/expo@clerk/expo-passkeys@clerk/express@clerk/fastify@clerk/hono@clerk/localizations@clerk/nextjs@clerk/nuxt@clerk/react@clerk/react-router@clerk/shared@clerk/tanstack-react-start@clerk/testing@clerk/ui@clerk/upgrade@clerk/vuecommit: |
Description
Surfaces the swingset sidebar's
groupmetadata (Components, Primitives) as section labels above their respective component entries, instead of rendering every component in one flat list. Each section header is a small uppercase muted label, with the component collapsibles grouped beneath it and spacing tightened so entries read as connected to their label. This is a UI-only change to the private@clerk/swingsetexplorer; runpnpm run dev:swingsetto view.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit