Skip to content

Update isVisible#178

Open
SalvoDiFede wants to merge 1 commit into
mattdelacdev:masterfrom
SalvoDiFede:patch-1
Open

Update isVisible#178
SalvoDiFede wants to merge 1 commit into
mattdelacdev:masterfrom
SalvoDiFede:patch-1

Conversation

@SalvoDiFede

Copy link
Copy Markdown

Using box.getBoundingClientRect() for better compatibility, please check and test before commit to main branch :-)

Using box.getBoundingClientRect() for better compatibility, please check and test before commit to main branch :-)
@attilaolah

Copy link
Copy Markdown
Collaborator

This looks very cool, but apparently the tests don't like it. I'll dig into that and see why.

Maybe we can also kill that funky offsetTop method then.

@attilaolah attilaolah self-assigned this Apr 15, 2015
@jlmakes

jlmakes commented Dec 20, 2015

Copy link
Copy Markdown

I tried implementing element.getBoundingClientRect() into ScrollReveal v3.0.0, and I can confirm it's likely not suitable for our scrolling animation libraries—the visibility factors in CSS transformations, whereas element offset does not.

This is most noticeable with large translations… where in some cases, the element will either be visible/invisible at the same time (based on the time at which is checked during animation), or never be visible at all.

A simple example can be seen here ScrollReveal Issue #193

Hope this helps! 🙇

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