angular-ui / ui-scrollpoint

Add a 'ui-scrollpoint' class to elements when the page scrolls past them.
https://htmlpreview.github.io/?https://github.com/angular-ui/ui-scrollpoint/master/demo/index.html
MIT License
28 stars 25 forks source link

Regression bug: scrollpoint with fixed position broken as of 1.2.1 #12

Closed karptonite closed 8 years ago

karptonite commented 8 years ago

ui-scrollpoint no longer works with the style position:fixed for the ui-scrollpoint class.

See these two Plunkers: Working, Version 1.1.0 Try scrolling up and down.

Now try it with the latest version Failing, Version 1.2.1

I haven't dug into exactly where the problem lies, but I thought it was worth reporting it now, with an example.

Is this perhaps a CSS problem? Using position: fixed is a necessity, I think, if you want to affix an element relative to the window, when it may be inside some other non-static ancestor.

karptonite commented 8 years ago

Looks like 1.2.0 works: 1.2.0 test

So this is definitely a bug with the most recent 1.2.1 commits by @plong0 . @plong0 , can you take a look when you have a chance?

plong0 commented 8 years ago

Hey thanks for reporting this! I do see what you mean and I think this is a really important use case for ui-scrollpoint. It's something I have dug into, so here are a couple notes on the issue:

  1. I have found that position: fixed; will not work reliably within a ui-scrollpoint-target. It does seem to work in Chrome, but digging into it on SO, I learned that this is an incorrect implementation of the CSS spec as position: fixed; is meant to always be relative to the window, not the parent element.
  2. When an element is fixed position, by logical design there is no way for the scroll offset to pass it. The solution used in 1.2.0 is to set the fixLimit variable, which essentially caches the element's position while the ui-srollpoint class is not applied. I have pushed a fix to my branch that does this. Plunkr: http://plnkr.co/edit/KmMIswpxOuXKSt0zrklN?p=preview
  3. Keep in mind that changing the positioning of the element removes it from the layout, so neighbouring elements would also change position.

RE: Notes 1 & 3 ... I have developed a ui-scrollpoint-pin directive that offers these enhancements:

My question to the community about this feature is whether it is within the scope of ui-scrollpoint or should it be its own project?

Although there are some "sticky" modules out there, I didn't find any that use ui-scrollpoint, and I do see a DRY benefit to using ui-scrollpoint. After reflecting on it, I am feeling that there is actually quite a lot that could be done around pinning/stickying elements and much of it would be out of scope of ui-scrollpoint.

Some examples of features of a pinning module (in addition to the two noted above):

karptonite commented 8 years ago

I definitely think that these could be beneficial, and I'm also unsure whether they belong in UI-Scrollpoint. I see UI-scrollpoint as a library for taking actions (adding/removing classes, etc) based on position. But it is agnostic about what the styles or classes are, and maybe that is for the best.

One possibility could be to make scrollpoint injectable, like affix in Angular Strap: https://github.com/mgcrea/angular-strap/blob/master/src/affix/affix.js That might make it possible for a whole array of versatile tools to be made.

For example, I'd love to have a library that does a sort of soft affix at both ends of an element: Say you have a div element that you want to affix, and that might be taller than the window. The behavior I imagine is that if you scroll down, it scrolls until the BOTTOM of the div is in view, then affixes the element to the bottom. If you change scroll directions, it should immediately un-affix, and allow you to scroll up until the TOP is in view, then affix it to the top. It is not simple behavior—dependent on scroll direction—but it seems like it would be useful enough that it should be out there.

PowerKiKi commented 8 years ago

Closed via #11