Add guide index to state to reuse WalkthroughElements (TS version)
Hi @jasongaare,
i gave up on merging the old branch as it was based on JS files , so I created a new, clean PR. You can compare that the changes are equal to the old PR #7 .
Thanks a lot for your efforts! Daniel
I also added a handler for the end of the walkthrough, it's fully optional first i wanted to create a distinct PR but then there would have been lots of merge conflicts, so i put it on top of this one
Looks good - only a couple thoughts.
-
The addition of allowing multiple steps with the same element id does kind of break the function
goToElementWithIdas it will always find the first matching element. I don't think any changes are quite necessary now but the unique ids did allow for that function to provide the one element you were looking for... -
Could you kindly update the README here https://github.com/jasongaare/react-native-walkthrough#startwalkthrough to add the
onFinishfunction? also personal preference but I kind of likeonWalkthroughComplete(although it is longer!)
Actually speaking of goToElementWithId it actually looks like the index is not getting changed ever in that case, and things get out of whack. Specifically we need to make sure every time setElement is called we check the index and set it there... this causes another, more direct issue with the goToElementWithId, but would still "work" just might always loop back to the first element with that ID, but at least the currentIndex would correctly update.
sorry I'm rambling I will attempt to update the example app to add some situations that might be useful to test
Dear @jasongaare thanks for your feedback, I was very busy the last weeks so i haven't had any time to update the PR.
i added 4 commits
- renamed the completion handler, i like your suggestion better too
- i created a single method that updates both, the index and the element to avoid the problem you mentioned; regarding multiple elements with the same id i just pick the first.
- i am not sure if i correctly understand the possible outcome element; is it an element that shows nothing but listens for events (e.g. user clicks on button xy) and then it continues the walkthrough with some element, which needs to be encoded in the action part of the handler (e.g. by
goToElementWithId)? I added a suggestion to put the start time into the closure of the wrapper function; this way you don't have to look it up in the state, which looked a bit brittle. - update to the README with a simple logging completion handler
Cheers, Daniel
@jasongaare ping
This PR looks great. Any chance we can merge it to the library? @jasongaare