Skip to content

Sticky without jQuery dependency #395

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

vovayatsyuk
Copy link

I've added methods to work with classes (can't use classList because of IE9,10) and proxy method to Sticky object.

}
}

Sticky.proxy = function(func, obj) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The native function bind is available in IE9+, so proxy isn't necessary.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But proxy would be handy for unofficial support for IE8. No harm in it.

@imakewebthings
Copy link
Owner

I could be game for adding noframework support for the sticky shortcut, but I'd need a couple things to change:

  • It's already confusing enough that there are 3 builds of the core library. I'd rather not have multiple builds of the shortcuts. I know this would require a pretty big refactoring of how your current implementation works.
  • Sticky tests need to also run against the noframework adapter. They currently only run against the jQuery adapter.

I know those may be some big asks with very little direction. If you would like to give that a shot, that would be awesome. If not I can leave this as a feature enhancement request.

@shellscape
Copy link

@imakewebthings have to disagree with you there. This PR follows the established patterns of the project, and @vovayatsyuk is following that pattern responsibly. I do agree with your point about this needing to run against the proper tests - that's a must have. But if that were implemented, I don't feel it's reasonable to ask a PR author to refactor your project just to submit a fix where the fix is valid, needed, and would be welcomed by the community.

@imakewebthings
Copy link
Owner

@shellscape Allow me to clarify.

I know those may be some big asks with very little direction. If you would like to give that a shot, that would be awesome. If not I can leave this as a feature enhancement request.

I realize I didn't communicate this, but by "leaving this as a feature enhancement request" what I mean is that, when I do get around to merging this, I can finish making the changes necessary for this to be released.

@mpolichette
Copy link

Hey @imakewebthings Thanks for the project, any status updates for this merge? Do we think it will eventually be merged in?

@thematan
Copy link

please check-in 👍

@imakewebthings
Copy link
Owner

@thematan My newborn son continues to grow, remain healthy, and delight me every single day for the few precious hours I get to spend with him. Thank you for asking!

@mpolichette This PR or something like it will be merged eventually and part of 5.0.

@sdras
Copy link

sdras commented Jun 14, 2016

Hey ya'll, any progress on this? I could use it too and it's passing tests. Would appreciate any work getting it through, thank you!

@imakewebthings imakewebthings added this to the 4.1.0 milestone Jun 21, 2016
@ericnkatz
Copy link

Thanks for all the hard work @imakewebthings! 👍 ❤️ waypoints.

@FranciscoG
Copy link

any possibility of getting this merged in?

@shellscape
Copy link

@FranciscoG way to raise this one from the dead. there have to be better alternatives out there by now, surely.

@Berkmann18
Copy link

@shellscape Would those alternatives (if those exist) work for say Gridsome/Vue?

@shellscape
Copy link

@Berkmann18 I've no idea. I haven't used this project in years.

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

Successfully merging this pull request may close these issues.

9 participants