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

Tooltips With Bounds considering also screen bounds (window frame) #240

Merged
merged 1 commit into from
Feb 11, 2018

Conversation

manuelrocha88
Copy link

💥 Breaking Changes

  • N/A

🚀 Enhancements

  • now it consider window frame. If chart is only half visible the tooltip will adjust to window frame and not get cut.

📝 Documentation

  • N/A

🐛 Bug Fix

  • N/A

🏠 Internal

  • N/A

@hshoff
Copy link
Member

hshoff commented Feb 7, 2018

Thanks for this @manuelrocha88. I want to take some time to understand the problem a bit more before I merge this. First thought is it makes sense. Just want to be safe and make sure I fully grasp why this is a win for everyone 😄

@manuelrocha88
Copy link
Author

manuelrocha88 commented Feb 9, 2018

@hshoff I give you a shortcut on the issue analysis.

From @vx/tooltip package if we make only use of the Tooltip component we get something like this:

captura de ecra 2018-02-09 as 15 11 53

As you can see the tooltip gets out of the chart bounds and get cut.

If we replace the Tooltip component by the TooltipWithBounds, we get a different, and success, reposition of the Tooltip and it's inside the chart bounds, as we can see at this picture:

captura de ecra 2018-02-09 as 15 12 17

But, this TooltipWithBounds package is only considering the chart bounds. What If the chart has part of it outside of the bounds of the frame screen (A small screen?!)?

You get something like this:

captura de ecra 2018-02-09 as 15 12 57

If you see the bubble is half visible only. When I make my mouse over it the Tooltip gets cut because is still inside chart bounds but outside frame screen bounds.

With the change I have on PR you will get something like this:

captura de ecra 2018-02-09 as 15 13 28

As you can check the Tooltip now is inside the bounds of chart and frame screen.

@hshoff
Copy link
Member

hshoff commented Feb 11, 2018

Sounds good

@hshoff hshoff added this to the v0.0.158 milestone Feb 11, 2018
@hshoff hshoff merged commit 6f303d0 into airbnb:master Feb 11, 2018
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