fix: panel height not updating when conditionally rendering fields #579
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 108592b:
|
Co-authored-by: gsimone <[email protected]>
Co-authored-by: gsimone <[email protected]>
Co-authored-by: gsimone <[email protected]>
|
@copilot write a story for this change |
Co-authored-by: gsimone <[email protected]>
Added a comprehensive Storybook story "Conditional Rendering - Panel Height Fix" in commit bda7da4. The story demonstrates:
You can find it at: Misc / Input options → Conditional Rendering - Panel Height Fix Screenshots showing the panel expanding as sections are toggled: Initial state: With Advanced Settings: Fully expanded: Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
gsimone
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR adds automatic panel height adjustment for Leva's collapsible folders when their content size changes dynamically. The main enhancement allows folders to smoothly animate their height when nested content is conditionally shown or hidden.
- Implements ResizeObserver in
useTogglehook to detect content size changes while expanded - Adds smooth height transitions when content changes using requestAnimationFrame
- Includes a comprehensive Storybook example demonstrating conditional rendering with nested folders
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/leva/src/hooks/useToggle.ts | Adds ResizeObserver-based dynamic height adjustment with transition animations for expanded panels |
| packages/leva/stories/input-options.stories.tsx | Adds Storybook story demonstrating the panel height auto-adjustment feature with conditional rendering examples |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| useEffect(() => { | ||
| if (!toggled || !contentRef.current || !wrapperRef.current) return | ||
|
|
||
| const wrapper = wrapperRef.current | ||
| let rafId: number | null = null | ||
| let currentTransitionHandler: (() => void) | null = null | ||
|
|
||
| const resizeObserver = new ResizeObserver(() => { | ||
| const content = contentRef.current | ||
| if (!content) return | ||
|
|
||
| // Cancel any pending animation | ||
| if (rafId !== null) { | ||
| cancelAnimationFrame(rafId) | ||
| rafId = null | ||
| } | ||
|
|
||
| // Remove any existing transition handler | ||
| if (currentTransitionHandler) { | ||
| wrapper.removeEventListener('transitionend', currentTransitionHandler) | ||
| currentTransitionHandler = null | ||
| } | ||
|
|
||
| // Get the current and target heights | ||
| const currentHeight = wrapper.getBoundingClientRect().height | ||
| const targetHeight = content.getBoundingClientRect().height | ||
|
|
||
| // Only update if there's a meaningful difference | ||
| if (Math.abs(currentHeight - targetHeight) > 1) { | ||
| // Set explicit height to enable transition | ||
| wrapper.style.height = currentHeight + 'px' | ||
|
|
||
| // Use requestAnimationFrame to ensure the height is set before changing it | ||
| rafId = requestAnimationFrame(() => { | ||
| rafId = null | ||
| wrapper.style.height = targetHeight + 'px' | ||
|
|
||
| // Remove fixed height after transition completes | ||
| const handleTransitionEnd = () => { | ||
| wrapper.style.removeProperty('height') | ||
| currentTransitionHandler = null | ||
| } | ||
| currentTransitionHandler = handleTransitionEnd | ||
| wrapper.addEventListener('transitionend', handleTransitionEnd, { once: true }) | ||
| }) | ||
| } | ||
| }) | ||
|
|
||
| resizeObserver.observe(contentRef.current) | ||
|
|
||
| return () => { | ||
| resizeObserver.disconnect() | ||
| if (rafId !== null) { | ||
| cancelAnimationFrame(rafId) | ||
| } | ||
| if (currentTransitionHandler) { | ||
| wrapper.removeEventListener('transitionend', currentTransitionHandler) | ||
| } | ||
| } | ||
| }, [toggled]) |
There was a problem hiding this comment.
Potential race condition when toggling from closed to open. Both the toggle animation effect (lines 96-127) and this ResizeObserver effect will be active simultaneously. During the expansion animation, the ResizeObserver may fire and attempt to adjust the height while the toggle animation is still in progress, causing conflicting height manipulations.
Consider one of these approaches:
- Add a delay before observing (e.g., using a timeout matching the transition duration)
- Track animation state with a ref and skip ResizeObserver adjustments during toggle animations
- Use a flag to temporarily disable ResizeObserver callbacks during toggle transitions
Example:
const isAnimating = useRef(false)
// In first useEffect, set isAnimating.current = true and clear in transitionend
// In ResizeObserver callback, check if (!isAnimating.current) before adjusting| const handleTransitionEnd = () => { | ||
| wrapper.style.removeProperty('height') | ||
| currentTransitionHandler = null | ||
| } | ||
| currentTransitionHandler = handleTransitionEnd | ||
| wrapper.addEventListener('transitionend', handleTransitionEnd, { once: true }) | ||
| }) |
There was a problem hiding this comment.
The transitionend event may not fire in certain edge cases (e.g., if the transition is interrupted, the element is hidden, or the browser skips very small transitions), which would leave the explicit height style set on the wrapper and prevent it from properly auto-sizing.
Consider adding a fallback timeout to ensure the height property is eventually removed:
const handleTransitionEnd = () => {
wrapper.style.removeProperty('height')
currentTransitionHandler = null
if (timeoutId !== null) {
clearTimeout(timeoutId)
timeoutId = null
}
}
currentTransitionHandler = handleTransitionEnd
wrapper.addEventListener('transitionend', handleTransitionEnd, { once: true })
// Fallback timeout (transition is 300ms)
const timeoutId = setTimeout(handleTransitionEnd, 400)| const handleTransitionEnd = () => { | |
| wrapper.style.removeProperty('height') | |
| currentTransitionHandler = null | |
| } | |
| currentTransitionHandler = handleTransitionEnd | |
| wrapper.addEventListener('transitionend', handleTransitionEnd, { once: true }) | |
| }) | |
| let timeoutId: number | null = null | |
| const handleTransitionEnd = () => { | |
| wrapper.style.removeProperty('height') | |
| currentTransitionHandler = null | |
| if (timeoutId !== null) { | |
| clearTimeout(timeoutId) | |
| timeoutId = null | |
| } | |
| } | |
| currentTransitionHandler = handleTransitionEnd | |
| wrapper.addEventListener('transitionend', handleTransitionEnd, { once: true }) | |
| // Fallback timeout (transition is 300ms) | |
| timeoutId = window.setTimeout(handleTransitionEnd, 400) |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe useToggle hook is enhanced with a ResizeObserver to detect content size changes and smoothly animate panel height adjustments when controls are conditionally rendered. A Storybook story is added to demonstrate this panel height auto-adjustment behavior. Changes
Sequence DiagramsequenceDiagram
participant User
participant Panel
participant ResizeObserver as ResizeObserver
participant AnimationFrame as requestAnimationFrame
participant DOM
User->>Panel: Toggle conditional field visibility
Panel->>ResizeObserver: Detect content size change
ResizeObserver->>Panel: Content dimensions changed
Panel->>AnimationFrame: Queue height animation
AnimationFrame->>DOM: Set wrapper to current height
AnimationFrame->>DOM: Animate to target height
DOM->>DOM: Transition height (CSS)
DOM->>Panel: transitionend event
Panel->>DOM: Remove explicit height property
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20-25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🪛 GitHub Actions: CIpackages/leva/src/hooks/useToggle.ts[error] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues in this file. packages/leva/stories/input-options.stories.tsx[error] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues in this file. 🔇 Additional comments (2)
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 |
The Leva panel height doesn't update when fields are conditionally rendered using the
renderoption. The panel remains at its original size, causing overflow or clipped content.Changes
useTogglehook to detect content size changes while the panel is expandedImplementation
The ResizeObserver compares rendered heights (not style properties) and triggers smooth animations only when differences exceed 1px. Uses
requestAnimationFrameto ensure height changes are properly batched for transitions.Storybook Story
Added a comprehensive story "Conditional Rendering - Panel Height Fix" that demonstrates:
Location:
Misc / Input options → Conditional Rendering - Panel Height FixScreenshots
Before: Panel doesn't resize when folder appears
After: Panel smoothly animates to fit new content
Story - Initial state with basic fields:
Story - Expanded with advanced settings:
Story - Fully expanded with all sections:
Original prompt
<issue_description>See #540 </issue_description>
Comments on the Issue (you are @copilot in this section)
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
Summary by CodeRabbit
New Features
Documentation