-
Notifications
You must be signed in to change notification settings - Fork 524
hoai2025: navigation logic #69083
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
hoai2025: navigation logic #69083
Conversation
| %li | ||
| <code>parent</code> (default): Clicking Continue on a sublevel returns to the parent bubble choice level. This is the default behavior if no navigation_type is set. | ||
| %li | ||
| <code>next_level</code>: Clicking Continue/Finish on a sublevel advances to the next in the level progression, or finishes the lesson if the are no more levels. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ultra nit: there
breville
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the thoughtful work, and for keeping in mind what needs to be done next.
mikeharv
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good. Looks like we'll need to update a test in progressReduxTest to add in the new navigationType values.
| className={moduleStyles.buttonWide} | ||
| /> | ||
| {showNavigation && ( | ||
| <NavigationArea |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this removes the custom 'Keep this' text - will address as part of https://codedotorg.atlassian.net/browse/LABS-1673
| level.sublevels.map(sublevel => processedLevel(sublevel, id)), | ||
| path: level.path, | ||
| parentLevelId, | ||
| navigationType: parentLevelId ? level.navigation_type : undefined, // Only applicable for sublevels. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello - just trying to understand this as related to #69085. If level has a parentLevelId, then isn't the parent level that has the navigation_type property? Just trying to understand this assignment - thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added the navigation_type property to the sublevels themselves so it would be easier to look up where to go at the point we're navigating away from the sublevel (see summarize_sublevels here:
| navigation_type: navigation_type, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh...that makes sense. thanks!
Updates the navigation buttons in the Guide UI to use the
NavigationAreacomponent to reuse logic and make sure the appropriate button (Continue/Finish) is displayed.Notably, this also adds new logic for displaying a Finish button on the last freeplay bubble choice level. Previously, clicking "continue" on a bubble choice sublevel always directed you back to the parent, and this was enforced both on the client and the server. With this PR, I've added another new bubble choice field called
navigation_type, which can currently be either'parent'(default) or'next_level'. If the navigation type for the bubble choice level is'next_level', then clicking Continue on a sublevel will take you to the next level in the progression, or finish the progression if on the last level. This is reflected now in both frontend and backend code. Later, we could add more navigation types (e.g.,'next_sublevel'which could direct you from the current sublevel to the equivalent sublevel in the next bubble choice level, effectively unlocking a version of "choice progressions").As of now, I've updated pilot 6 to use the new navigation type, but note that the Finish button just directs back to the script overview page for the time being. Eventually in our final script, this Finish button will show the congrats/share dialog, but this will require marking the script with the correct marketing initiative and adjusting a few other parameters for the share dialog to work properly.
Also note that I intentionally didn't do any styling work on the button so it does look different than what it was previously; I figured we have more styling work to do on the Guide as a whole and can update then.
This also updates the standalone project experience to not show the navigation buttons. However, we'll need to make some edits here anyway to update the "narrative" of the Guide UI to reflect that we're in standalone mode (e.g. not saying "Let's continue" and allowing users to regenerate on demand). The standalone experience needs some work in general, so that will be addressed separately.
Links
https://codedotorg.atlassian.net/browse/LABS-1630
Testing story
Tested with HoAI script and a few others to verify the
navigation_typebehavior.