fix/dyad census and tie strength census #73
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
if there are no steps, dyad and tie strength should automatically allow progress to next stage even if there are additional prompts (because those will also not have steps). this functionality was removed with implementation of useNavigationHelpers. reimplemented using an optional prop options in moveForward and moveBackward
jthrilly
approved these changes
Feb 2, 2024
There was a problem hiding this comment.
Really well done for figuring this out.
I don't love the idea of polluting the function signature for moveForward/moveBackward with options, but I see why it has to be done.
There are two things to think about:
- Is
optionsand thenforceStageChangeright right structure and naming convention? Here I would ask if we are likely to add more "options" in the future (my thinking is probably not), and then if not could I make it a single function parameter perhaps just simply calledforce. We would get some IDE autocomplete on then when using it, which would work nicely. - The other approach, which could actually be combined with the approach above, would be to have the hook export alternative versions of the navigation methods that don't perform the prompt check. You could call these
forceMoveBackwardandforceMoveForward. They could be implemented internally using a variable as in (1), but this implementation detail could be hidden from the consumer. The obvious downside to this is more verbiosity.
Just posting these considerations as stuff to think about if you didn't already! Going to merge this in as-is.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When there are no steps and multiple prompts on Dyad Census and Tie Strength Census interfaces, navigation back/forward should ignore multiple prompts and force next/prev stage.
With the refactor of navigation and implementation of
useNavigationHelpers, this functionality was removed. The default navigation prompted navigation to the next/prev prompt instead. This caused an issue where next/prev navigation buttons appeared to need to be clicked multiple times to advance past the introduction, once for each prompt.This PR fixes navigation for dyad census and tie strength census by forcing navigation to next/prev stage if there are no steps. This is implemented using an optional
optionsprop in themoveBackwardandmoveForwardmethods inuseNavigationHelpers.Runtime errors on these interfaces are also fixed here.