Closed voytek98 closed 3 years ago
I just want to get some clarification. Are you wanting to set stepName
so you can pass that into goToStep
? You may already know this, but you can do this with hashKey
, so long as isHashEnabled
is enabled, like:
<First hashKey={'FirstStep'} />
and then call
goToStep('FirstStep')
Are you hoping to accomplish this without having to use isHashEnabled
?
I just want to get some clarification. Are you wanting to set
stepName
so you can pass that intogoToStep
? You may already know this, but you can do this withhashKey
, so long asisHashEnabled
is enabled, like:<First hashKey={'FirstStep'} />
and then call
goToStep('FirstStep')
Are you hoping to accomplish this without having to use
isHashEnabled
?
Yes, I want to accomplish it without isHashEnabled
. Think about cases where we:
isHashEnabled
)and we just simply want to write cleaner code, instead of using magic numbers (goToStep(1)
=> goToStep('load')
)
You bring up some valid points. I am concerned it could get confusing with passing either hashKey
or stepName
to goToStep
. As of now, the hashKey
takes priority and I'm sure someone will try to use both hashes and names.
For that reason, I think we should drop the namedStepsEnabled
flag and assume you can always pass stepName
to a step component. And rather than calling goToStep
we create another method called goToStepName
. What are your thoughts on that approach?
I would love to reuse goToStep
for step names but it does make it more error prone to support magic numbers, hashKeys, and stepNames. I just think it would be better to decouple that logic.
Your arguments are strong and the only one thing I would change is naming. goToStepName
sounds a bit confusing, I would rather change it to goToNamedStep
, as we are changing to different step, not to it's name. I'm okay with dropping namedStepsEnabled
flag. If you approve that, I'm going to push new changes and we can merge dat boy
I like that a lot better! If you push those changes up I’ll get it merged!
Waiting for your merge then! https://github.com/jcmcneal/react-step-wizard/blob/3bec9a625e1a4b089d65e2cd6c0f91dafef4d15f/src/index.js#L166-L173
fixes #38
I've added this feature because it might be useful in some cases., especially in project that I'm currently working at. It's just like
isHashEnabled
feature but simplifiedChanges
namedSeps
to state,namedStepsEnabled
prop to<StepWizard>
,stepName
prop to<Step>
,readme
,snapshot
,TypeScript
Example