feat: add smooth animation to Accordion component#1661
Closed
vadim-cebanu wants to merge 1 commit intothemesberg:mainfrom
Closed
feat: add smooth animation to Accordion component#1661vadim-cebanu wants to merge 1 commit intothemesberg:mainfrom
vadim-cebanu wants to merge 1 commit intothemesberg:mainfrom
Conversation
|
@vadim-cebanu is attempting to deploy a commit to the Bergside Team on Vercel. A member of the Team first needs to authorize it. |
|
Contributor
📝 WalkthroughWalkthroughThe AccordionContent component now animates expansion and collapse using CSS transitions. The hidden attribute is replaced with inline styles: maxHeight transitions from 0px (closed) to 1000px (open) over 0.3 seconds, with overflow hidden to manage content clipping smoothly. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes 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 unit tests (beta)
📝 Coding Plan
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 Tip You can disable the changed files summary in the walkthrough.Disable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/ui/src/components/Accordion/AccordionContent.tsx (1)
35-40: Merge consumer styles instead of allowing full overwrite.Because
...restPropsis spread afterstyle, a passedstyleprop can wipe out animation styles. Merge incoming style explicitly.Proposed fix
- const { className, ...restProps } = resolveProps(props, provider.props?.accordionContent); + const { className, style, ...restProps } = resolveProps(props, provider.props?.accordionContent); @@ style={{ + ...style, overflow: "hidden", maxHeight: isOpen ? "1000px" : "0", transition: "max-height 0.3s ease-in-out", }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/Accordion/AccordionContent.tsx` around lines 35 - 40, The component currently spreads {...restProps} after setting style which allows a consumer-provided style to fully overwrite the animation/overflow styles; change AccordionContent.tsx to extract style from restProps (e.g. const { style: consumerStyle, ...otherProps } = restProps), build a mergedStyle object that sets overflow, maxHeight (based on isOpen), and transition and then spreads consumerStyle last into that object (so consumer can override specific keys if desired), and finally render with style={mergedStyle} and {...otherProps} instead of {...restProps}.
🤖 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/ui/src/components/Accordion/AccordionContent.tsx`:
- Around line 37-38: The hard-coded maxHeight ("1000px") in AccordionContent
causes clipping; change AccordionContent to measure the panel element's
scrollHeight via a ref (e.g., contentRef) and set its style.maxHeight to isOpen
? `${contentRef.current.scrollHeight}px` : "0" to animate to the actual content
height, and optionally clear maxHeight to "none" after the transition ends to
allow dynamic content resizing; update any useEffect or event handlers in
AccordionContent that currently toggle maxHeight to use this measured value and
listen for transitionend to reset if needed.
- Around line 35-40: AccordionContent currently animates collapse using
maxHeight but doesn't set the hidden semantics, leaving collapsed content
reachable by assistive tech; modify the AccordionContent component to manage a
hidden boolean (or use the DOM hidden attribute) alongside isOpen: when opening,
remove hidden before starting the maxHeight transition; when closing, start the
transition then set hidden=true in an onTransitionEnd handler (use a ref to the
container and listen for its transitionend) so the element is removed from
accessibility tree after the animation completes; update any places using
isOpen/restProps and ensure the element includes aria-hidden or the hidden
attribute consistently.
---
Nitpick comments:
In `@packages/ui/src/components/Accordion/AccordionContent.tsx`:
- Around line 35-40: The component currently spreads {...restProps} after
setting style which allows a consumer-provided style to fully overwrite the
animation/overflow styles; change AccordionContent.tsx to extract style from
restProps (e.g. const { style: consumerStyle, ...otherProps } = restProps),
build a mergedStyle object that sets overflow, maxHeight (based on isOpen), and
transition and then spreads consumerStyle last into that object (so consumer can
override specific keys if desired), and finally render with style={mergedStyle}
and {...otherProps} instead of {...restProps}.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 70534b6c-2e89-4c78-b3e8-2d29ad7c61e0
📒 Files selected for processing (1)
packages/ui/src/components/Accordion/AccordionContent.tsx
Comment on lines
+35
to
40
| style={{ | ||
| overflow: "hidden", | ||
| maxHeight: isOpen ? "1000px" : "0", | ||
| transition: "max-height 0.3s ease-in-out", | ||
| }} | ||
| {...restProps} |
Contributor
There was a problem hiding this comment.
Preserve hidden semantics for collapsed panels.
Replacing hidden with only max-height animation can leave collapsed content reachable by assistive tech/focus until/unless explicitly hidden. Please keep animation and restore hidden semantics after collapse completes.
Proposed fix
-import type { ComponentProps } from "react";
+import { useEffect, useState, type ComponentProps } from "react";
@@
export function AccordionContent(props: AccordionContentProps) {
const { isOpen } = useAccordionContext();
+ const [isHidden, setIsHidden] = useState(!isOpen);
+
+ useEffect(() => {
+ if (isOpen) setIsHidden(false);
+ }, [isOpen]);
@@
<div
className={twMerge(theme.base, className)}
data-testid="flowbite-accordion-content"
+ hidden={isHidden}
+ aria-hidden={!isOpen}
+ onTransitionEnd={() => {
+ if (!isOpen) setIsHidden(true);
+ }}
style={{
overflow: "hidden",
maxHeight: isOpen ? "1000px" : "0",
transition: "max-height 0.3s ease-in-out",
}}
{...restProps}
/>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/Accordion/AccordionContent.tsx` around lines 35 -
40, AccordionContent currently animates collapse using maxHeight but doesn't set
the hidden semantics, leaving collapsed content reachable by assistive tech;
modify the AccordionContent component to manage a hidden boolean (or use the DOM
hidden attribute) alongside isOpen: when opening, remove hidden before starting
the maxHeight transition; when closing, start the transition then set
hidden=true in an onTransitionEnd handler (use a ref to the container and listen
for its transitionend) so the element is removed from accessibility tree after
the animation completes; update any places using isOpen/restProps and ensure the
element includes aria-hidden or the hidden attribute consistently.
Comment on lines
+37
to
+38
| maxHeight: isOpen ? "1000px" : "0", | ||
| transition: "max-height 0.3s ease-in-out", |
Contributor
There was a problem hiding this comment.
Avoid hard-coded maxHeight: 1000px; it can truncate content.
If panel content exceeds 1000px, the open state is clipped. Measure and animate to scrollHeight instead of a fixed cap.
Proposed fix
-import type { ComponentProps } from "react";
+import { useLayoutEffect, useRef, useState, type ComponentProps } from "react";
@@
export function AccordionContent(props: AccordionContentProps) {
const { isOpen } = useAccordionContext();
+ const contentRef = useRef<HTMLDivElement>(null);
+ const [maxHeight, setMaxHeight] = useState("0px");
+
+ useLayoutEffect(() => {
+ const el = contentRef.current;
+ if (!el) return;
+ setMaxHeight(isOpen ? `${el.scrollHeight}px` : "0px");
+ }, [isOpen, props.children]);
@@
<div
+ ref={contentRef}
className={twMerge(theme.base, className)}
data-testid="flowbite-accordion-content"
style={{
overflow: "hidden",
- maxHeight: isOpen ? "1000px" : "0",
+ maxHeight,
transition: "max-height 0.3s ease-in-out",
}}
{...restProps}
/>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/Accordion/AccordionContent.tsx` around lines 37 -
38, The hard-coded maxHeight ("1000px") in AccordionContent causes clipping;
change AccordionContent to measure the panel element's scrollHeight via a ref
(e.g., contentRef) and set its style.maxHeight to isOpen ?
`${contentRef.current.scrollHeight}px` : "0" to animate to the actual content
height, and optionally clear maxHeight to "none" after the transition ends to
allow dynamic content resizing; update any useEffect or event handlers in
AccordionContent that currently toggle maxHeight to use this measured value and
listen for transitionend to reset if needed.
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.
Adds a smooth slide-down animation to the Accordion component when opening/closing panels.
Currently the Accordion expands and collapses instantly which can be jarring for users.
This fix adds a CSS transition using max-height to create a smooth animation effect.
Fixes #909
Summary by CodeRabbit