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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sticky element rely on viewport/contentHeight, even if it's out of viewport #63

Closed
mahboob-awan opened this issue Apr 6, 2017 · 9 comments

Comments

@mahboob-awan
Copy link

Really appreciate Your efforts and a nice approach to manage scroll listener. 馃憤

Ref - https://github.com/stutrek/scrollMonitor/blob/master/src/container.js#L168
As it rely on viewport/contentHeight, it cause an issue when we have multiple containers with different heights, So sticky element does not "recalculateLocation" even it's out of viewport.

I tried to explain the problem with a JSFiddle Demo

This Problem fix, if i remove remove a condition but that maybe a wrong fix.

So is this a bug or i am missing something?

Kindly ask if You need more detail. Thanks

@stutrek
Copy link
Owner

stutrek commented Apr 25, 2017

Thanks for using this!

I think you have a logic problem here. It's surprisingly hard to get everything right when using the specific events, I usually do an elementWatcher.stateChange and check the boolean properties. You'll probably need to check elementWatcher.isBelowViewport.

You may need to use elementWatcher.lock or elementWatcher.recalculate at some point too.

@stutrek stutrek closed this as completed Apr 25, 2017
@mahboob-awan
Copy link
Author

Problem is
1 - sticky button work fine initially
2 - but after load more contents, problem start appearing
3 - Sticky button still considering old position.

Can You please integrate Your suggestion within JSFiddle Demo

Thanks

@stutrek
Copy link
Owner

stutrek commented Jun 8, 2017

You might need to use scrollMonitor.recalculateLocations() or watcher.recalculateLocation() after changing the DOM.

@mahboob-awan
Copy link
Author

Nice that scrollMonitor.recalculateLocations() or watcher.recalculateLocation() fixed the case mention in previous GIF but it does not restore sticky position after hide "more contents".

Steps to reproduce it:
1 - initially it stick properly
2 - After adding scrollMonitor.recalculateLocations(), it sticky properly when load more contents
3 - But if hide more contents, sticky element does not restore it's position.
source(JSFiddle)

@stutrek
Copy link
Owner

stutrek commented Jun 14, 2017

Call recalculateLocations again.

@mahboob-awan
Copy link
Author

Kindly check current code that execute when user press "Toggle more contents"[BUTTON]

 button.onclick = function(){
   document.getElementById("morecontents").classList.toggle('hide');
   scrollMonitor.recalculateLocations();//Sol#1
    //elementWatcher.recalculateLocation()//Sol#2 - Same result as Sol#1
  }

but duplicating same call(again) also did not work:

 button.onclick = function(){
   document.getElementById("morecontents").classList.toggle('hide');
   scrollMonitor.recalculateLocations();//Sol#1
   scrollMonitor.recalculateLocations();
    //elementWatcher.recalculateLocation()//Sol#2 - Same result as Sol#1
  }

@stutrek
Copy link
Owner

stutrek commented Jun 15, 2017

You have to call it when you collapse the content too.

@mahboob-awan
Copy link
Author

But i am calling it, as same function is used to expand & collapse using .toggle('hide');

 button.onclick = function(){
   document.getElementById("morecontents").classList.toggle('hide');
   scrollMonitor.recalculateLocations();//Sol#1
    //elementWatcher.recalculateLocation()//Sol#2 - Same result as Sol#1
  }

@stutrek
Copy link
Owner

stutrek commented Jun 16, 2017

I see. When it's recalculated it's already in the viewport because it has fixed positioning. Try watching the content rather than the button you're changing.

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

No branches or pull requests

2 participants