stutrek / scrollmonitor

A simple and fast API to monitor elements as you scroll
MIT License
3.3k stars 244 forks source link

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

Closed mahboob-awan closed 7 years ago

mahboob-awan commented 7 years ago

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 commented 7 years ago

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.

mahboob-awan commented 7 years ago

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 commented 7 years ago

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

mahboob-awan commented 7 years ago

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 commented 7 years ago

Call recalculateLocations again.

mahboob-awan commented 7 years ago

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 commented 7 years ago

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

mahboob-awan commented 7 years ago

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 commented 7 years ago

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.