headroom.js icon indicating copy to clipboard operation
headroom.js copied to clipboard

Breaking API change - pin/unpin -> up/down

Open WickyNilliams opened this issue 9 years ago • 4 comments

I feel like the pin/unpin metaphor might have been stretched too much. The change would involve the following changes to the public API, in particular the following parts of the options object would change:

{
  onScrollDown : function() {}, // renamed from onUnpin. maybe not best name, since it's not on every scroll
  onScrollUp : function() {}, // renamed from onPin. again, naming issues.
  classes : {
    up : 'headroom--pinned', // renamed from pin
    down : 'headroom--unpinned', // renamed from unpin
  }
}

I like the change to up/down as it would be consistent with the properties in the tolerance option.

Does anybody have any thoughts on this? Is it actually a problem or am i overthinking it? This change would of course come before v1.0.0 (i.e. this would be the last breaking change i'd make) if it were to happen

WickyNilliams avatar Aug 04 '14 21:08 WickyNilliams

The reason i say this is because I've had people ask about using headroom to have elements come in from the bottom, and then pin/unpin makes no sense

WickyNilliams avatar Aug 04 '14 21:08 WickyNilliams

I've not yet had a chance to implement headroom.js (I'm hoping to in the next several months), but have been following your script for a while. I am one who is also interested in using headroom.js for having an element come in from the bottom (in addition to having a different element come in from the top on the same page). So I'm in favor of the naming changes as it does make it less confusing and more intuitive for use in both contexts (as I interpret that onScrollDown would always refer to downwards scrolling, regardless of whether one was using headroom.js to bring an element in from the top or in from the bottom, correct?).

I think onScrollDown and onScrollUp are decent names from my perspective (with my limited knowledge of headroom.js). Adding a note somewhere to indicate that it's not on every scroll should be sufficient.

I like headroom--unpinned as well, as it describes the state that headroom.js is in, as opposed to implying an action (and state makes more sense to me here).

One small suggestion: I might change the order slightly, just to make both sets of options in the same order.

{ 
   onScrollDown : function() {}, // renamed from onUnpin. maybe not best name, since it's not on every scroll 
   onScrollUp : function() {}, // renamed from onPin. again, naming issues. 
   classes : { 
      down : 'headroom--unpinned', // renamed from unpin 
      up : 'headroom--pinned', // renamed from pin 
   } 
}

One other thought: Are onTop and onNotTop going to make sense with the new naming? Because I've not yet implemented headroom.js I don't totally grasp what the various options are doing. But I'm wondering if maybe something like onWithinOffset and onBeyondOffset would work better for using headroom.js in both top and bottom contexts? Or alternatively: onInOffset, onOutsideOffset (I don't like these as much--they feel a little cumbersome/confusing).

Thanks for this great script! I'm looking forward to when I get to implement it for my site! :)

Sean

EnigmaSolved avatar Aug 09 '14 01:08 EnigmaSolved

First, thanks for writing this plugin, I use it a lot on my projects!

Considering the terminology, I still think that pinned/unpinned is the best approach, since that up/down is relative to the user device and/or settings.

Example: if you use the Magic Mouse or the MacBook Trackpad on OS X with the option “Scroll direction: natural” (enable by default), the content follows finger movement, pretty much like on touch devices. The scroll inverses than, up is down and vice-versa.

Anyway, that's my 2 cents.

manfromanotherland avatar Apr 13 '15 12:04 manfromanotherland

I just noticed that the options are now called pinned/unpinned instead of up/down. I'm a little late to the discussion, I guess. :P

manfromanotherland avatar Apr 13 '15 12:04 manfromanotherland