feat: add pin property #205
Conversation
|
Hey @lunsdorf thanks for the PR! Looks like a nice thing to add! I'll try to have a closer look at it this week. Meanwhile I had a question that came to me while glancing at it initially: What if the Also would be great if you could mention the prop in the docs ( |
|
Ok, I added the prop in the docs and also check for |
src/index.js
Outdated
| } else if (prevProps.pin !== this.props.pin) { | ||
| this.handleScroll() |
There was a problem hiding this comment.
Is there any reason to put it in else if instead of if? Previous conditions check if props changed from disabled: true to disabled: false or the other way around. Those two cases are handled differently but it's unrelated to change of pin prop. So I think you could change it to a separate if instead of else if.
There was a problem hiding this comment.
I think I simply added it the the block I saw without any particular reason :) I changed it to a separate if.
This PR applies all changes from #131 as a patch together with an additional fix for the unit tests. I did it this way, because I deleted to fork I worked on years ago and couldn't figure out how to append to an existing PR with all sources deleted.
I did this because people keep asking if the changes could be integrated. I did not re-evaluate or actually test the changes I made when I originally wrote this. So somebody should definitly do that.