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

Return tooltip to original position when possible #207

Merged
merged 1 commit into from
Apr 11, 2018
Merged

Return tooltip to original position when possible #207

merged 1 commit into from
Apr 11, 2018

Conversation

gris-martin
Copy link
Contributor

No description provided.

@wwayne
Copy link
Collaborator

wwayne commented Oct 13, 2016

I'm not sure we should add this option, in which situation you need such function?

@gris-martin
Copy link
Contributor Author

I'm making a playback slider for a sort of video player, which uses this tooltip to display the time at which the mouse hovers (compare to YouTube). Since you will want to see as much of the slider as possible when hovering, and also be able to move the mouse continuously along the track, it's good if the tooltip stays above the mouse as much as possible. But I'll find another solution if this doesn't fit with your design goals :)

@wwayne
Copy link
Collaborator

wwayne commented Oct 14, 2016

Okay I just checked Youtube slider, and I suppose that you are adding an option to keep the tooltip alway in a same position? no matter if it is out of the screen or not?

@gris-martin
Copy link
Contributor Author

Sorry for being unclear before. What I mean is this:

This is what the tooltip looks like when hovering over the middle of the slider, and what I want it to look like:
tooltip_normal

This is what the tooltip looks like when close to the right edge of the browser, which is fine too:
tooltip_edge

However, after being close to the edge, the tooltip keeps the left orientation until you move the mouse away from the slider, resulting in this if I move the mouse away from the right edge:
tooltip_after_edge

The commit makes the tooltip go back to its original position (like in the first image) after being close to the edge. The "original position" is specified by the 'place' prop during creation, or defaults to 'top', so the API is not affected.

@wwayne
Copy link
Collaborator

wwayne commented Oct 14, 2016

Hm, thanks for the detailed explanation, I'm clear now.

But I don't think it is a good idea to add an new option for reverting the tooltip to its origin, I suppose it would be better if the tooltip can revert to its origin automatically instead of keeping new position? I'm not sure how to implement this, but I will take investigation later.

@gris-martin
Copy link
Contributor Author

It isn't supposed to be an option, but rather exactly what you describe: that the tooltip reverts to its original orientation automatically. But I'm happy that you'll look into it, and I'm sure you'll think of a good solution :)

@huumanoid
Copy link
Contributor

huumanoid commented Mar 20, 2017

Hey guys I'm really impressed by this fix, I think this fix is very important.

Here is another approach to achieve desired behavior, please, take a look!
https://github.com/huumanoid/react-tooltip/blob/fix-original-position/src/index.js#L385

Commit: 948e5c5

@huumanoid
Copy link
Contributor

After a while, I can't said why my solution is better that @hassanbot's one. @wwayne, please clarify what do you want from a good solution?

it seems that getPosition can't automatically switch to the origin because it needs information about the origin. So, anyway we have to handle somehow that tooltip has the original place. @hassanbot handles it in the getPosition, and I'm handle it in the updatePosition.

@huumanoid huumanoid self-requested a review March 23, 2017 22:02
@aronhelser
Copy link
Collaborator

@hassanbot @huumanoid do you have a preference here? I've tested hassanbot's solution, and it works, and it appears a bit simpler than huumanoid's solution. I'm inclined to merge this PR.

@wwayne
Copy link
Collaborator

wwayne commented Apr 10, 2018

@aronhelser Please go ahead if it is good for you, thanks 👍

@aronhelser aronhelser merged commit e5f3329 into ReactTooltip:master Apr 11, 2018
@smakosh
Copy link

smakosh commented Jun 1, 2018

this doesn't actually work as the tooltip changes it's position from bottom to left

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.

5 participants