Skip to content

[EuiPopover] Refactor and remove delay - #10066

Draft
weronikaolejniczak wants to merge 24 commits into
elastic:mainfrom
weronikaolejniczak:feat/improve-popover
Draft

weronikaolejniczak wants to merge 24 commits into
elastic:mainfrom
weronikaolejniczak:feat/improve-popover

Conversation

@weronikaolejniczak

@weronikaolejniczak weronikaolejniczak commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Tip

I recommend reviewing commit-by-commit. They are granular and self-enclosed.

Closes #9484
Closes #10065

This PR:

  • refactors EuiPopover from class component to functional component,
  • removes the open and close delays,
  • updates the transition to be the same as for the EuiToolTip,
  • memoized styles,
  • fixes some bugs:
    • makes the component SSR-safe,
    • makes it reposition without hacks when props change, also fixed a bug reported internally (Slack thread),
    • replaces the unsafe hasOwnProperty with hasOwn,
  • and some small refactors like consolidating 2 unit test suite files.

The reason I'm doing both things at once is so that we only test the component once, not twice. Only the transition removal and animation update require the changelog.

API Changes

component / parent prop / child change description
EuiPopover ref Changed Ref type changed from the EuiPopover class instance to EuiPopoverRef, which exposes positionPopoverFluid. Consumers typing refs as EuiPopover must migrate to EuiPopoverRef.

Screenshots

Transition

Before After
Kapture 2026-09-21 at 14 55 36 Kapture 2026-09-21 at 14 56 28

Flickering reposition

Before After
Kapture 2026-09-21 at 14 56 03 Kapture 2026-09-21 at 14 56 48

Impact Assessment

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

  • 🔴 Breaking changes — EuiPopover refs must use EuiPopoverRef instead of the former class instance type.
  • 💅 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.

The component still supports ref but its type changes from the class instance to EuiPopoverRef, which exposes positionPopoverFluid.

Impact level: 🟡 Moderate
Kibana test PR: elastic/kibana#292619
Kibana prep commit: elastic/kibana@cc88067

Release Readiness

  • Documentation:
  • 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

Test the PR preview vs production.

Verify:

  • there is no behavioral regression,
  • the component is keyboard accessible,
  • there transition is the same as EuiToolTip.

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.

@weronikaolejniczak weronikaolejniczak self-assigned this Sep 21, 2026
@weronikaolejniczak weronikaolejniczak added the ci:regression-integration-test-kibana Run a Regression Integration Test in Kibana against this PR label Sep 21, 2026
@elastic-vault-github-plugin-prod

Copy link
Copy Markdown

📷 2 visual difference(s) found

Look at the visual diff below. If everything is expected, run Approve visual changes to update baselines, re-run the job or make appropriate fixes.

See the visual regression testing wiki for more information.

Expand to review

euicallout (1 difference)

StoryDiff %BeforeAfterDiff
with popover desktop 4.19%

euipopover (1 difference)

StoryDiff %BeforeAfterDiff
panel padding size mobile 4.34%

@elastic-vault-github-plugin-prod

Copy link
Copy Markdown

📷 2 visual difference(s) found

Look at the visual diff below. If everything is expected, run Approve visual changes to update baselines, re-run the job or make appropriate fixes.

See the visual regression testing wiki for more information.

Expand to review

euicallout (1 difference)

StoryDiff %BeforeAfterDiff
with popover desktop 4.19%

euipopover (1 difference)

StoryDiff %BeforeAfterDiff
panel padding size mobile 4.34%

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The ref contract is unsound, and resize and dynamic-positioning behavior can become stale.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 3 Medium severity

Open (3)
What changed in this PR

Refactors EuiPopover to hooks, removes transition delays, improves repositioning, and aligns animation with EuiToolTip.

Changes:

  • Converts the popover to a functional, ref-forwarding component.
  • Introduces immediate opacity animation and expanded repositioning.
  • Consolidates and extends tests while memoizing related styles.
File Description
selectable_template_sitewide_popover.tsx Requires the close callback.
popover.tsx Implements the functional popover.
popover.test.tsx Expands consolidated tests.
popover.stories.tsx Adds interactive coverage.
popover.rtl.test.tsx Removes the redundant suite.
popover_title.tsx Memoizes title styles.
_popover_panel.tsx Refactors panel styling.
_popover_panel.test.tsx Updates panel tests.
_popover_panel.styles.ts Adds the opacity animation.
_popover_panel.test.tsx.snap Updates panel snapshots.
popover_footer.tsx Memoizes footer styles.
_popover_arrow.tsx Memoizes arrow styles.
input_popover.tsx Uses the new ref API.
index.ts Exports the ref type.
popover.test.tsx.snap Updates consolidated snapshots.
popover.rtl.test.tsx.snap Removes obsolete snapshots.
split_button_actions.tsx Uses the props type directly.
10066.md Documents the visual behavior change.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/eui/src/components/popover/popover.tsx Outdated
Comment thread packages/eui/src/components/popover/popover.tsx
Comment thread packages/eui/src/components/popover/popover.tsx
@elastic-vault-github-plugin-prod

