captivationsoftware / react-sticky

<Sticky /> component for awesome React apps
MIT License
2.64k stars 385 forks source link

Support for non static sticky element #263

Closed zerubeus closed 5 years ago

zerubeus commented 6 years ago

This is PR to add support for non static element, and adding tests to the CI:

Changes :

This is a non breaking change if the library user support React 16.6

zerubeus commented 6 years ago

@vcarl can you take a look please :)

vcarl commented 6 years ago

Could you split this up into multiple PRs? This is a LOT of changes to land in one go. Separate PRs for:

would make it a lot easier to land some of these while others are discussed. Because the library lists React 15 as a peer-dep, many of these changes are breaking, so they'd have to be released as 7.0.0. I don't want to do that without also talking about other potential API changes to avoid unnecessary churn.

vcarl commented 6 years ago

Also I want to call out that I am SUPER GRATEFUL that you've taken the time to do these!! A lot of them have been somewhere on the backburner for me, but never made it to the front of my todo list.

vcarl commented 6 years ago

It's very difficult for me to review these changes because they're all in a single commit, with lots of Prettier formatting changes interspersed. Please separate this out into multiple PRs and use default Prettier settings to avoid diff noise.

zerubeus commented 6 years ago

@vcarl ok I'll split this to multiple RP's

vcarl commented 5 years ago

Hey @zerubeus since there hasn't been any movement on this in a while, I'm going to close it. I'd still love to see this split into a few PRs, it's a great contribution but I don't have the bandwidth to tackle it myself.