Skip to content

Add customizable top and bottom to v2.0.0 #7

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.

Sign up for GitHub

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

Jump to bottom
Open
DanWilkerson opened this issue Sep 11, 2017 · 9 comments
Open

Add customizable top and bottom to v2.0.0 #7

DanWilkerson opened this issue Sep 11, 2017 · 9 comments

Comments

@DanWilkerson
Copy link
Contributor

The top and bottom configurations didn't make it into the initial v2.0.0 release. Basically, we want to support:

var tracker = ScrollTracker({
  top: '.header',
  bottom: '.footer'
});
@aviveros
Copy link

Hi Dan,

I think v2.0 has a problem with document height values. In my site when my page load fires all scroll events together :

25 % at 200px
50% at 400px
75% at 600px
100% at 800px

but my real document height is 3000px but document.body returns 800px.

Seems like in some point of the code, is using document.body or document.documentElement for height value instead of using function docHeight() that fix height values.

This behaviour doesn't happen on version 1.2. ;)

@DanWilkerson
Copy link
Contributor Author

@aviveros can you share the page in question and your config settings? I'll take a look right away.
Thanks!

@aviveros
Copy link

Hi Dan,

Thanks for your message, but my page is not public yet.
Our html is a mess and it could be cause that strange behaviour. I just want to mention to you, because if happens again in other projects

About setting, I just import json file , and use basic settings :

`tracker.on({
percentages: {
every: [25]
}
}, function(evt) {

console.log(evt.data.label); // > "25%"
console.log(evt.data.depth); // > 500

});`

Thanks

@DanWilkerson
Copy link
Contributor Author

If you discover the reason for the issue, I'd love to help (definitely unhappy to hear about this).

@jhpacker
Copy link

jhpacker commented Sep 5, 2018

I seem to be having the same or similar issue with a default installation of 2.0.4 code via GTM container with nothing else in it and no configuration option changes.
It fires all the events at the same time as if the page was very short.
It seems to be determining the current depth correctly, but the overall height of the page incorrectly. Here's an example:
http://recipetest.quantable.com/scroll_fresh.html
thanks!

@DanWilkerson
Copy link
Contributor Author

DanWilkerson commented Sep 6, 2018 •
edited
Loading

This was a fun little rabbit hole!

Long story short, your example fails because it doesn't have a DOCTYPE declaration (e.g. <!DOCTYPE html>. This causes the browser to render the document in backwards compatability mode, aka quirks mode (document.compatMode). This messes with the behavior of Element.clientHeight - it returns the visible height instead of the total height of the element. Add the DOCTYPE and all will be well. Hat tip to @id-pogni!

The larger issue is that this behavior should be accounted for. I'm working on a fix!

@jhpacker
Copy link

jhpacker commented Sep 6, 2018

Great, thanks! Indeed it does work when I specify a doctype.
Unfortunately my real-world example page for deployment still fails in the same way, and it does have a doctype and is in standards mode:
http://recipetest.quantable.com/test.html

@DanWilkerson
Copy link
Contributor Author

So the other way this fails is if body { height: 100% } is in the styles:

screen shot 2018-09-06 at 11 05 43 am

Hoping to fix both! Changing the contextHeight check to use docHeight() instead of document.body.clientHeight might do it, but I've got some tests for nested contexts that are failing at present.

@stephanschubert
Copy link

Any updates on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@stephanschubert @jhpacker @DanWilkerson @aviveros