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

[tooltip] add offset props to <TooltipWithBounds /> #193

Merged
merged 1 commit into from
Nov 9, 2017

Conversation

williaster
Copy link
Collaborator

🚀 Enhancements

The goal of this PR is to fix issues like this where the tooltip overlaps the point:

To do so I added optional leftOffset and topOffset props to the <TooltipWithBounds /> which offset the final left and top positions depending on the boundingClientRects it receives from withBoundingRects:

  • if the tooltip will be right of the pointer, it it shifted +leftOffset
  • if the tooltip will be left of the pointer, it it shifted -leftOffset
  • if the tooltip will be below of the pointer, it it shifted +topOffset
  • if the tooltip will be above of the pointer, it it shifted -topOffset

I also considered creating computeLeft({ left, rect, parentRect }) and computeTop({ top, rect, parentRect }) props that allow the user to fully override the calculation but decided this was a little simpler.

Let me know if you have thoughts!

Tested functionally:

@hshoff
Copy link
Member

hshoff commented Nov 9, 2017

Makes sense. LGTM

@hshoff hshoff added this to the v0.0.147 milestone Nov 9, 2017
@hshoff hshoff merged commit a590ef3 into airbnb:master Nov 9, 2017
@techniq
Copy link
Collaborator

techniq commented Nov 9, 2017

I was just admiring Rechart's tooltip today ironically and how they keep the tooltip within the bounds of the chart and not block the point. They appear to be using a single offset (10), but the same approach. LGTM too

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.

3 participants