Skip to content

[EuiFlyout] Fix flaky return focus and child flyouts closing their parent flyouts #8878

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

Merged
merged 6 commits into from
Jul 21, 2025

Conversation

mgadewoll
Copy link
Contributor

@mgadewoll mgadewoll commented Jul 17, 2025

Summary

closes #8867

This PR aims to solve a flaky behavior that we noticed as part of the EUI upgrade for 105.0.0 in Kibana (PR). The flaky behavior was noticed on failing cypress tests (build) where a click event after closing a flyout would not target the correct element anymore.

Additionally, the flyout changes also affected another test and made it flaky. This was captured here. Kibana updated the test for now in this PR.

expected unexpected
Screenshot 2025-07-10 at 16 55 07 Screenshot 2025-07-10 at 16 39 29

The issue was tracked down to the changed returnFocus behavior on EuiFlyout (code).

The manual handling of the return focus is not needed. the FocusTrap handles the focus already generally correctly and internally with applied delay (code).

Additional changes

  • closing child flyouts independently from its parent flyout on Escape keypress
  • removing function example stories from VRT

Why are we making this change?

Fixing flaky closing behavior of flyouts that affected focus behavior of other interactions.

Screenshots

Impact to users

⚠️ We previously added a temporary fix for the flaky tests here by adding { force: true} to the click event. This needs to be removed once this change is released.

⚠️ Another flaky test was adjusted here and we should revert this change.

QA

The fix for the flaky behavior was tested in Kibana, the CI passed with the update applied and { force: click } removed.

  • (optional) manually run the affected Kibana cypress test locally (a few times) and verify it passes
    • checkout testing branch eui/324-input-visual-refresh-qa
    • run yarn kbn bootstrap
    • in x-pack/test/security_solution_cypress run yarn cypress:open:serverless --spec './cypress/e2e/detection_response/detection_engine/exceptions/**/*.cy.ts' and select the test auto_populate_with_alert_data.cy.ts to run

ℹ️ Storybook

  • verify that closing child flyouts with Escape key closes the child flyout only
  • verify that closing parent EuiFlyout and child flyouts returns focus correctly
    • if the flyout is open on mount it returns focus on close to the body
    • when opening a flyout from a trigger button, the flyout returns the focus to the trigger button
    • closing a child flyout returns the focus to the parent flyout

General checklist

  • Browser QA
    • Checked in both light and dark modes
    • Checked in both MacOS and Windows high contrast modes
    • Checked in mobile
    • Checked in Chrome, Safari, Edge, and Firefox
    • Checked for accessibility including keyboard-only and screenreader modes
  • Docs site QA
  • Code quality checklist
  • Release checklist
    • A changelog entry exists and is marked appropriately.
    • If applicable, added the breaking change issue label (and filled out the breaking change checklist)
  • Designer checklist
    • If applicable, file an issue to update EUI's Figma library with any corresponding UI changes. (This is an internal repo, if you are external to Elastic, ask a maintainer to submit this request)

@mgadewoll mgadewoll self-assigned this Jul 17, 2025
@mgadewoll mgadewoll force-pushed the flyout/8867-fix-return-focus branch from 491d59e to 74c66eb Compare July 17, 2025 13:11
@mgadewoll mgadewoll force-pushed the flyout/8867-fix-return-focus branch from 74c66eb to c7e7312 Compare July 17, 2025 15:19
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @mgadewoll

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @mgadewoll

@mgadewoll mgadewoll marked this pull request as ready for review July 18, 2025 08:48
@mgadewoll mgadewoll requested a review from a team as a code owner July 18, 2025 08:48
Copy link
Contributor

@acstll acstll left a comment

Choose a reason for hiding this comment

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

🟢 Working as expected and the changes look good. I followed the QA guidelines comprehensively. Cypress test passed 3 times in a row. Flyout behavior is perfect, including the Escape key fix. (Thanks for taking care of this one!)

@mgadewoll mgadewoll merged commit f1e538e into elastic:main Jul 21, 2025
6 checks passed
weronikaolejniczak added a commit to elastic/kibana that referenced this pull request Jul 23, 2025
`@elastic/eui`: `106.0.0` ⏩ `106.1.0`
`@elastic/eslint-plugin-eui`: `2.1.0` ⏩ `2.2.1`

[Questions? Please see our Kibana upgrade
FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)

New props and defaults have been added to `EuiFlyout`, including
`includeSelectorInFocusTrap`, plus new global CSS variables for push
mode offset control. Additionally, we introduced a small spacing fix for
invalid `EuiDualRange` append component and added support for custom
styles on `EuiTreeView` nodes via the css key.

:warning: **React 16 is now deprecated:** EUI no longer tests against it
or fixes issues specific to it.

## Changes

- I removed forced click in:
`x-pack/test/security_solution_cypress/cypress/tasks/rule_details.ts`
- I removed `{enter}` in:
`x-pack/test/security_solution_cypress/cypress/tasks/common/rule_actions.ts`
(see #228517)

## Package updates

### `@elastic/eui`

#### [`v106.1.0`](https://github.com/elastic/eui/releases/v106.1.0)

- Added `--euiPushFlyoutOffsetInlineStart` and
`--euiPushFlyoutOffsetInlineEnd` global CSS variables set by the
`EuiFlyout` in `push` mode.
([#8872](elastic/eui#8872))
- Reduced the `min-width` for inputs in `EuiRange` and `EuiDualRange`
([#8866](elastic/eui#8866))
- Added `includeSelectorInFocusTrap` prop for `EuiFlyout`
([#8849](elastic/eui#8849))
- Added component defaults for `EuiFlyout` that include
`includeSelectorInFocusTrap` and `includeFixedHeadersInFocusTrap`
([#8849](elastic/eui#8849))

**Bug fixes**

- Fixed flaky manual return focus behavior on `EuiFlyout` by relying on
`FocusTrap` returning focus instead
([#8878](elastic/eui#8878))
- Fixed `EuiFlyoutChild` closing its parent `EuiFlyout` on `Escape`
keypress ([#8878](elastic/eui#8878))
- Fixed support for `css` key in items object passed to `EuiTreeView`
([#8864](elastic/eui#8864))

**Deprecations**

- Deprecated support for React 16. Please update to a modern version of
React or stay on EUI version 106.0.0 if you can't switch right now.
([#8868](elastic/eui#8868))
  
### `@elastic/eslint-plugin-eui`

####
[`v2.2.1`](https://www.npmjs.com/package/@elastic/eslint-plugin-eui/v/2.2.1)

**Bug fixes**

- Removed `no-css_color` entry which mapped to a duplicate
`no-css-color` rule ([#8888](elastic/eui#8888))

####
[`v2.2.0`](https://www.npmjs.com/package/@elastic/eslint-plugin-eui/v/2.2.0)

- Added new `prefer-eui-icon-tip` rule.
([#8877](elastic/eui#8877))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove "force click" in Cypress test in Kibana
3 participants