kuy / redux-tooltip

A tooltip React component for Redux.
130 stars 31 forks source link

Tooltip in relatively positioned element and onMouseEnter using e.target as origin and causes problems #24

Open gtoma4 opened 8 years ago

gtoma4 commented 8 years ago

I am using redux-tooltip in a production application and found a couple of issues that I am working to resolve in preparation for a pull request. I have a that is position:'relative' that contains the tooltip and the origin, as well as an absolutely positioned

that is a dropdown menu. It looks like this when the menu is up. image image

One issue that I think I have resolved is that because the containing the origin and tooltip is position: 'relative', the placement of the tooltip should be calculated based on origin.offsetTop and origin.offsetLeft rather than the origin position in the window. Without doing this, the tooltip is rendered off screen 700px to the left of the 'origin' container rather than the offsetLeft of -6px that will properly position the tooltip.

The other issue I am having that I need your input on is that when mouse enter is from the bottom, the e.target is the with name="info" and ref="wrapper". When mouse enter is from top or either side, e.target is either the or rather than the wrapper. For some reason, when e.target is the svg or path, the values for offsetLeft and offsetTop are undefined, causing the offscreen rendering. To address this, I want to change onMouseEnter in origin.js so that it sets the origin to this.refs.wrapper rather than e.target. This seems to clear up the tooltip positioning issue.

Some questions for you: Is there a reason to set origin to e.target all the time? When does the el prop get set versus the origin property? Is there another way that I should be dealing with tooltips that are contained in relatively positioned elements?

Thanks.

Tom Athens

For reference, here is the react rendered elements (via chrome react dev tool) : image

kuy commented 8 years ago

@gtoma4 Thanks for working on this issue! First of all, I have to say that your use-case ( contains ) is unexpected usage. Overriding onMouseEnter property and passing an element or a position is the simplest way. To set origin to e.target is just a default of my use-case.

In redux-tooltip's examples, there are no use-cases that contain your issues. Can you PR or gist a minimum example code that reproduces your issue? I'd like to work with you to resolve this issue and find the best way.

And I'm welcome your work-in-progress PR.

gtoma4 commented 8 years ago

@kuy - I have created an example in my forked version of redux-tooltips. If you look at my github (github.com/gtoma4/redux-tooltip) you'll see it in examples/contained. This code currently has my 'fixes' running. To see the container issue, comment out lines 52 - 57 in utils.js.
The change I made is to look for origin.offsetTop and origin.offsetLeft, and replace pos.left and pos.top with those values if they are not undefined.

Another use-case I discovered an issue with is if an origin is absolutely positioned relative to the right side of the window, resizing the screen does not trigger a recomputing of the origin position. I made a change to tooltip.js in the componentWillReceiveProps() function that checks if the origin is in the same position as it was the last time, and will call updatePosition if not. This is another issue I am planning a pull request. I included an example of this in the contained example. (Note: when preparing the example, I initially tried with a

, but the issue was not replicated. I originally discovered it with an , so that's what I put into the demo.) To see the issue, comment out line 9 in tooltip.js.

Finally another question for you. I tried overriding the onMouseEnter function, however it did not work because I need access to the 'show' action and I don't know how I would get it from outside the redux-tooltip component. I am still working on a sample that has different origin depending on the direction of mouse enter. I'll let you know when I get it reproduced.

I look forward to resolving these issues in the best way. Let me know if you have any questions.

Thanks for your help!

Tom

gtoma4 commented 8 years ago

@yuk I just updated the demo with an example of how setting the origin to e.target by default causes issue with tooltip positioning depending on direction of mouse enter. Thanks again for your assistance.

Tom

kuy commented 8 years ago

@gtoma4 Sorry, now I don't have enough time to review this and other your issues immediately. Please wait.