search

WickyNilliams / headroom.js

Give your pages some headroom. Hide your header until you need it
https://wicky.nillia.ms/headroom.js/
MIT License
10.86k stars 824 forks source link

Add support for multiple state classes #348

Closed hacknug closed 4 years ago

hacknug commented 5 years ago

As discussed in that issue, this PR allows users to specify multiple classes on their Headroom options using a space-delimited string.

I used the spread operator but I'm not sure if we're using Babel or not. Can rewrite it using .apply() if not but maybe it would be better to introduce new tolling so we can start writing modern JS for v1.0 👍

Closes #157.

WickyNilliams commented 5 years ago

Thanks for this!

Yes can you switch to apply for now, there's no babel or anything at the moment, everything is ES5. Also the build has failed due to linting issues

hacknug commented 5 years ago

All good now. Had to add eslint to devDependencies to make it work on my side (guessing you have it installed globally). Also added eslint-plugin-cypress to prevent linting issues on the tests 👍

WickyNilliams commented 5 years ago

Good spot on the eslint installation. I think it's implicitly installed by rollup-plugin-eslint, it should really be a peerDependency of that library, but for some reason it is. Better to be explicit anyway :)

WickyNilliams commented 5 years ago

Any updates on this @hacknug?

WickyNilliams commented 5 years ago

I think this branch could actually be merged to master since it is a non-breaking change? Would save you rebasing etc then. I'm happy to do that on the v1 branch

hacknug commented 4 years ago

@WickyNilliams sorry but I took some days off open-source due to the protests and everything going on here in Barcelona. Had to catch up with client work after that and a few days of vacations.

Just pushed the changes you requested and changed the target branch 👍

WickyNilliams commented 4 years ago

No worries, thanks for coming back to finish it off :) I think the only issue now is that the branch needs a git rebase master. There are some changes in the diff which are from the v1 branch, so I think rebasing will fix that

WickyNilliams commented 4 years ago

I've created a new PR to supersede this one. I have cherry picked your commits so that you still get the authorship in the history :)