-
-
Notifications
You must be signed in to change notification settings - Fork 996
fix: Add smooth transition on Accordian #4569
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: master
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
|
We require all PRs to follow Conventional Commits specification. |
WalkthroughThe AccordionItem component now always renders the content area but controls visibility through dynamic CSS classes instead of conditional rendering. This enables smooth height and opacity transitions when expanding or collapsing accordion items, fixing animation jankiness reported in the linked issue. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
fix-faq-section.mp4 |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4569--asyncapi-website.netlify.app/ |
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 (2)
components/Accordion/AccordionItem.tsx (2)
60-64: Consider addingaria-hiddenfor accessibility.The content is always in the DOM but visually hidden when inactive. Screen readers may still announce it.
- { - <div className={`rounded-sm border-t border-gray-200 font-body font-regular text-gray-700 antialiased overflow-hidden transition-all duration-300 ease-in-out ${isActive ? 'max-h-40 opacity-100 py-2' : 'max-h-0 opacity-0 py-0'}`}> + <div + className={`rounded-sm border-t border-gray-200 font-body font-regular text-gray-700 antialiased overflow-hidden transition-all duration-300 ease-in-out ${ + isActive ? 'max-h-screen opacity-100 py-2' : 'max-h-0 opacity-0 py-0' + }`} + aria-hidden={!isActive} + > {content} </div> - }
60-64: Remove unnecessary curly braces.The wrapping
{ }on lines 60 and 64 serve no purpose and reduce readability.- { - <div className={`...`}> + <div className={`...`}> {content} </div> - }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/Accordion/AccordionItem.tsx(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: PR testing - if Node project
components/Accordion/AccordionItem.tsx
[error] 61-61: Prettier formatting issue. Replace the long className string with a formatted multi-line expression. Run 'prettier --write' to fix code style.
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: Lighthouse CI
| {isActive && ( | ||
| <div className='rounded-sm border-t border-gray-200 py-2 font-body font-regular text-gray-700 antialiased'> | ||
| { | ||
| <div className={`rounded-sm border-t border-gray-200 font-body font-regular text-gray-700 antialiased overflow-hidden transition-all duration-300 ease-in-out ${isActive ? 'max-h-40 opacity-100 py-2' : 'max-h-0 opacity-0 py-0'}`}> |
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.
Critical: max-h-40 will truncate content exceeding 160px.
The fixed max-h-40 class (160px) will clip FAQ answers longer than that height, breaking the user experience. FAQ content varies significantly in length.
Use max-h-screen or a sufficiently large value instead:
- <div className={`rounded-sm border-t border-gray-200 font-body font-regular text-gray-700 antialiased overflow-hidden transition-all duration-300 ease-in-out ${isActive ? 'max-h-40 opacity-100 py-2' : 'max-h-0 opacity-0 py-0'}`}>
+ <div className={`rounded-sm border-t border-gray-200 font-body font-regular text-gray-700 antialiased overflow-hidden transition-all duration-300 ease-in-out ${isActive ? 'max-h-screen opacity-100 py-2' : 'max-h-0 opacity-0 py-0'}`}>📝 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.
| <div className={`rounded-sm border-t border-gray-200 font-body font-regular text-gray-700 antialiased overflow-hidden transition-all duration-300 ease-in-out ${isActive ? 'max-h-40 opacity-100 py-2' : 'max-h-0 opacity-0 py-0'}`}> | |
| <div className={`rounded-sm border-t border-gray-200 font-body font-regular text-gray-700 antialiased overflow-hidden transition-all duration-300 ease-in-out ${isActive ? 'max-h-screen opacity-100 py-2' : 'max-h-0 opacity-0 py-0'}`}> |
🧰 Tools
🪛 GitHub Actions: PR testing - if Node project
[error] 61-61: Prettier formatting issue. Replace the long className string with a formatted multi-line expression. Run 'prettier --write' to fix code style.
🤖 Prompt for AI Agents
In components/Accordion/AccordionItem.tsx around line 61, the hardcoded class
max-h-40 (160px) will truncate longer FAQ answers; replace it with a much larger
max-height such as max-h-screen (or a large explicit value like max-h-[1000px])
so content is not clipped when expanded, keeping the existing transition/opacity
classes intact and ensuring the expanded state uses the new max-h class instead
of max-h-40.
Fix Prettier formatting violation.
The pipeline flagged this line for exceeding length limits. Break the className into multiple lines.
- <div className={`rounded-sm border-t border-gray-200 font-body font-regular text-gray-700 antialiased overflow-hidden transition-all duration-300 ease-in-out ${isActive ? 'max-h-40 opacity-100 py-2' : 'max-h-0 opacity-0 py-0'}`}>
+ <div
+ className={`rounded-sm border-t border-gray-200 font-body font-regular text-gray-700 antialiased overflow-hidden transition-all duration-300 ease-in-out ${
+ isActive ? 'max-h-screen opacity-100 py-2' : 'max-h-0 opacity-0 py-0'
+ }`}
+ >
{content}
</div>Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: PR testing - if Node project
[error] 61-61: Prettier formatting issue. Replace the long className string with a formatted multi-line expression. Run 'prettier --write' to fix code style.
🤖 Prompt for AI Agents
In components/Accordion/AccordionItem.tsx around line 61, the single-line JSX
className exceeds Prettier's line-length limit; break the className string into
multiple concatenated/template segments or multiline JSX expression so each line
is shorter. Reformat the div's className prop across multiple lines (group
static classes on one or more lines and the conditional part on its own line),
keeping the same classes and ternary logic, ensuring indentation aligns with JSX
and Prettier will accept the wrapped lines.
Description
This change is only for UI UX purpose , no functionality is changed or added.
Fixed the Accordian transition by adding style properties max height and opacity , which makes the item visible only when it is active, this styling change allows a smooth transition to take place
Related issue(s)
Fixes #4566
Summary by CodeRabbit