-
Notifications
You must be signed in to change notification settings - Fork 191
Proposal: Shadcn registry integration and example for accordion-faq #414
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughA new FAQ Accordion React component using Radix UI and Framer Motion was introduced and registered in a new JSON registry. Supporting changes include a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ComponentPreview
participant OpenInV0
User->>ComponentPreview: Render (with name prop)
ComponentPreview->>OpenInV0: Render (name prop)
OpenInV0-->>ComponentPreview: Returns anchor (if name == "accordion-faq")
ComponentPreview->>User: Renders OpenInV0 + iframe
sequenceDiagram
participant User
participant FaqSection
User->>FaqSection: Render with FAQ data
FaqSection->>FaqSection: Track open/closed state
FaqSection->>User: Display accordion items
User->>FaqSection: Click question
FaqSection->>FaqSection: Animate expand/collapse
FaqSection->>User: Show answer with animation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
components/open-in-v0.tsx (1)
17-26: Consider extracting SVG icon to improve maintainability.The inline SVG icon is quite large and makes the component harder to read. Consider extracting it to a separate icon component or constant.
+const V0Icon = () => ( + <svg viewBox="0 0 40 20" fill="none" xmlns="http://www.w3.org/2000/svg" className="size-5"> + <path + d="M23.3919 0H32.9188C36.7819 0 39.9136 3.13165 39.9136 6.99475V16.0805H36.0006V6.99475C36.0006 6.90167 35.9969 6.80925 35.9898 6.71766L26.4628 16.079C26.4949 16.08 26.5272 16.0805 26.5595 16.0805H36.0006V19.7762H26.5595C22.6964 19.7762 19.4788 16.6139 19.4788 12.7508V3.68923H23.3919V12.7508C23.3919 12.9253 23.4054 13.0977 23.4316 13.2668L33.1682 3.6995C33.0861 3.6927 33.003 3.68923 32.9188 3.68923H23.3919V0Z" + fill="currentColor" + /> + <path + d="M13.7688 19.0956L0 3.68759H5.53933L13.6231 12.7337V3.68759H17.7535V17.5746C17.7535 19.6705 15.1654 20.6584 13.7688 19.0956Z" + fill="currentColor" + /> + </svg> +); <span>Open in</span> - <svg viewBox="0 0 40 20" fill="none" xmlns="http://www.w3.org/2000/svg" className="size-5"> - <path - d="M23.3919 0H32.9188C36.7819 0 39.9136 3.13165 39.9136 6.99475V16.0805H36.0006V6.99475C36.0006 6.90167 35.9969 6.80925 35.9898 6.71766L26.4628 16.079C26.4949 16.08 26.5272 16.0805 26.5595 16.0805H36.0006V19.7762H26.5595C22.6964 19.7762 19.4788 16.6139 19.4788 12.7508V3.68923H23.3919V12.7508C23.3919 12.9253 23.4054 13.0977 23.4316 13.2668L33.1682 3.6995C33.0861 3.6927 33.003 3.68923 32.9188 3.68923H23.3919V0Z" - fill="currentColor" - /> - <path - d="M13.7688 19.0956L0 3.68759H5.53933L13.6231 12.7337V3.68759H17.7535V17.5746C17.7535 19.6705 15.1654 20.6584 13.7688 19.0956Z" - fill="currentColor" - /> - </svg> + <V0Icon />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
animata/container/animated-dock.stories.tsx(1 hunks)animata/container/animated-dock.tsx(1 hunks)components/component-preview.tsx(2 hunks)components/open-in-v0.tsx(1 hunks)package.json(1 hunks)public/r/accordion-faq.json(1 hunks)registry.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
components/component-preview.tsx (1)
components/open-in-v0.tsx (1)
OpenInV0(1-29)
🔇 Additional comments (9)
package.json (1)
12-12: LGTM! Script supports registry build process.The new
shadcn:buildscript appropriately cleans the registry output directory before rebuilding, which aligns with the Shadcn registry integration objectives.animata/container/animated-dock.tsx (1)
1-14: LGTM! Import organization improves code structure.The import reordering follows good practices by grouping React/Next.js imports first, maintaining logical grouping within library imports, and placing utilities last. No functional changes were made.
animata/container/animated-dock.stories.tsx (1)
1-4: LGTM! Import reordering maintains consistency.The import organization matches the changes made to the main animated-dock.tsx file, improving consistency across related files. No functional changes to the stories.
components/component-preview.tsx (2)
8-8: LGTM! OpenInV0 import supports new v0.dev integration.The import statement correctly adds the new OpenInV0 utility component for Shadcn registry integration.
93-93: Suffix removal logic verified
- Only one occurrence of
name.replaceAll("--docs", "")exists incomponents/component-preview.tsx:93.- This will strip every
--docssubstring; with the current naming convention (appending--docsonly as a suffix), it behaves as intended.- If you anticipate component names containing
--docselsewhere (not just at the end), you may opt for a stricter pattern, e.g.:to target only the trailingname.replace(/--docs$/, "")--docs.registry.json (1)
1-21: Registry Definition Verified and ApprovedAll required dependencies and files are present and correctly configured:
- “@radix-ui/react-accordion” and “framer-motion” entries found in package.json.
- Source component file at animata/accordion/faq.tsx exists.
- Public registry file at public/r/accordion-faq.json is generated.
No further changes needed.
components/open-in-v0.tsx (1)
10-14: LGTM: Proper security attributes and styling.The external link implementation correctly uses
target="_blank"withrel="noopener noreferrer"for security. The positioning and styling classes are appropriate for the UI placement.public/r/accordion-faq.json (2)
1-7: LGTM: Proper registry schema and metadata.The JSON structure correctly follows the Shadcn registry schema with appropriate metadata, component type, and dependencies specified.
12-14: LGTM: Correct file mapping configuration.The file mapping correctly specifies the component type and target path for the Shadcn registry installation.
| if (name !== "accordion-faq") { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove hardcoded component filtering to improve reusability.
The hardcoded condition limiting functionality to only "accordion-faq" makes this component non-reusable and violates the DRY principle. This will require code changes for every new component added to the registry.
Consider one of these approaches:
Option 1: Remove the condition entirely (if all components should have this feature):
- if (name !== "accordion-faq") {
- return null;
- }Option 2: Make it configurable via props:
-export function OpenInV0({ name }: { name: string }) {
+export function OpenInV0({ name, enabled = true }: { name: string; enabled?: boolean }) {
const registryUrl = `https://animata.cban.top/r/${name}.json`;
const v0Url = `https://v0.dev/chat/api/open?url=${encodeURIComponent(registryUrl)}`;
- if (name !== "accordion-faq") {
+ if (!enabled) {
return null;
}Option 3: Accept a list of supported components:
-export function OpenInV0({ name }: { name: string }) {
+export function OpenInV0({ name, supportedComponents = [] }: { name: string; supportedComponents?: string[] }) {
const registryUrl = `https://animata.cban.top/r/${name}.json`;
const v0Url = `https://v0.dev/chat/api/open?url=${encodeURIComponent(registryUrl)}`;
- if (name !== "accordion-faq") {
+ if (supportedComponents.length > 0 && !supportedComponents.includes(name)) {
return null;
}📝 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.
| if (name !== "accordion-faq") { | |
| return null; | |
| } | |
| export function OpenInV0({ name }: { name: string }) { | |
| const registryUrl = `https://animata.cban.top/r/${name}.json`; | |
| const v0Url = `https://v0.dev/chat/api/open?url=${encodeURIComponent(registryUrl)}`; | |
| return ( | |
| <a href={v0Url} target="_blank" rel="noopener noreferrer"> | |
| {/* SVG icon */} | |
| </a> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In components/open-in-v0.tsx around lines 5 to 7, remove the hardcoded check for
"accordion-faq" to avoid limiting the component's reusability. Instead, either
eliminate the condition entirely if the feature should apply to all components,
or modify the component to accept a prop that specifies which component names
are supported, allowing dynamic filtering based on that prop. This change will
make the component more flexible and maintainable.
| "files": [ | ||
| { | ||
| "path": "animata/accordion/faq.tsx", | ||
| "content": "\"use client\";\n\nimport React, { useState } from \"react\";\nimport { motion } from \"framer-motion\";\n\nimport * as Accordion from \"@radix-ui/react-accordion\";\n\ninterface FAQItem {\n id: number;\n question: string;\n answer: string;\n icon?: string;\n iconPosition?: string;\n}\n\ninterface FaqSectionProps {\n data: FAQItem[];\n}\n\nexport default function FaqSection({ data }: FaqSectionProps) {\n const [openItem, setOpenItem] = useState<string | null>(null);\n\n return (\n <div\n className=\"mx-auto max-w-md rounded-lg bg-white p-4\"\n style={{ maxWidth: \"700px\", minWidth: \"700px\" }}\n >\n <div className=\"mb-4 text-sm text-gray-500\">Every day, 9:01 AM</div>\n\n <Accordion.Root\n type=\"single\"\n collapsible\n value={openItem || \"\"}\n onValueChange={(value) => setOpenItem(value)}\n >\n {data.map((item) => (\n <Accordion.Item value={item.id.toString()} key={item.id} className=\"mb-2\">\n <Accordion.Header>\n <Accordion.Trigger\n className=\"flex w-full items-center justify-start gap-x-4\"\n style={{ width: \"100%\" }}\n >\n <div\n className=\"relative flex items-center space-x-2 rounded-xl bg-gray-100 p-2 hover:bg-[#E0F7FA]\"\n style={{\n backgroundColor: openItem === item.id.toString() ? \"#E0F7FA\" : \"\",\n }}\n >\n {item.icon && (\n <span\n className={`absolute bottom-6 ${\n item.iconPosition === \"right\" ? \"right-0\" : \"left-0\"\n }`}\n style={{\n transform: item.iconPosition === \"right\" ? \"rotate(7deg)\" : \"rotate(-4deg)\",\n }}\n >\n {item.icon}\n </span>\n )}\n <span className=\"font-medium text-gray-700\">{item.question}</span>\n </div>\n\n <span className=\"cursor-pointer text-lg font-bold text-gray-400\">\n {openItem === item.id.toString() ? (\n <svg\n xmlns=\"http://www.w3.org/2000/svg\"\n viewBox=\"0 0 24 24\"\n fill=\"#7CB9E8\"\n className=\"size-6\"\n >\n <path\n fillRule=\"evenodd\"\n d=\"M12 2.25c-5.385 0-9.75 4.365-9.75 9.75s4.365 9.75 9.75 9.75 9.75-4.365 9.75-9.75S17.385 2.25 12 2.25Zm3 10.5a.75.75 0 0 0 0-1.5H9a.75.75 0 0 0 0 1.5h6Z\"\n clipRule=\"evenodd\"\n />\n </svg>\n ) : (\n <svg\n xmlns=\"http://www.w3.org/2000/svg\"\n viewBox=\"0 0 24 24\"\n fill=\"currentColor\"\n className=\"size-6\"\n >\n <path\n fillRule=\"evenodd\"\n d=\"M12 2.25c-5.385 0-9.75 4.365-9.75 9.75s4.365 9.75 9.75 9.75 9.75-4.365 9.75-9.75S17.385 2.25 12 2.25ZM12.75 9a.75.75 0 0 0-1.5 0v2.25H9a.75.75 0 0 0 0 1.5h2.25V15a.75.75 0 0 0 1.5 0v-2.25H15a.75.75 0 0 0 0-1.5h-2.25V9Z\"\n clipRule=\"evenodd\"\n />\n </svg>\n )}\n </span>\n </Accordion.Trigger>\n </Accordion.Header>\n <Accordion.Content asChild forceMount style={{ display: \"block\" }}>\n <motion.div\n initial=\"collapsed\"\n animate={openItem === item.id.toString() ? \"open\" : \"collapsed\"}\n variants={{\n open: { opacity: 1, height: \"auto\" },\n collapsed: { opacity: 0, height: 0 },\n }}\n transition={{ duration: 0.4 }}\n style={{ width: \"100%\", overflow: \"hidden\" }}\n >\n <div\n className=\"ml-7 mt-1 rounded-lg p-3 text-white md:ml-16\"\n style={{\n borderRadius: \"12px\",\n textAlign: \"left\",\n width: \"100%\",\n }}\n >\n <div className=\"relative max-w-xs rounded-2xl bg-blue-500 px-4 py-2 text-white\">\n {item.answer}\n <div className=\"absolute bottom-0 right-0 h-0 w-0 border-l-[10px] border-t-[10px] border-l-transparent border-t-blue-500\"></div>\n </div>\n </div>\n </motion.div>\n </Accordion.Content>\n </Accordion.Item>\n ))}\n </Accordion.Root>\n </div>\n );\n}\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Mix of inline styles and Tailwind classes creates inconsistency.
The component content mixes inline styles with Tailwind classes inconsistently, and has hardcoded dimensions that reduce flexibility.
Issues in the embedded component:
- Hardcoded dimensions:
maxWidth: "700px", minWidth: "700px"should be configurable or use responsive Tailwind classes - Mixed styling approaches: Some elements use inline styles while others use Tailwind classes
- Accessibility concerns: Missing ARIA attributes for accordion functionality
Consider these improvements for the embedded component:
Replace hardcoded dimensions:
- style={{ maxWidth: "700px", minWidth: "700px" }}
+ className="mx-auto w-full max-w-2xl rounded-lg bg-white p-4"Use consistent Tailwind classes instead of inline styles:
- style={{
- backgroundColor: openItem === item.id.toString() ? "#E0F7FA" : "",
- }}
+ className={`relative flex items-center space-x-2 rounded-xl p-2 transition-colors hover:bg-cyan-50 ${
+ openItem === item.id.toString() ? "bg-cyan-50" : "bg-gray-100"
+ }`}Add accessibility attributes:
<Accordion.Trigger
- className="flex w-full items-center justify-start gap-x-4"
+ className="flex w-full items-center justify-start gap-x-4"
+ aria-expanded={openItem === item.id.toString()}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In public/r/accordion-faq.json at line 11, the embedded React component mixes
inline styles with Tailwind CSS classes and uses hardcoded fixed widths,
reducing flexibility and consistency. To fix this, replace the fixed inline
styles for maxWidth and minWidth with responsive Tailwind width classes (e.g.,
max-w-xl or max-w-2xl) to allow better responsiveness. Remove inline style
objects where possible and convert those styles into equivalent Tailwind utility
classes for consistency. Additionally, add appropriate ARIA attributes to the
Accordion elements to improve accessibility, such as aria-expanded and
aria-controls on triggers and roles on accordion items.
|
Thanks for the PR @projectashik. I will create a dedicated registry and add all components. |
I have create the registry for the
accordion-faqcomponent to try out for.When this command is executed, it'll install @radix-ui/accordion, framer-motion and will create a file in components/animata/faq.tsx.
Summary by CodeRabbit
New Features
Chores
Style