Feature: basic start and end callbacks#48
Conversation
|
Hey, thank you for the PR. I’m travelling atm, but I’ll take a look tomorrow. |
source/Plx.js
Outdated
| if (!wasActive && isActive) { | ||
| this.props.onPlxStart(); | ||
| } | ||
| if (wasActive && !isActive) { |
There was a problem hiding this comment.
else if would skip unneeded checks
There was a problem hiding this comment.
Yes, silly error! I'll fix it now.
source/Plx.js
Outdated
| style: {}, | ||
| tagName: 'div', | ||
| onPlxStart: emptyFunction, | ||
| onPlxEnd: emptyFunction, |
There was a problem hiding this comment.
Why not null? IMHO this.props.onPlxStart !== null is a bit cleaner check.
There was a problem hiding this comment.
True.
I'd done this to keep one flag "callbacksEnabled" and then fire both if even one prop (ex. onlt onPlxStart and not onPlxEnd) is defined. I'm changing the code now to have two flags, plxStartEnabled and plxEndEnabled, so that we can fire only the applied prop.
|
Hey @blnk-space, I'm wondering if we can improve callback system even further. Current implementation allows only for "global" callbacks for a single Plx instance. Maybe it would be better to introduce them per section (for each item in Related issue: #26 Cheers! |
|
Yeah, even I don't love the "hacky" method, I thought of doing it until this way I could spend more time and implement a direct solution. Next step for me would be to implement a function like getClasses that does this for us without string comparison. And per-section callBacks are also something I've been wanting to implement, I'll hopefully do this all in the next couple weeks! |
|
Released in 1.3.11 🎉 |
2 participants
This PR adds two props, onPlxStart and onPlxEnd.
This might be helpful in certain cases and doesn't hurt the library by increasing any complexity. I've pretty much just added a decorator to already computed values to enable this feature. It behaves as it previously did if we have neither of the aforementioned new props specified to the component.
@Stanko Let me know what you think!