Skip to content

Conversation

@weronikaolejniczak
Copy link
Contributor

@weronikaolejniczak weronikaolejniczak commented Oct 27, 2025

Summary

Add both EuiPopover and EuiToolTip's repositionOnScroll to componentDefaults.

Why are we making this change?

Resolves #8984

We had a short conversation about this with Anton and we think that the componentDefaults is the first nice step.

What repositionOnScroll does is it fixes EuiPopover / EuiToolTip positioning on anchors that are position: fixed. That is not always the behavior that we want. It adds listeners and recalculates the position so it isn't without a cost either.

That being said, we have a logic in the tooltip that makes only one be displayed at a time. And usually, popovers do not display at once (or display briefly at once).

The short-term proposal would be:

  1. Switch the repositionOnScroll to true by default.
  2. Build the @elastic/eui locally.
  3. Test within Kibana to see if there is a significant performance overhead or behavior issues (intuition tells me - there won't be).
  4. Add EuiPopover and EuiToolTip repositionOnScroll to componentDefaults.
  5. Release and update in Kibana.
  6. Using EuiProvider, pass the feature flag to repositionOnScroll in componentDefaults. This way, we will have enough time to test the result while not causing potential regression to existing functionality.

Estimation: up to 2 days

Because repositionOnScroll affects the component behavior I would not do it globally within EUI. That being said, through componentDefaults that we hook up to the feature flag for the new grid layout, that might be good!

The long-term proposal would be:

  1. Redoing our overlays to not be portaled.
  2. Provide better composition (likely, not wrapping the triggers).

We are making this change for the grid layout initiative in Kibana.

See more details here: elastic/kibana#240882

Screenshots #

EuiPopover

Kapture.2025-10-27.at.18.01.06.mp4

EuiToolTip

Kapture.2025-10-27.at.18.50.02.mp4

Impact to users

🟢 This is not a breaking change. It's an enhancement.

QA

Specific checklist

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)
    • If the changes unblock an issue in a different repo, smoke tested carefully (see Testing EUI features in Kibana ahead of time)
  • 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)

@weronikaolejniczak weronikaolejniczak self-assigned this Oct 27, 2025
@weronikaolejniczak weronikaolejniczak force-pushed the feat/reposition-on-scroll-component-defaults branch from 24361f9 to f95d46b Compare October 27, 2025 17:44
@weronikaolejniczak weronikaolejniczak force-pushed the feat/reposition-on-scroll-component-defaults branch from f95d46b to 0664662 Compare October 27, 2025 17:55
@weronikaolejniczak weronikaolejniczak force-pushed the feat/reposition-on-scroll-component-defaults branch from 0664662 to 3fa4cf7 Compare October 27, 2025 18:18
@weronikaolejniczak weronikaolejniczak marked this pull request as ready for review October 27, 2025 18:18
@weronikaolejniczak weronikaolejniczak requested a review from a team as a code owner October 27, 2025 18:18
@weronikaolejniczak weronikaolejniczak force-pushed the feat/reposition-on-scroll-component-defaults branch from 3fa4cf7 to b158a7e Compare October 27, 2025 18:35
@acstll acstll requested review from acstll and removed request for acstll October 28, 2025 09:47
@Dosant
Copy link
Contributor

Dosant commented Oct 28, 2025

I tested the change in Kibana, and it seems to be working well! 👍

@weronikaolejniczak
Copy link
Contributor Author

@mgadewoll ready for re-review 🙏🏻

@weronikaolejniczak
Copy link
Contributor Author

@mgadewoll thank you for all the suggestions to make the implementation cleaner and improve test quality 🙏🏻 I applied all of them and I re-tested but I'd appreciate a test from you as well. Let me know if you have any more thoughts!

Copy link
Contributor

@mgadewoll mgadewoll left a comment

Choose a reason for hiding this comment

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

🟢 The changes LGTM and the usage in popover and tooltip work as expected. Nice work! 🎉

⏰ Friendly reminder to revert the test commits.

@weronikaolejniczak weronikaolejniczak force-pushed the feat/reposition-on-scroll-component-defaults branch 3 times, most recently from 2db21e8 to 9594536 Compare October 31, 2025 12:11
@weronikaolejniczak weronikaolejniczak force-pushed the feat/reposition-on-scroll-component-defaults branch from 9594536 to 2ed993e Compare October 31, 2025 18:14
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @weronikaolejniczak

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @weronikaolejniczak

@weronikaolejniczak weronikaolejniczak merged commit 4b8099c into elastic:main Nov 5, 2025
7 checks passed
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.

[EuiPopover] Expose repositionOnScroll in componentDefaults

5 participants