Add finish button to last level of a lab2 progression

[fisher-alice]

This PR adds a 'Finish' button to the last level (lab2) of a lesson. After a user clicks on the 'Finish' button, we post to milestone so that the level bubble turns green (or purple for an assessment level). Then a message is displayed below the disabled button.

Before clicking:

After clicking:

Note that currently, validation has not been added to aichat levels. Also, for music lab levels, the 'Continue' or 'Finish' buttons appear after validation conditions are satisfied.
When the 'Continue'/'Finish' button is clicked, we post to milestone with idealPassResult so that we will always pass or attain a green (or purple for assessment levels) bubble after clicking the button.

Future work includes sending accurate data to the backend and consuming the server response, i.e., the redirect url.

Scrreencast videos AFTER update:

aichat-finish.mp4

music-finish.mp4

Links

jira

Testing story

Tested locally in both music lab and aichat progressions.

Deployment strategy

Follow-up work

Currently, an assessment level does not turn 'bubble' after 'Continue' or 'Finish' button is clicked until the page reloads. Investigate and resolve if possible.
Update: this is existing behavior but not noticeable in legacy labs since page always reloads on level change. Here's an example in a CSP progression. Note that bubble turns green immediately for an app lab level even before page reload but not for an assessment level - at 0:25 in video.

legacy.mp4

I see in ProgressBubble.jsx that assessment levels render a progress bubble differently.

code-dot-org/apps/src/templates/progress/ProgressBubble.jsx

Lines 70 to 85 in 48feb7d

	const displayAssessmentBadge =
	isLevelAssessment(level) && !hideAssessmentBadge;
	if ((displayAssessmentBadge || hasKeepWorkingFeedback) && !smallBubble) {
	return (
	<BubbleBadge
	badgeType={
	hasKeepWorkingFeedback
	? BadgeType.keepWorking
	: BadgeType.assessment
	}
	bubbleSize={bubbleSize}
	bubbleShape={bubbleShape}
	/>
	);
	}

Privacy

Security

Caching

PR Checklist:

• Tests provide adequate coverage
• Privacy and Security impacts have been assessed
• Code is well-commented
• New features are translatable or updates will not break translations
• Relevant documentation has been added or updated
• User impact is well-understood and desirable
• Pull Request is labeled appropriately
• Follow-up work items (including potential tech debt) are tracked and linked

[sanchitmalhotra126]

Nice! This seems like a solid temp solution for now, and we can revisit in more depth once we have a better understanding of what we want to do across all Lab2 labs. Just a few minor comments.

[sanchitmalhotra126]

Curious, do the default DSCO button styles not work here?

[fisher-alice]

Thanks for catching this! I added this back when I was using <button> and added this before deciding to use the DSCO button. Will remove.

[fisher-alice]

FYI - After discussing with @breville , we're going to keep the style as is to not making any changes, even if subtle, before the music lab launch. After the two launches, we can make a lab2-wide decision on what buttons to use for common components. Thanks!

[cnbrenci]

Nit: optional suggestion for this interface & in general - IMO it's preferrable to give a variable/param a more descriptive name rather than a short name with extra description in a comment. That way you know the full meaning wherever that var is used as you're reading the code, and it's less likely to get out of date as the code changes (easier to forget to update the comment than the var name).

some examples:
text -> primaryDisplayText
showContinueButton -> shouldShowContinueButton
showFinishButton -> shouldShowFinishButton
beforeNextLevel -> onBeforeNextLevelNavigation
onNextPanel -> onNextButtonClick

These name changes would cause the extra comments to be redundant and they could be removed.

[fisher-alice]

Thanks! I probably lean towards longer descriptive names, but I've gotten feedback in the past to shorten variable names. So I think I'll keep as is for now, but definitely worth a team discussion!

[cnbrenci]

Does this need to be translated?

[fisher-alice]

This is a temporary UI solution and will most likely be iterated on in the next couple weeks. We wanted something in place for the pilot so I think maybe for now, keeping as a constant makes sense until we land on a more permanent solution. Thanks for bring up!

[fisher-alice]

@sanchitmalhotra126 @cnbrenci - since you reviewed, I added this so that 'Finish' button is no longer disabled when user leaves the last level.

[sanchitmalhotra126]

Not blocking but does it work if you use canShowFinishButton as the dependency for this useEffect? i.e. reset the finish button state whenever we go from showing the finish button to not showing the finish button? Asking only because I'd like to avoid using redux in this inner component if possible (the idea was this inner component could be used standalone outside of the lab2/progress redux framework, but currently it's only used by the parent Instructions component, so not a big deal).

[fisher-alice]

Oh no! I didn't see this until after I merged. Sorry about that! I can do a fast folllow-up for this. Thanks!

[fisher-alice]

#58528