-
Notifications
You must be signed in to change notification settings - Fork 860
[EuiFlyout] Forward ref to the flyout components #9318
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
[EuiFlyout] Forward ref to the flyout components #9318
Conversation
| ref | ||
| ) => { | ||
| const flyoutId = useFlyoutId(id); | ||
| const [flyoutRef, setFlyoutRef] = useState<HTMLElement | null>(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.
The reason we use useState here is because of useResizeObserver hook that accepts a DOM element (container) as an argument and uses useEffect to attach the observer, see:
eui/packages/eui/src/components/observer/resize_observer/resize_observer.tsx
Lines 92 to 100 in 1bddb4c
| useEffect(() => { | |
| if (container != null) { | |
| const observer = makeResizeObserver(container, ([entry]) => { | |
| const { inlineSize, blockSize } = entry.borderBoxSize[0]; | |
| setSize({ | |
| width: inlineSize, | |
| height: blockSize, | |
| }); | |
| }); |
If we use useRef, flyoutRef.current is null during the initial render. That means when the component mounts and the ref is populated, useRef does not trigger a re-render and the useResizeObserver is not re-evaluated so the observer fails to attach to the element.
By using useState and "callback ref" pattern we basically ensure that the component will re-render and pass the actual el to the observer.
I memoized the ref array before passing to useCombinedRefs to minimize unnecessary updates.
09359b4 to
34e2f32
Compare
💚 Build SucceededHistory
|
💚 Build Succeeded
|
| }); | ||
| }); | ||
|
|
||
| describe('ref forwarding', () => { |
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.
Thanks for adding tests! 🙏
|
[no change request, unrelated to the changes] ℹ️ While testing I did encounter an unexpected behavior but it's unrelated to the changes in this PR as it's reproducible also on production. Screen.Recording.2026-01-15.at.16.31.14.mov |
mgadewoll
left a comment
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.
🟢 All changes are LGTM.
The flyout functionality works as expected and there is no regression with production.
Thanks for tackling this bug fix! 💪
|
@mgadewoll That's a very good catch, I was able to reproduce. The reason why this happens is when the flyouts are stacked (because of small viewport), the main flyout ( The fix is very simple, it's just using the child flyout's width for the push padding calculation when in stacked layout mode and it's the main flyout. I have the code prepared and it seems to be working well: Kapture.2026-01-16.at.10.23.06.mp4It's not urgent while this PR is, so I'll push it on a separate one. Thank you for the fast review, Lene! 🙏🏻 EDIT: #9322 |
## Dependency updates - `@elastic/eui`: `v111.1.0` ⏩ `v112.0.0` - `@elastic/eslint-plugin-eui`: `v2.6.0` ⏩ `v2.7.0` --- ## Package updates ### `@elastic/eui` [v112.0.0](https://github.com/elastic/eui/releases/tag/v112.0.0) - Added `productDiscover` icon ([#9311](elastic/eui#9311)) - Updated `chartGauge` icon glyph ([#9311](elastic/eui#9311)) - Updated icon glyphs `endpoint` `eraser` `errorFill` `error` `eyeSlash` `faceHappy` `faceNeutral` `faceSad` `folder` `fullScreenExit` `fullScreen` `gradient` `grid` `heart` `home` `if` `image` `infinity` `inputOutput` `key` `keyboard` `lineBreakSlash` `lineBreak` `lineDash` `lineDot` `lineSolid` `logOut` `magnifyMinus` `magnifyPlus` `magnify` `mail` `map` `mapping` `menuLeft` `menuRight` `menu` `merge` `minusCircle` `minusSquare` `minus` `money` `moon` `move` `nested` `number` `package` `paintBucket` `palette` `paperClip` `partial` `pattern` `pause` `pencil` `percent` `pinFill` `pin` `pivot` `plusCircle` `plusSquare` `plus` `popper` `presentation` `processor` `productStreamsWired` `queryField` `queryOperand` `querySelector` `queryValue` `query` `question` `quote` `radar` `readOnly` `redo` `reporter` `return` `rocket` `scissors` `send` `shard` `share` `snowflake` `sortAscending` `sortDescending` `starFill` `star` `stop` `sun` `tableInfo` `tableTime` `textAlignCenter` `textAlignLeft` `textAlignRight` `textBold` `textHeading` `textItalic` `textStrike` `textUnderline` `thermometer` `thumbDown` `thumbUp` `timeline` `transitionLeftIn` `transitionLeftOut` `transitionTopIn` `transitionTopOut` `undo` `vectorSquare` `vectorTriangle` `videoPlayer` `warningFill` `waypoint` `wifiSlash` `wifi` ([#9303](elastic/eui#9303)) ([#9303](elastic/eui#9303)) - Added icons - `archive` `unarchive` `axisX` `axisYLeft` `axisYRight` `bulb` `cloud` `hourglass` `megaphone` `workflow` ([#9303](elastic/eui#9303)) - Added `headerVisibility` prop on `EuiDataGrid` to support rendering the datagrid header element optionally ([#9281](elastic/eui#9281)) - Updated 244 icon definitions to a more consistent naming convention. All 100 renamed icons include a backward-compatible alias in the icon map to support legacy references. ([#9279](elastic/eui#9279)) - Added icons `briefcase`, `productCloudInfra`, `productDashboard`, `productML` ([#9301](elastic/eui#9301)) - Updated glyphs `bullseye`, `bolt` ([#9301](elastic/eui#9301)) - Added `dismissButtonProps` prop to `EuiCallOut` ([#9285](elastic/eui#9285)) **Bug fixes** - Fixed `EuiFlyout` to properly forward refs when `session` prop is used. ([#9318](elastic/eui#9318)) - Fixed `EuiDataGrid` cells scrolling into view while trying to select text ([#9276](elastic/eui#9276)) **Breaking changes** - Removed `euiPaletteForLightBackground` and `euiPaletteForDarkBackground` deprecated palette functions. Use `euiTheme.colors.vis.euiColorVisText{NUMBER}` tokens instead. ([#9296](elastic/eui#9296)) **Accessibility** - Improved the accessibility of `EuiDataGrid`s column selector drag handle buttons by ensuring distinctive labels ([#9282](elastic/eui#9282)) ### `@elastic/eslint-plugin-eui` v2.7.0 - Added `no-static-z-index` rule ([#9236](elastic/eui#9236))
## Dependency updates - `@elastic/eui`: `v111.1.0` ⏩ `v112.0.0` - `@elastic/eslint-plugin-eui`: `v2.6.0` ⏩ `v2.7.0` --- ## Package updates ### `@elastic/eui` [v112.0.0](https://github.com/elastic/eui/releases/tag/v112.0.0) - Added `productDiscover` icon ([elastic#9311](elastic/eui#9311)) - Updated `chartGauge` icon glyph ([elastic#9311](elastic/eui#9311)) - Updated icon glyphs `endpoint` `eraser` `errorFill` `error` `eyeSlash` `faceHappy` `faceNeutral` `faceSad` `folder` `fullScreenExit` `fullScreen` `gradient` `grid` `heart` `home` `if` `image` `infinity` `inputOutput` `key` `keyboard` `lineBreakSlash` `lineBreak` `lineDash` `lineDot` `lineSolid` `logOut` `magnifyMinus` `magnifyPlus` `magnify` `mail` `map` `mapping` `menuLeft` `menuRight` `menu` `merge` `minusCircle` `minusSquare` `minus` `money` `moon` `move` `nested` `number` `package` `paintBucket` `palette` `paperClip` `partial` `pattern` `pause` `pencil` `percent` `pinFill` `pin` `pivot` `plusCircle` `plusSquare` `plus` `popper` `presentation` `processor` `productStreamsWired` `queryField` `queryOperand` `querySelector` `queryValue` `query` `question` `quote` `radar` `readOnly` `redo` `reporter` `return` `rocket` `scissors` `send` `shard` `share` `snowflake` `sortAscending` `sortDescending` `starFill` `star` `stop` `sun` `tableInfo` `tableTime` `textAlignCenter` `textAlignLeft` `textAlignRight` `textBold` `textHeading` `textItalic` `textStrike` `textUnderline` `thermometer` `thumbDown` `thumbUp` `timeline` `transitionLeftIn` `transitionLeftOut` `transitionTopIn` `transitionTopOut` `undo` `vectorSquare` `vectorTriangle` `videoPlayer` `warningFill` `waypoint` `wifiSlash` `wifi` ([elastic#9303](elastic/eui#9303)) ([elastic#9303](elastic/eui#9303)) - Added icons - `archive` `unarchive` `axisX` `axisYLeft` `axisYRight` `bulb` `cloud` `hourglass` `megaphone` `workflow` ([elastic#9303](elastic/eui#9303)) - Added `headerVisibility` prop on `EuiDataGrid` to support rendering the datagrid header element optionally ([elastic#9281](elastic/eui#9281)) - Updated 244 icon definitions to a more consistent naming convention. All 100 renamed icons include a backward-compatible alias in the icon map to support legacy references. ([elastic#9279](elastic/eui#9279)) - Added icons `briefcase`, `productCloudInfra`, `productDashboard`, `productML` ([elastic#9301](elastic/eui#9301)) - Updated glyphs `bullseye`, `bolt` ([elastic#9301](elastic/eui#9301)) - Added `dismissButtonProps` prop to `EuiCallOut` ([elastic#9285](elastic/eui#9285)) **Bug fixes** - Fixed `EuiFlyout` to properly forward refs when `session` prop is used. ([elastic#9318](elastic/eui#9318)) - Fixed `EuiDataGrid` cells scrolling into view while trying to select text ([elastic#9276](elastic/eui#9276)) **Breaking changes** - Removed `euiPaletteForLightBackground` and `euiPaletteForDarkBackground` deprecated palette functions. Use `euiTheme.colors.vis.euiColorVisText{NUMBER}` tokens instead. ([elastic#9296](elastic/eui#9296)) **Accessibility** - Improved the accessibility of `EuiDataGrid`s column selector drag handle buttons by ensuring distinctive labels ([elastic#9282](elastic/eui#9282)) ### `@elastic/eslint-plugin-eui` v2.7.0 - Added `no-static-z-index` rule ([elastic#9236](elastic/eui#9236))
Summary
Important
This is a high-priority PR. It should be reviewed, tested and released as soon as possible.
EuiFlyoutChild,EuiFlyoutMainandEuiFlyoutManagedwithforwardRefand passed it toEuiFlyoutComponent.refinEuiFlyouttoEuiFlyoutChildandEuiFlyoutMain.null.Why are we making this change?
Resolves #9313
When session flyouts are used, the focus doesn't properly move to the flyout content, thus creating a big accessibility issue and potentially breaking focus-reliant functionality on consumer side. The reason was flyout components (
EuiFlyoutChild,EuiFlyoutMainetc.) weren't wrapped withforwardRefandEuiFlyoutdidn't pass theref. It came back asnull.It has been reported internally by Kibana teams. Unblocks elastic/kibana#246719.
Screenshots #
In Discover
Screen.Recording.2026-01-15.at.12.38.54.mov
Screen.Recording.2026-01-15.at.12.31.05.mov
Impact to users
🟢 This is a bug fix. It's not a breaking change. No changes needed on consumer side. No visual changes were made.
QA
Specific checklist
overlayandpushas expectedGeneral checklist
@defaultif default values are missing) and playground toggles