Skip to content

[EuiFocusTrap] Migrate to function component#9599

Open
mgadewoll wants to merge 4 commits intoelastic:mainfrom
mgadewoll:focustrap/migrate-to-function-component
Open

[EuiFocusTrap] Migrate to function component#9599
mgadewoll wants to merge 4 commits intoelastic:mainfrom
mgadewoll:focustrap/migrate-to-function-component

Conversation

@mgadewoll
Copy link
Copy Markdown
Contributor

@mgadewoll mgadewoll commented Apr 17, 2026

Summary

closes #9490

This PR refactors EuiFocusTrap from a class component to a function component.

API Changes

⚪ no API changes

Screenshots

description Before After
non strict mode
Screen.Recording.2026-04-17.at.14.15.55.mov
Screen.Recording.2026-04-17.at.14.18.48.mov
strict mode
Screen.Recording.2026-04-17.at.14.17.08.mov
Screen.Recording.2026-04-17.at.14.12.22.mov

Impact Assessment

Note: Most PRs should be tested in Kibana to help gauge their Impact before merging.

  • 🔴 Breaking changes — What will break? How many usages in Kibana/Cloud UI are impacted?
  • 💅 Visual changes — May impact style overrides; could require visual testing. Explain and estimate impact.
  • 🧪 Test impact — May break functional or snapshot tests (e.g., HTML structure, class names, default values).
  • 🔧 Hard to integrate — If changes require substantial updates to Kibana, please stage the changes and link them here.

Impact level: 🟢 None

The refactor is a 1:1 functional and structural migration. There should not be any impact.

ℹ️ The changes were run in Kibana CI (🟢 CI build)

Release Readiness

  • Documentation: {link to docs page(s)}
  • Figma: {link to Figma or issue}
  • Migration guide: {steps or link, for breaking/visual changes or deprecations}
  • Adoption plan (new features): {link to issue/doc or outline who will integrate this and where}

QA instructions for reviewer

💻 EuiFocusTrap storybook
💻 React 18 strict mode testing codesandbox
💻 React 19 strict mode testing codesandbox

  • verify there is no regression with production
  • verify the component works as expected in strict mode (e.g. use the above linked codesandboxes or enable it locally in Storybook by adding strictMode: true here or spin up your own react app)
  • verify that (example) components usingEuiFocusTrap work as expected (see above linked codesandboxes)
    • ⚠️ In React 18 strict mode, EuiFocusTrap does not return the focus. That's a pre-existing issue likely from react-focus-on. For the purpose of this migration PR this is in "parity".

Checklist before marking Ready for Review

Reviewer checklist

  • Approved Impact Assessment — Acceptable to merge given the consumer impact.
  • Approved Release Readiness — Docs, Figma, and migration info are sufficient to ship.

@mgadewoll mgadewoll self-assigned this Apr 17, 2026
@mgadewoll mgadewoll added the skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) label Apr 17, 2026
@mgadewoll mgadewoll marked this pull request as ready for review April 28, 2026 10:29
@mgadewoll mgadewoll requested a review from a team as a code owner April 28, 2026 10:29
Copilot AI review requested due to automatic review settings April 28, 2026 10:29
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors EuiFocusTrap (accessibility-critical focus management utility) from a class component to a hook-based function component while aiming to preserve existing behavior.

Changes:

  • Replaced class lifecycle methods with useEffect/useUpdateEffect and hook state for click-outside disabling
  • Reworked click-outside mouseup handling to use document-level listeners stored in a ref
  • Consolidated prop defaulting via destructuring and continued support for EuiProvider component defaults via usePropsWithComponentDefaults

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/eui/src/components/focus_trap/focus_trap.tsx Outdated
Comment thread packages/eui/src/components/focus_trap/focus_trap.tsx Outdated
@mgadewoll mgadewoll force-pushed the focustrap/migrate-to-function-component branch from d831551 to 70fc730 Compare April 28, 2026 12:20
@elasticmachine
Copy link
Copy Markdown
Collaborator

💚 Build Succeeded

History

cc @mgadewoll

@elasticmachine
Copy link
Copy Markdown
Collaborator

💚 Build Succeeded

History

cc @mgadewoll

@mgadewoll
Copy link
Copy Markdown
Contributor Author

ℹ️ The changes were re-run in Kibana after merging main (on state of 114.3.0 (🟢 CI run)

@tkajtoch tkajtoch self-requested a review April 29, 2026 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[EuiFocusTrap] Migrate from class to function component

3 participants