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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
WickyNilliams opened this issue Aug 4, 2014 · 4 comments
Open

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

WickyNilliams opened this issue Aug 4, 2014 · 4 comments
Labels

Comments

@WickyNilliams
Copy link
Owner

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
Copy link
Owner Author

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

@EnigmaSolved
Copy link

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

@manfromanotherland
Copy link

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants