Skip to content
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

fix: Insight tooltip overscrolling issue #17906

Merged
merged 1 commit into from
Oct 11, 2023
Merged

Conversation

benjackwhite
Copy link
Contributor

Problem

The tooltip can get rendered offscreen and because it is a child of the root it causes a big over-scroll.

This is partly because we changed the scroll so that the root is no longer a truly scrollable element but if a hover element is placed off screen that triggers the scroll

Changes

  • Don't display the tooltip until it is first actually used (stops it flowing the DOM)
  • Fix the bounds of the problematic tooltip (so that it doesn't suddenly jump from off screen when it gets displayed)

Really that tooltip needs a re-think. Lots of duplicated code everywhere but this at least fixes the issue for now

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

How did you test this code?

Copy link
Contributor

@thmsobrmlr thmsobrmlr left a comment

Choose a reason for hiding this comment

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

I could not replicate the original issue locally (seems there are some preconditions that I'm unaware of). Changes seem fine and I trust your reasoning.

@benjackwhite
Copy link
Contributor Author

I could not replicate the original issue locally (seems there are some preconditions that I'm unaware of). Changes seem fine and I trust your reasoning.

You need to add a "Big number" insight to a dashboard far enough down that it is off the screen. Thats how I recreated it

@benjackwhite benjackwhite merged commit fab25ed into master Oct 11, 2023
73 checks passed
@benjackwhite benjackwhite deleted the fix/tooltip-overscroll branch October 11, 2023 10:19
daibhin pushed a commit that referenced this pull request Oct 23, 2023
thmsobrmlr added a commit that referenced this pull request Dec 21, 2023
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.

2 participants