react-native-progress-steps Prop updates

colbymillerdev / react-native-progress-steps

A simple and fully customizable React Native component that implements a progress stepper UI.

MIT License

361 stars 145 forks source link

react-native-progress-steps Prop updates #38

Closed curtismenmuir closed 4 years ago

curtismenmuir commented 4 years ago

Description

Added removeBtnRow prop to ProgressStep so that the Buttons can be completely removed
Added componentDidUpdate function to ProgressSteps so that activeStep prop can be used to programmatically update the current Step position
Added isComplete prop to ProgressSteps so that a complete stage can be shown (All ProgressSteps set to completed).
Added completedLabelColor prop so that Completed labels can be set differently to Disabled labels

NOTE: I am happy to split this up into separate PR's if needed -> I required all of the changes for a release at work.

curtismenmuir commented 4 years ago

See issue: https://github.com/colbymillerdev/react-native-progress-steps/issues/39

colbymillerdev commented 4 years ago

Hi @curtismenmuir - Thanks for opening a PR and wanting to contribute to the project!

Removing activeStep from the internal state of <ProgressSteps /> will break existing implementations of the project. The activeStep prop hasn't been required to be set/incremented previously by the user for the buttons to work. So I think this needs to be implemented in a way that makes it compatible with existing functionality.

I will take a look at the other proposed changes to see if they make sense to include in the next release!

curtismenmuir commented 4 years ago

@colbymillerdev sorry, I didn't think about the buttons as I have used the new removeBtnRow prop that I have added! Happy to remove the activeStep change from PR if you are happy with other changes!

curtismenmuir commented 4 years ago

@colbymillerdev Could use componentWillUpdate and check if activeStep prop has changed:

componentWillUpdate(prevProps) {
    if(prevProps.activeStep !== this.props.activeStep){
      this.setActiveStep(this.props.activeStep);
    }
  }

Will update PR with example

curtismenmuir commented 4 years ago

PR updated so that activeStep prop is set in state, but it can be updated due to componentWillUpdate function

colbymillerdev commented 4 years ago

@curtismenmuir Awesome, thanks for updating that! I'd rather not use componentWillUpdate since it's been deprecated and will be removed in a future React release. We could try using the getSnapshotBeforeUpdate lifecycle method instead.

Everything else is great, but I'd like to hold off on the hasFailures functionality for now to give it more thought. Would you mind also updating the PR to remove that feature? Thanks!

curtismenmuir commented 4 years ago

@colbymillerdev Sorry, I have had a couple busy days. No worries, I will get those changes made tomorrow and will let you know when they are ready!

colbymillerdev commented 4 years ago

@curtismenmuir Awesome, sounds good!

curtismenmuir commented 4 years ago

@colbymillerdev I have been looking into getSnapshotBeforeUpdate and I believe that this may only require updating componentWillUpdate to componentDidUpdate. We could use getSnapshotBeforeUpdate to feed into componentDidUpdate but I believe there is no need as componentDidUpdate also has access to prevProps. I have tested locally and all is working fine. What are your thoughts on this update?

colbymillerdev commented 4 years ago

@curtismenmuir Cool, that works for me! I'll test it out also to make sure everything works as expected.

curtismenmuir commented 4 years ago

@colbymillerdev Requested updates are ready for review. I have tested locally and looks good! Also updated PR description as per updates. Let me know if there is anything else you want me to update.

colbymillerdev commented 4 years ago

@curtismenmuir Looks good to me and got it merged in. Thanks again for making these changes!