Copy link
Copy Markdown

📷 4 visual difference(s) found

Look at the visual diff below. If everything is expected, run Approve visual changes to update baselines, re-run the job or make appropriate fixes.

See the visual regression testing wiki for more information.

Expand to review

euicallout (1 difference)

StoryDiff %BeforeAfterDiff
with popover desktop 4.03%

euipopover (3 differences)

StoryDiff %BeforeAfterDiff
interactive content desktop 94.50%
interactive content mobile 78.39%
panel padding size mobile 3.83%

@elastic-vault-github-plugin-prod

Copy link
Copy Markdown

📷 3 visual difference(s) found

Look at the visual diff below. If everything is expected, run Approve visual changes to update baselines, re-run the job or make appropriate fixes.

See the visual regression testing wiki for more information.

Expand to review

euipopover (3 differences)

StoryDiff %BeforeAfterDiff
interactive content desktop 94.50%
interactive content mobile 78.39%
panel padding size mobile 2.46%

@elastic-vault-github-plugin-prod

Copy link
Copy Markdown

📷 3 visual difference(s) found

Look at the visual diff below. If everything is expected, run Approve visual changes to update baselines, re-run the job or make appropriate fixes.

See the visual regression testing wiki for more information.

Expand to review

euipopover (3 differences)

StoryDiff %BeforeAfterDiff
interactive content desktop 94.50%
interactive content mobile 78.39%
panel padding size mobile 2.46%

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The refactor introduces an undocumented ref API break, unstable imperative handles, and SSR warnings.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity · 2 Medium severity

Open (3)
Resolved since last review (3)

Comment thread packages/eui/src/components/popover/popover.tsx
Comment thread packages/eui/src/components/popover/popover.tsx Outdated
Comment thread packages/eui/src/components/popover/popover.tsx Outdated
@weronikaolejniczak weronikaolejniczak added breaking change PRs with breaking changes. (Don't delete - used for automation) ci:regression-integration-test-kibana Run a Regression Integration Test in Kibana against this PR and removed ci:regression-integration-test-kibana Run a Regression Integration Test in Kibana against this PR labels Sep 21, 2026
@github-actions

Copy link
Copy Markdown

This PR contains breaking changes. The opener of this pull request is asked to perform the following due diligence steps below, to assist EUI in our next Kibana upgrade:

  • If this PR contains prop/API changes:
    • Search through Kibana for <EuiComponent usages (example search)
    • In the PR description or in a PR comment, include a count or list with the number of component usages in Kibana that will need to be updated (if that amount is "none", include that information as well)
  • If this PR contains CSS changes:
    • Search through Kibana for the changed EUI selectors, e.g. .euiComponent (example search)
    • In the PR description or in a PR comment, include a count or list with the number of custom CSS overrides in Kibana that will need to be updated (if that amount is "none", include that information as well)
  • 🔍 Tip: When searching through Kibana, consider excluding **/target, **/*.snap, **/*.storyshot files to reduce noise and only look at source code usages
  • ⚠️ For extremely risky changes, the EUI team should potentially consider the following precautions:
    • Using a pre-release release candidate to test Kibana CI ahead of time
    • Using kibana-a-la-carte for manual QA, and to give other Kibana teams a staging server to quickly test against

@weronikaolejniczak
weronikaolejniczak requested a balanced review from Copilot September 21, 2026 14:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Kibana Regression Integration Test

Status: 💔 Kibana CI failed
Branch: update-dependencies/1790073058
Kibana PR: elastic/kibana#292634

Please check Kibana CI logs for details.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Co-authored-by: Cursor <cursoragent@cursor.com>
weronikaolejniczak and others added 5 commits September 28, 2026 16:32
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@infra-vault-gh-plugin-prod

Copy link
Copy Markdown

💚 Build Succeeded

History

cc @weronikaolejniczak

@infra-vault-gh-plugin-prod

Copy link
Copy Markdown

💚 Build Succeeded

History

cc @weronikaolejniczak

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change PRs with breaking changes. (Don't delete - used for automation) ci:regression-integration-test-kibana Run a Regression Integration Test in Kibana against this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[EuiPopover] Remove opening delay and update animation [EuiPopover] Migrate from class to function component

3 participants