Skip to content
This repository has been archived by the owner on Feb 16, 2021. It is now read-only.

Calculate offset between html and body elements #491

Closed
JamesMGreene opened this issue Sep 12, 2014 · 12 comments · Fixed by #517
Closed

Calculate offset between html and body elements #491

JamesMGreene opened this issue Sep 12, 2014 · 12 comments · Fixed by #517

Comments

@JamesMGreene
Copy link
Member

From a commit comment, @sporkmonger suggested we factor in the use of document.body.getBoundingClientRect() and document.documentElement.getBoundingClientRect() to calculate the top/left offsets between the html and body elements, if any, within the _getDOMObjectPosition internal function:

Should probably incorporate document.body.getBoundingClientRect().top - document.body.parentElement.getBoundingClientRect().top into the calculation here. For sites with position: fixed headers that shift the whole body element down, the offsets seem to end up being wrong for some reason. In those cases, delta between document.body.getBoundingClientRect() and document.body.parentElement.getBoundingClientRect().top has the real top offset.

@JamesMGreene
Copy link
Member Author

May also be related to Issue #423.

@sporkmonger
Copy link

FWIW, I can confirm that we implemented the change on our site and it works as expected now. I just don't know if the change would have side-effects on other page layouts or not.

@vladshcherbin
Copy link

If body has margins and position:relative (for absolute items in it) - swf has wrong position.

Example

@JamesMGreene, is there any possible quick fix ?

@sporkmonger
Copy link

@vladshcherbin The code in that fiddle is minified, but from glancing at it, it appears that it doesn't include the fix I've been referring to. Try adding the fix in and see if it resolves.

@vladshcherbin
Copy link

@sporkmonger the code is minified 2.1.6, without any fixes
here is this example with 2.1.6, but not minified - link
can you please check, how it works and maybe post a fix or a link to an example with your fix ?

@sporkmonger
Copy link

https://github.com/bitpesa/zeroclipboard-rails/commit/63c6ec3034d5352c45073b1ec025c6af7ca63adf

We're using zeroclipboard/zeroclipboard-rails, so we forked that and implemented our fix there even though, if it works, the code should ultimately live in the upstream project.

@vladshcherbin
Copy link

@sporkmonger thanks, it finally works.
Confirmed on the latest version here (2.1.6)

@sporkmonger
Copy link

OK, great, independent confirmation! Anybody want to chime in and confirm that the code doesn't break sites that were already working? That's actually the detail I'm more worried about.

@JamesMGreene
Copy link
Member Author

Anybody want to chime in and confirm that the code doesn't break sites that were already working? That's actually the detail I'm more worried about.

Ditto to that worry! 😛

@JamesMGreene
Copy link
Member Author

Related: #495 ("Create positioning unit tests")

@robdecker
Copy link

@sporkmonger Your fix works great on version 2.1.6. Thanks!

@JamesMGreene
Copy link
Member Author

Yeah, I'll try to get this and a few other fixes rolled into a v2.1.7/v2.2.0 release soon.

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

Successfully merging a pull request may close this issue.

4 participants