Just curious - do we still need to add loc strings?

I do so here because this is a global element that is visible in legacy applications... though, it is still unlikely to get a translation. We might go in and manually add a manual translation for a few languages, however. For instance, a spanish translation we manually add seems appropriate for this. This is follow-up work.

Nit: could name something like nextLevelNumber and currentLevel as currentLevelNumber (below).

Could you add in the doc what happens when type is NEXT_LEVEL and not PARENT? And to confirm my understanding, would the level after the parent level be returned? If so, would 'Continue to Level X' be displayed?

Also, what happens if the bubble_choice level is last in the progression with NEXT_LEVEL navigation type? Would 'Finish Lesson' be displayed?

Tagging @sanchitmalhotra126 who added navigation_type in #69083 - it seems like if the sublevel navigation type is NEXT_LEVEL, then the level after parent level would be returned, and if the parent level was the last in a progression, undefined would be returned?

Yep correct! If getNextLevel returns undefined, in either case, we will show the "Finish" text rather than Continue.

Nit: could update to include both navigation types:
* Bubble choice levels return the parent level in the case where the navigation type is 'PARENT' and not 'NEXT_LEVEL'

That's technically true, however, it's a default behavior where NEXT_LEVEL indicates breaking the default behavior. It's documenting the guard and not the else statement: Here's what to do to influence the default behavior for sublevels... which usually go to the parent level when you are done. I don't want to over-document in case anything is updated where the documentation could go out of sync... though to be honest a good way to deal with that is a test and we have none for these. The proper thing to do would be to add some tests to ensure we capture the documented behavior. I will do this.

Thanks Wilkie for your thoughtful response - this makes sense!

This change also impacts the Hour of AI Guide UI since the NavigationArea is shared between resource panel and GuideInstructions. Maybe add a prop when we want to add more descriptive text?

And do you mind checking levels in music lab? They share the navigation area, but their navigation button is not pinned to the bottom as it is in other lab2 labs. It appears once the required actions are taken by user. Thanks!

Good find. Yes, these buttons are problematic when localized too since being too long is difficult to accommodate.

Resolved this by using a resize observer. If the button is too small for whatever reason, it will render the simpler text we were using before.

Please see the video in the PR description.

Thanks for adding the variant prop!

Updated to just truncate the text with text-overflow so it shows ... (although another update just used the added variant prop to force Guide components to use the old text)

Similar impact on Guide UI - probably need to check if folks want 'Finish Lesson' instead of 'Finish' on last level of Hour of AI.

Ah, so if there is only one lesson? That's an interesting question.

Resolved this by investigating the number of lessons in the script. If there's only one, then we always just say 'Finish' instead of 'Finish Lesson'.

Nice!

It is part of this ticket to update the background color to have it stand out against feedback messages in the instructions panel... so that the button doesn't look like it is relating to any feedback or messaging that might appear directly above it.

When I load a level, the full button text is shown, but when I go to next level, shortened version if shown. But full text is shown if I reload level.

I think that we're missing dependency on level id?

The element gets unmounted and hidden for a moment as the level switches leading it to have a 0 width for a frame. However, it doesn't trigger the event again when it is revealed. It's a little haphazard and mysterious, though it is fixed if you ignore 0 width... I've elected to abandon it and just do normal text truncation since the main issue was with Guide components and we just force those to render the old text anyway.

Nit: could rename to something like buttonTextVariant.

+1, or something like simpleButtonText?: boolean? variant feels a bit generic here

I dislike boolean properties! What if you want to add a third option? You'd have to rewrite the whole property. I learned this lesson from a W3C CSS editor when he asked me why I thought there were no boolean properties in CSS: because you don't want to be stuck with it. Doesn't 100% apply here of course... we can change them, but it's on my mind a whole dang lot.

variant is a conventional React property for influencing the appearance where it doesn't necessarily affect the behavior of a component. It felt right to use it when it is a property that decides a certain type of rendering behavior. It is meant to be generic. It's possible we might want a third option at some point.

That's fair! I do still think a more specific property name would be helpful though as NavigationArea refers to more than just the navigation button.

Thanks Wilkie for sharing your thoughts! I agree with Sanchit that a more specific property name would be helpful, like buttonTextSize or buttonTextVariant since NavigationArea is used in InstructionsV2 and the resource panel which both contain a long list of props, some of which get passed to NavigationArea.
But I'm good with your keeping variant if you feel strongly about it. Thanks for the discussion!

Thanks for adding the variant prop!

Nit: could update to include both navigation types:
* Bubble choice levels return the parent level in the case where the navigation type is 'PARENT' and not 'NEXT_LEVEL'

I wonder for the 'small' variant if we just want to always display 'Finish'? @dju90 curious for your thoughts here.

Yep. Makes sense. Done.

Sorry, what's the alternative?

