Skip to content

Conversation

@woodcoder
Copy link

This change totals up the offsetLeft and offsetTops of the target element and all its offsetParents to hopefully correctly locate elements that are inside fixed positioned elements, for example.

@oliveirafilipe
Copy link
Collaborator

@woodcoder Thank you Andy! We will make some tests and (hopefully) in few days give you a feedback.
ping @menosprezzi

@menosprezzi
Copy link
Collaborator

@woodcoder Hello! Nice that you want work with us on it!
Analysing your PR here. Do you have tested with our dist/demo.html code? In that HTML, we test the many ways that we use the plugin.
We'll work on an E2E tests soon.

@menosprezzi
Copy link
Collaborator

In the second case of demo.html, it causes:
screen shot 2018-03-24 at 18 58 23

@woodcoder
Copy link
Author

Hi @menosprezzi

Thanks for the library!

I'm sorry I didn't see the test page -- I've now looked and you're right the last commit fails in the way you show. Interestingly my original PR commit 6a1c0a1 is fine with these tests.

My final commit was to fix an issue with a translate3d transformed element in my local project, but it's obviously breaking other situations so I've reverted that commit.

@woodcoder
Copy link
Author

I added a test that's similar to the situation in my local project and pushed a fix that seems to work for this and all the other tests (I wasn't taking into account the scrollY position).

@menosprezzi
Copy link
Collaborator

Wow. Right, we have problems. In the stable v2 at the repo, the demo.html runs fine in all the cases but not in elements positioned relatively in fixed parents, I understand it now! @woodcoder I'll analyse another way to calc the Y position with your points.

menosprezzi added a commit that referenced this pull request Mar 26, 2018
…sition when targeting elements positioned relatively to fixed parents.
@menosprezzi menosprezzi self-assigned this Mar 26, 2018
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.

3 participants