Add help and tips to Code Bridge

[molly-moen]

This PR enables editing and showing help and tips in Python Lab. The ability to see help and tips is enabled in Web Lab 2 as well, but for now I'm keeping level editing changes scoped to Python Lab.

I made a rough approximation of the figma version of help and tips. It is a dropdown on what was previously only the instructions panel which will only show up if the level has help and tips. I've renamed the instructions component "Info Panel" since it now encompasses more than just instructions.

We did not have a component like the one in the figma (an icon-only dropdown button), so I patched one together using the design system Button components and some copied css from the design system dropdown components. Since this is still an approximate UI I didn't spend time perfecting it.

Figma

Current UX

May-16-2024.09-20-34.mp4

The hover color is light gray because that is the default in the design system.

Links

• jira ticket: CT-461

Testing story

Tested locally.

Follow-up work

If this is the final design, there is some polish work to do on the dropdown.

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

[molly-moen]

this just removes a rogue ; that was showing up under the editor

[mikeharv]

Looks good, a couple questions. Pulled, reseeded, and tested and seems to be working great!

[mikeharv]

Nit: If this map function just returns the list item, I think we can actually remove the function's outer curly brackets and explicit return keyword.

[molly-moen]

I get errors when I do that. I believe the reason is this is inline code rather than a function, and any code inside the html needs to be surrounded by curly braces.

[mikeharv]

What errors are you getting? I was curious and tried modifying locally and it seems to work. You should be able to remove the curly braces and return - and the return statement's ;, which I missed initially!
I believe you only need the curly braces and return if the arrow function includes other steps.

[molly-moen]

oh I misunderstood what you were saying. I thought you meant remove the curly braces around panelOptions().map.... Updated to remove them from the inner function.

[mikeharv]

Does this need to be a function or could panelOptions just be defined as an array? Is there a need to recalculate the options each time panelOptions() is called or would it be sufficient to do it once?

Going a step further, would it be worth memoizing panelOptions so that it only re-calculates when there are changes to mapReference or referenceLinks?

[molly-moen]

The reason to keep it as a function is we could switch levels and go from having to not having help and tips, and want to update accordingly.
It's a pretty cheap operation, so I don't think we need to memoize. The react docs say you don't need to memoize for cheap operations.

[mikeharv]

Sounds good to me! These hooks are still pretty new to me, so I've been trying to test my understanding of how (and why) to use them. Thanks for explaining!

[molly-moen]

I ended up changing panelOptions to be a state value instead, because I found a bug where if you switch levels and were previously on a panel that does not exist on the new level it will still show as your selected panel, but be blank. So I needed to add a useEffect that depended on the panel options changing.

[mikeharv]

In your Start Over PR, you used useCallback for functions like resetToStartSource and onClickStartOver. Could we do the same thing here to avoid re-creating this function unless the dependencies change?

I have the same question for changePanel and renderHeaderButton in this file.

[thomasoniii]

I'm pretty sure this can be a statically wired dispatch lookup instead.

Move it outside the component and name as:

const getCurrentPanel = {
  [Panels.Instructions] : InstructionsView,
  [Panels.HelpAndTips] : HelpAndTips
}

Then call it as:

{ getCurrentPanel[currentPanel]() }

(annoyingly, react will not render <CurrentPanel[currentPanel] /> (AFAIK), so we need the janky syntax)

Or, to make it prettier, but less compact:

const CurrentPanel = getCurrentPanel[currentPanel]
...
<CurrentPanel/>

Or, if you still want it inline:

const CurrentPanel = useMemo(() => {
  const panelLookup = {
    [Panels.Instructions] : InstructionsView,
    [Panels.HelpAndTips] : HelpAndTips
  }
  return panelLookup[currentPanel]
}, [currentPanel]);
...
<CurrentPanel/>

[molly-moen]

I'll update this to be a static lookup.
To the question on useCallback, the reason I added it around the start over logic was because I was seeing some issues with unexpected resets of the project code due to us firing a callback too often. However, I probably was overly cautious and didn't clean up ones that didn't need to be wrapped in useCallback.

[mikeharv]

Curious about my question of removing the return, but definitely not a blocker!

[breville]

I'm curious about some broader architectural questions. I see that we have added some support to Lab2 for this, but the component itself lives in CodeBridge. Do we consider this work a Lab2 feature?