I was suggesting that If the button text variant is 'small' (as it is in the GuideInstructions for the Mix and Move progresion), the text will always be 'Finish', even if there is more than one lesson in the course unit.
The alternative would be that even if the button text variant is 'small', that 'Finish lesson' would be displayed if there is more than one lesson.

Oh I see. Okay yes, having just "Finish" always for the small variant sounds good to me.

Nice!

nit: remove?

That's not a nit. That's just wrong!!

Just curious - Is it wrong or is it not necessary/redundant?

+1, or something like simpleButtonText?: boolean? variant feels a bit generic here

Optional: could move the logic for computing the button text into one selector instead of pulling out these two values separately

I generally dislike adding logic to the redux selectors, but I'm happy to do this.

Yeah I suggest this just because then technically the component will re-render only when the actual text value changes, rather than when either of these values change.

What's confusing is why this is levelNumber - 1 to get to the level after the parent level. I'd expect just levelNumber.

But it is because the levelNumber is 1 based and we need to convert it to a zero-based index.

Although if it has an unplugged activity, all this logic already breaks down unless the level data specifically has title || position in such a way that it provides the right 0-based index. hm.

One of my favorite unplugged lessons is Binary Bracelets. Looking at the script, I see
position is a Script Level property that gets assigned tolevelNumber on the frontend I think?

Indeed. I love that we promote unplugged and more offline content.

Here's the logic (processedLevel()) that decides the levelNumber... indeed it is decided on the frontend and is an interpretation of the data sent from the backend. So, the level data contains a levelNumber that is undefined if unplugged and otherwise is the title value (falling back to the position if title is false-y (0 or undefined))

So if you follow the logic through, when it constructs the level array that it is using to dictate the level progression, it does this by calling levelsByLesson() which maps each level via levelWithProgress():

Which always expands the level data via that aforementioned processedLevel method:

So, if you have level data (like the test data being used in progressReduxTest.js) that is [{kind: 'unplugged', title: 'Unplugged'}, {title: 1}, {title: 2}] you'll get a levels array that is: [{levelNumber: undefined}, {levelNumber: 1}, {levelNumber: 2}] which then breaks the logic because if you are on the second level which has levelNumber equal to 1, it will decide that the current level index is 0 (not 1) and advance to level "index" 1, which is the level it is already on.

I don't know why this isn't a problem elsewhere. Probably because the test data is not very representative? It says that the test data is real data, though. But I would assume that the title is somehow accurate to level "index" in real-life situations.

Thanks for the walk-through! So levelNumber can sometimes NOT be a number and sometimes a string like for the bracelet level!

And yes, it does seem like for most levels, title is the same as level 'index'.

Well, it would be the same as the level index + 1, but yes. In the test data, it would render in the progress bubbles as (Unplugged Activity) (1) (2) which would break the expected behavior. This is a tangent of the original intention of this task... but to add tests, I needed to fix it to work for the test case. Overall, it breaking in the test case makes me nervous regardless. Let's see if I can prove that this never actually happens in the wild.

In practice, title is not a real field. It is computed relative to position:

It does seem like a lesson with an unplugged activity as level 1 would break the progression logic for the continue button on any level in that lesson... if the subsequent levels have their positions be 2, 3, 4, etc? title is not a very good representation of levelNumber in this case... or the logic to produce the levels array should ignore the unplugged levels (where levelNumber === undefined).

Searching for this:

[development] dashboard > ls=Lesson.all.filter{|lesson| lesson.unplugged_lesson?}
[development] dashboard > ls_of_interest=ls.filter{|lesson| lesson.script_levels.second&.level_display_text == 1}
[development] dashboard > ls_of_interest.map(&:script).map(&:name).uniq
=> ["AlgebraB", "algebra", "course1", "course2", "course3", "k5concepts"]

So all deprecated courses and k5concepts which doesn't use the progression logic.

It just so happens we used course3 as the test example for the progress redux when it's the rare example of a course that breaks the progress logic. Neat.

TIL the difference between an unplugged level and an external level displayed as an unplugged activity. Also learned more about levelNumber and title and see that the Unplugged Activity bubble label is actually set by whether level.isUnplugged is true or not in BubbleFactory.jsx (not by levelNumber).

Nice investigation! I see that lesson.unplugged_lesson? only returns true if the first level (or oldest active level) is a true unplugged level:

So it makes sense that the title or level_display_text in modern courses including those with unplugged activities are assigned position (and not position - 1).
Here’s an example of a modern course with an unplugged activity as the first level and the second level as bubble 2. ​​https://studio.code.org/courses/coursea-2025/units/1/lessons/11/levels/1

As you found, only deprecated courses uses the position - 1 for title. You can see the second level has bubble 1 in the screenshot the deprecated course 3 lesson 1 progression.

I wonder if maybe in a follow-up we should replace the test data in progressReduxTest with more modern course data ('coursea' instead of 'course3')?

Learned a ton - thanks Wilkie!