Fix EuiAccordion zero height under React Strict Mode - #9811
Open
cleydyr wants to merge 6 commits into
Open
Conversation
…e compatibility - updated EuiAccordionChildren to handle content height correctly - change useObserver to reconnect observers correctly when remounting - reset cached dimensions for accurate onResize callbacks - improved tests for observer components to validate behavior under Strict Mode (I can remove it if it's not convenient for it to be there) - added a new story for EuiAccordion demonstrating initial open state within Strict Mode (ditto, can remove it).
… EuiFlyout - new test for EuiAccordion to validate its behavior under React Strict Mode when rendered inside an EuiFlyout. - added a corresponding story to demonstrate the accordion's functionality in this context, addressing issue elastic#9029. - enhanced the existing EuiAccordion stories to include examples of its use in Strict Mode (can remove it if needed).
Co-authored-by: Cursor <cursoragent@cursor.com>
|
👋 Since this is a community submitted pull request, a Buildkite build has not been started automatically. Would an Elastic organization member please verify the contents of this pull request and kick off a build manually? |
github-actions
Bot
added
the
community contribution
Contributor
|
buildkite test this |
💚 Build Succeeded
|
💔 Build Failed
Failed CI StepsHistory
|
weronikaolejniczak
requested changes
Jul 21, 2026
Contributor
There was a problem hiding this comment.
blocking:
We can add a small targeted unit test in the existing accordion.test.tsx file. We don't need a separate suite just for Strict Mode 😄
Contributor
There was a problem hiding this comment.
blocking:
For testing the result it's fine but before I approve, we have to remove these stories. We don't need StrictMode specific ones.
Comment on lines
+96
to
+108
| const heightInlineStyle = useMemo(() => { | ||
| if (!isOpen) { | ||
| return { blockSize: 0 }; | ||
| } | ||
|
|
||
| // Avoid overriding CSS `height: auto` with `0` before the first resize | ||
| // measurement (e.g. `initialIsOpen` under React Strict Mode). | ||
| if (contentHeight === 0) { | ||
| return undefined; | ||
| } | ||
|
|
||
| return { blockSize: contentHeight }; | ||
| }, [isOpen, contentHeight]); |
Contributor
There was a problem hiding this comment.
nit:
This is very opinionated but we could tighten this like so:
Suggested change
| const heightInlineStyle = useMemo(() => { | |
| if (!isOpen) { | |
| return { blockSize: 0 }; | |
| } | |
| // Avoid overriding CSS `height: auto` with `0` before the first resize | |
| // measurement (e.g. `initialIsOpen` under React Strict Mode). | |
| if (contentHeight === 0) { | |
| return undefined; | |
| } | |
| return { blockSize: contentHeight }; | |
| }, [isOpen, contentHeight]); | |
| const heightInlineStyle = useMemo(() => { | |
| if (!isOpen) return { blockSize: 0 }; | |
| // Avoid overriding CSS `height: auto` with `0` before the first resize. | |
| if (contentHeight === 0) return undefined; | |
| return { blockSize: contentHeight }; | |
| }, [isOpen, contentHeight]); |
instead of having 13 lines we have 6 and the code is equally meaningful.
Contributor
There was a problem hiding this comment.
blocking:
The filename is supposed to be 9811.md, from the PR number.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a React 18 Strict Mode regression where EuiAccordion content could render at zero height (notably when initialIsOpen inside EuiFlyout) by hardening observer lifecycle handling and avoiding premature block-size: 0 inline styles before first measurement.
Changes:
- Reconnect observers in
useObserverafter Strict Mode effect cleanup and ensure observers can resume even when refs aren’t re-fired. - Ensure
EuiResizeObserveremitsonResizeafter reconnect by resetting cached dimensions on (re)observe. - Prevent open accordions from temporarily forcing
block-size: 0before the first resize measurement; add stories + regression tests and update snapshots/changelog.
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/eui/src/components/observer/use_observer.ts | Reconnects observers after Strict Mode cleanup; clears observer ref on cleanup. |
| packages/eui/src/components/observer/use_observer.test.tsx | Adds Strict Mode reconnection regression test. |
| packages/eui/src/components/observer/resize_observer/resize_observer.tsx | Resets cached size on observe so post-reconnect onResize fires. |
| packages/eui/src/components/observer/resize_observer/resize_observer.test.tsx | Adds unit coverage asserting onResize fires again after reconnect. |
| packages/eui/src/components/accordion/accordion_children/accordion_children.tsx | Avoids inline block-size: 0 while open and awaiting first measurement. |
| packages/eui/src/components/accordion/accordion.stories.tsx | Adds Strict Mode reproduction stories (standalone + in flyout). |
| packages/eui/src/components/accordion/accordion_strict_mode.test.tsx | Adds regression test for Strict Mode + flyout + initialIsOpen. |
| packages/eui/src/components/accordion/snapshots/accordion.test.tsx.snap | Updates snapshots for removed block-size: 0 inline style when open. |
| packages/eui/changelogs/upcoming/9029.md | Adds bugfix changelog entry for issue #9029. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+23
to
+31
| jest.mock('../observer/resize_observer/resize_observer.tsx', () => { | ||
| const { createResizeObserverMock } = jest.requireActual< | ||
| typeof import('../../test/internal') | ||
| >('../../test/internal'); | ||
| resizeObserverMockRef.current = createResizeObserverMock(); | ||
| global.ResizeObserver = resizeObserverMockRef.current.ResizeObserver; | ||
|
|
||
| return jest.requireActual('../observer/resize_observer/resize_observer.tsx'); | ||
| }); |
Comment on lines
+110
to
+112
| (process.env.REACT_VERSION === '18' ? it : it.skip)( | ||
| 'reconnects the observer after StrictMode remounts without a ref change', | ||
| () => { |
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
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.
Summary
EuiAccordionrendering with zero height in React 18 Strict Mode, including when mounted insideEuiFlyoutwithinitialIsOpen.contentHeightstayed0— most commonly seen withinitialIsOpenand in flyouts.useObserver— Reconnect observers after Strict Mode effect cleanup (React 18 often disconnects without re-firing the ref callback). ClearobserverRefon cleanup.EuiResizeObserver— Reset cached dimensions inbeginObserveso the first callback after reconnect always firesonResize.EuiAccordionChildren— Omit inlineblock-size: 0while open and awaiting the first measurement, allowing CSSheight: autoto show content immediately.Regression coverage added in
accordion_strict_mode.test.tsx,use_observer.test.tsx, andresize_observer.test.tsx. Storybook stories added for standalone and flyout reproductions.API Changes
Screenshots
(from Storybook)
Impact Assessment
style="block-size: 0;"before the first resize measurement. Snapshot tests updated accordingly. No intentional visual regression expected once content is measured.block-size: 0on open). New unit/integration tests added. Full suite passes on React 17 and 18 (459 suites / 6461 tests).Impact level: 🟢 Low
Release Readiness
QA instructions for reviewer
packages/eui:yarn start.euiAccordion__childWrapper— should not haveblock-size: 0pxwhile openinitialIsOpenaccordions (see issue [Strict Mode] EuiAccordion doesn't display within EuiFlyout #9029 for known usages)Checklist before marking Ready for Review
packages/eui/changelogs/upcoming/9029.md(rename to PR number after opening, if followingyarn yo-changelogconvention)Reviewer checklist