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: Rely on getBoundingClientRect for width and height #14599

Merged
merged 1 commit into from
Sep 25, 2014

Conversation

hnrch02
Copy link
Collaborator

@hnrch02 hnrch02 commented Sep 11, 2014

Fixes #14553.

@hnrch02 hnrch02 added the js label Sep 11, 2014
@hnrch02 hnrch02 added this to the v3.2.1 milestone Sep 11, 2014
@cvrebert
Copy link
Collaborator

Well, I'm glad this lets us get rid of the weird SVG special-casing.

Should we add a regression unit test for #14553, since none of the existing tests caught it?

@hnrch02
Copy link
Collaborator Author

hnrch02 commented Sep 11, 2014

I was working on one, but I abandoned it because of IE8. I'll look into it more later. If you want to do it, please go ahead.

@fat
Copy link
Member

fat commented Sep 14, 2014

nice work

@hnrch02
Copy link
Collaborator Author

hnrch02 commented Sep 24, 2014

@cvrebert Any idea on how to do a cross-browser compatible unit test? rotate could work but would require a lot of CSS to work properly in all browsers.

@cvrebert
Copy link
Collaborator

@hnrch02 My thought was that we should just disable the test in incompatible browsers (which would be only IE8?), like we already do in the SVG tooltip test.

@hnrch02
Copy link
Collaborator Author

hnrch02 commented Sep 24, 2014

@cvrebert Think guarding it by

var style = document.documentElement.style
if ('transform' in style || 'webkitTransform' in style || 'msTransform' in style)

would suffice?

@cvrebert
Copy link
Collaborator

Sounds good to me.

@hnrch02
Copy link
Collaborator Author

hnrch02 commented Sep 24, 2014

Working on it.

@hnrch02 hnrch02 force-pushed the tooltip-outerDims-only-when-body branch from 1508b11 to 5c3fdb8 Compare September 24, 2014 23:34
@hnrch02
Copy link
Collaborator Author

hnrch02 commented Sep 24, 2014

Added a unit test.

@hnrch02 hnrch02 force-pushed the tooltip-outerDims-only-when-body branch from 5c3fdb8 to 24ae068 Compare September 24, 2014 23:35
@hnrch02
Copy link
Collaborator Author

hnrch02 commented Sep 24, 2014

@cvrebert LGTY?

@hnrch02 hnrch02 changed the title Tooltip: Rely ongetBoundingClientRect for width and height Tooltip: Rely on getBoundingClientRect for width and height Sep 24, 2014
@cvrebert
Copy link
Collaborator

Yep.

hnrch02 added a commit that referenced this pull request Sep 25, 2014
Tooltip: Rely on `getBoundingClientRect` for `width` and `height`
@hnrch02 hnrch02 merged commit 61a4fee into master Sep 25, 2014
@hnrch02 hnrch02 deleted the tooltip-outerDims-only-when-body branch September 25, 2014 00:08
@cvrebert cvrebert mentioned this pull request Sep 25, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tooltip popover offset when rotate
3 participants