react-progress-button icon indicating copy to clipboard operation
react-progress-button copied to clipboard

Add dontReenableAfterSuccess prop to support other default states bes…

Open codylieu opened this issue 5 years ago • 4 comments

…ides enabled

There seems to be a bug where the currentState state value and the state prop fall out of sync. This manifested itself in my project when the default state of the button (what it should get set to after success according to the onSuccess prop) is something other than ''.

The code already partially supports not automatically setting the button to enabled after success. This PR adds a prop which can be used to configure this. I tested this in my project and it works as expected when my code sets the button state to 'disabled' after success.

codylieu avatar Feb 08 '20 07:02 codylieu

Attaching an image of the inconsistency between state prop and currentState state. I'm using react hooks / onSuccess prop to set the button state to 'disabled' after it's state is 'success'. Not too familiar with react / js still so let me know if there's something I'm missing

image

codylieu avatar Feb 08 '20 07:02 codylieu

I'm confused, don't you want to set controlled to true instead?

mathieudutour avatar Feb 09 '20 08:02 mathieudutour

@mathieudutour Sorry, I do have controlled set to true, my screenshot above was a bit incorrect. Here's a correct screenshot that shows controlled=true with the inconsistency between the state prop and the currentState state attribute: image

More details in case they're helpful:

After I put my button in the success state, I have the following function passed to the onSuccess prop:

const resetRecordMetricButtonOnSuccess = useCallback(() => {
        setButtonState('disabled')
}, [])

I've noticed that the button becomes disabled as expected, but then becomes enabled immediately afterwards. I've looked over my code fairly carefully and don't see anything that would be setting it to enabled after the disabled state.

I think this is happening because the state prop (disabled) falls out of sync with the currentState state attribute ('') as shown in the picture above.

I commented out the following code in the ProgressButton class and it fixed my issue:

if (!dontRemove) {
        _this2.setState({ currentState: STATE.NOTHING });
}

This is what led me to add this dontReenableAfterSuccess prop. Let me know if I might be missing something. Also let me know if there are more details I can share to help you understand my issue

codylieu avatar Feb 09 '20 11:02 codylieu

maybe the fix would be to change if (!dontRemove) { this.setState({currentState: STATE.NOTHING}) } to if (!dontRemove && !this.props.controlled) { this.setState({currentState: STATE.NOTHING}) }? That seems like what we want in a controlled state.

mathieudutour avatar Feb 10 '20 07:02 mathieudutour