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

Make getPosition() calculate offsets correctly for svg elements #366

Merged
merged 2 commits into from
May 2, 2018

Conversation

P0lip
Copy link
Contributor

@P0lip P0lip commented Feb 13, 2018

Current implementation of getPosition is broken for children of svg elements.
Their clientHeight and clientWidth values are usually (or always, not sure, though) equal to 0, therefore calculated offsets are wrong.
This may be a common issue for tooltips whose tips are some sort of svg child elements, like polygon, rect, line, circle etc. In such cases, their placement is usually wrong.

@austil
Copy link
Contributor

austil commented Feb 28, 2018

Hey, I runned into the exact same problem at work (tooltip on svg elements).
I cherry picked your commit and it work perfectly. Thanks a lot !

@aronhelser
Copy link
Collaborator

This looks very reasonable.
@P0lip could you comment on what differences we might see using getBoundingClientRect() vs clientWidth,clientHeight? If there are differences, and we can test, I'm fine merging.

I'd love to see an example added that used SVG!

@austil
Copy link
Contributor

austil commented May 2, 2018

Check this https://bugzilla.mozilla.org/show_bug.cgi?id=874811

tldr :

SVG elements don't have a CSS layout box
The clientTop, clientLeft, clientWidth, and clientHeight attributes must return zero if the element does not have any associated CSS layout box

use getBoundingClientRect

PS : nice to see a maintainer back on this project

@aronhelser
Copy link
Collaborator

aronhelser commented May 2, 2018

Looking at https://developer.mozilla.org/en-US/docs/Web/API/Element/clientWidth, it seems getBoundingClientRect() will return fractional values. Otherwise it seems equivalent, and works for SVG. Have you seen any ill-effects because of fractional values?

Update: I tested the examples, and can observe no differences.

@P0lip
Copy link
Contributor Author

P0lip commented May 2, 2018

Yes, getBoundingClientRect() returns fractional values, yet the general result doesn't differ from clientWidth, clientHeight. Well, at least, I can't seem to find any change.

Regarding example - I will try to add some in a couple of minutes.

@aronhelser aronhelser merged commit 1ce0a12 into ReactTooltip:master May 2, 2018
@aronhelser
Copy link
Collaborator

aronhelser commented May 2, 2018

Oops, sorry, I missed your comment before I merged. Feel free to submit another PR, I'll get it right in.
If you start your commit message like:

fix(examples): add SVG example

then I can test auto-publish of the examples 😄

@aronhelser
Copy link
Collaborator

🎉 This PR is included in version 3.5.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants