feat(headless): add Accordion primitive#8475
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: f13cdb6 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 |
6e1dfb8 to
2a5b8e2
Compare
9f473a8 to
5b5ddfa
Compare
2a5b8e2 to
16b5d39
Compare
5b5ddfa to
47cce7a
Compare
47cce7a to
78ffbf1
Compare
78ffbf1 to
22dc58f
Compare
54016b4 to
4430af5
Compare
483637a to
a7cbc25
Compare
a7cbc25 to
2379a11
Compare
2379a11 to
8b06f02
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a complete Accordion primitive to headless: context plumbing, Root state orchestration, Item/Header/Trigger/Panel components with ARIA/animation, export/build wiring, README, and comprehensive tests. ChangesAccordion Primitive Implementation
Sequence Diagram(s)sequenceDiagram
participant User
participant AccordionTrigger
participant AccordionRoot
participant AccordionContext
participant AccordionPanel
User->>AccordionTrigger: click toggle(itemValue)
AccordionTrigger->>AccordionRoot: invoke toggle(itemValue)
AccordionRoot->>AccordionContext: update open value / call onValueChange
AccordionContext->>AccordionPanel: expose open/ids to render
AccordionPanel->>AccordionPanel: measure height (ResizeObserver)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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. 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 |
@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/vue
commit: |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
packages/headless/src/primitives/accordion/accordion-panel.tsx (1)
8-8: ⚡ Quick winAdd JSDoc documentation for public interface.
This public interface needs JSDoc documentation for consumers and potentially generated reference docs.
📝 Suggested JSDoc
+/** + * Props for the AccordionPanel component. + * Extends all standard HTML div attributes. + */ export interface AccordionPanelProps extends ComponentProps<'div'> {}As per coding guidelines, all public APIs in packages must be documented with JSDoc.
🤖 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/headless/src/primitives/accordion/accordion-panel.tsx` at line 8, Add a JSDoc block for the public interface AccordionPanelProps: document the purpose of the interface (props for an Accordion panel component), note that it extends ComponentProps<'div'>, describe expected/important behaviors or keys consumers might rely on (e.g., children, className, id), mark it as public API, and include a short usage example and `@extends` tag referencing ComponentProps<'div'>; update the AccordionPanelProps declaration accordingly in accordion-panel.tsx.Source: Coding guidelines
packages/headless/src/primitives/accordion/accordion-trigger.tsx (1)
8-8: ⚡ Quick winAdd JSDoc documentation for public interface.
This public interface is part of the accordion primitive's API surface and needs JSDoc documentation for consumers and potentially generated reference docs.
📝 Suggested JSDoc
+/** + * Props for the AccordionTrigger component. + * Extends all standard HTML button attributes. + */ export interface AccordionTriggerProps extends ComponentProps<'button'> {}As per coding guidelines, all public APIs in packages must be documented with JSDoc, and public/reference-facing APIs should be treated as customer-facing documentation.
🤖 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/headless/src/primitives/accordion/accordion-trigger.tsx` at line 8, Add JSDoc to the exported AccordionTriggerProps public interface: document what this prop type represents, its relationship to ComponentProps<'button'>, typical consumers, and any important notes (e.g., forwarded props or accessibility considerations). Place the JSDoc immediately above the declaration of AccordionTriggerProps so generated docs pick it up and ensure the description is concise and mentions it extends ComponentProps<'button'> and is intended for the Accordion trigger button element.Source: Coding guidelines
🤖 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/headless/src/primitives/accordion/accordion-context.ts`:
- Around line 12-18: The exported hook functions lack explicit return types;
update the function signatures for useAccordionContext and
useAccordionItemContext to include their respective return annotations
(useAccordionContext: AccordionContextValue and useAccordionItemContext:
AccordionItemContextValue), ensuring the functions still perform the runtime
null-check and throw the same Error when ctx is missing; adjust any imports or
type references if needed so AccordionContextValue and AccordionItemContextValue
are available to the module.
In `@packages/headless/src/primitives/accordion/accordion-header.tsx`:
- Line 7: The exported component AccordionHeader lacks an explicit return type;
update the function signature for AccordionHeader to include an explicit return
annotation of React.JSX.Element | null (since renderElement can return null when
enabled is false) so the public API has a declared return type; locate the
AccordionHeader function declaration and add the type annotation to its return
type accordingly.
In `@packages/headless/src/primitives/accordion/accordion-item.tsx`:
- Line 14: The exported component function AccordionItem currently lacks an
explicit return type; update its signature to include a component return type
(for example append ": JSX.Element" or ": React.ReactElement") so it becomes
"export function AccordionItem(props: AccordionItemProps): JSX.Element" (or
React.ReactElement) and ensure any necessary imports/types are available (e.g.,
import React types if your TSX config requires them).
In `@packages/headless/src/primitives/accordion/accordion-panel.tsx`:
- Around line 10-82: The public function AccordionPanel lacks an explicit return
type and JSDoc; update the function signature to include an explicit React
return type (e.g., React.ReactElement | null) on the exported
AccordionPanel(props: AccordionPanelProps) declaration and add a JSDoc block
above it that documents the component, its props (`@param` props
AccordionPanelProps), the return value (`@returns` React.ReactElement | null) and
a short `@example` showing usage (e.g., <Accordion.Panel>...</Accordion.Panel>);
keep existing logic (useAccordionItemContext, useTransition, panelRef,
measure/ResizeObserver, renderElement, mergeProps) untouched and only modify the
signature and add the JSDoc comment.
In `@packages/headless/src/primitives/accordion/accordion-trigger.tsx`:
- Around line 10-54: The public component AccordionTrigger currently lacks an
explicit return type and JSDoc; update the function signature to include an
explicit React.ReactElement return type (export function AccordionTrigger(props:
AccordionTriggerProps): React.ReactElement) and add a JSDoc block above it
documenting the component, its props (`@param` props - AccordionTriggerProps), the
return value (`@returns` React.ReactElement), and a short `@example` showing usage
of <Accordion.Trigger> with children; keep references to the relevant symbols in
the doc (AccordionTrigger, AccordionTriggerProps) and do not change behavior of
useAccordionContext, useAccordionItemContext, CompositeItem, mergeProps, or
renderElement.
---
Nitpick comments:
In `@packages/headless/src/primitives/accordion/accordion-panel.tsx`:
- Line 8: Add a JSDoc block for the public interface AccordionPanelProps:
document the purpose of the interface (props for an Accordion panel component),
note that it extends ComponentProps<'div'>, describe expected/important
behaviors or keys consumers might rely on (e.g., children, className, id), mark
it as public API, and include a short usage example and `@extends` tag referencing
ComponentProps<'div'>; update the AccordionPanelProps declaration accordingly in
accordion-panel.tsx.
In `@packages/headless/src/primitives/accordion/accordion-trigger.tsx`:
- Line 8: Add JSDoc to the exported AccordionTriggerProps public interface:
document what this prop type represents, its relationship to
ComponentProps<'button'>, typical consumers, and any important notes (e.g.,
forwarded props or accessibility considerations). Place the JSDoc immediately
above the declaration of AccordionTriggerProps so generated docs pick it up and
ensure the description is concise and mentions it extends
ComponentProps<'button'> and is intended for the Accordion trigger button
element.
🪄 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: c18d7ed6-2b5d-4624-84f3-d4e272334791
📒 Files selected for processing (12)
packages/headless/package.jsonpackages/headless/src/primitives/accordion/README.mdpackages/headless/src/primitives/accordion/accordion-context.tspackages/headless/src/primitives/accordion/accordion-header.tsxpackages/headless/src/primitives/accordion/accordion-item.tsxpackages/headless/src/primitives/accordion/accordion-panel.tsxpackages/headless/src/primitives/accordion/accordion-root.tsxpackages/headless/src/primitives/accordion/accordion-trigger.tsxpackages/headless/src/primitives/accordion/accordion.test.tsxpackages/headless/src/primitives/accordion/index.tspackages/headless/src/primitives/accordion/parts.tspackages/headless/vite.config.ts
Apply Frederick's Dialog review feedback to Accordion: - Merge consumer ref with the internal panel ref via useMergeRefs so a consumer-supplied ref no longer clobbers the ref used for height measurement (matches Dialog.Popup). - Force the derived trigger/panel ids to win over consumer-supplied ids so the trigger/panel aria pairing can't be silently broken. - Resolve lint errors (empty interfaces, non-null assertions, curly, imports).
Summary
--cl-accordion-panel-heightCSS custom propertyCompositeReview Stack
Test plan
pnpm buildsucceedspnpm testpasses (+35 accordion tests)🤖 Generated with Claude Code
Summary by CodeRabbit