Skip to content
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

[PythonLab] basic Start Mode #58405

Merged
merged 12 commits into from
May 6, 2024
Merged

[PythonLab] basic Start Mode #58405

merged 12 commits into from
May 6, 2024

Conversation

mikeharv
Copy link
Contributor

@mikeharv mikeharv commented May 3, 2024

This adds a basic start mode to Python Lab.

Levelbuilders can select [s]tart from the Extra Links menu:
image

In Start Mode, the text "Levelbuilder: edit start code" and a save button are added to the page header. The workspace is loaded with code from the existing level config, or a hard-coded default project if that's missing. A channel is not created so that the levelbuilder's personal project in the level view is unused and unaffected. The levelbuilder can create and edit files, and press save when done:
image

Upon revisiting the level, the levelbuilder will still see their previous project files because we have yet to implement a Start Over button or Version History. However, the new start sources can be confirmed by opening the level in an incognito tab.

image

Technical Details

Levelbuilder

On the Dashboard side, I extended access to JavaLab's existing :start_sources level path link to PythonLab. This creates adds it to Extra Links. We also need to know if the level options has an edit_blocks value which is normally part of app options. I copy the value into Lab2's limited app_options so that we can access it using the front end's PartialAppOptions interface.

Initial Sources

I created a new React hook for determining the correct initial sources to use when loading a level/project. If we are in start mode, we will always get the level sources (or hard-coded default project). If it's not start mode, we'll try to get the project sources if they exist. Note: This PR does not add a working start mode to WebLab2, but I've implemented this hook there in order to make that eventual work easier.

Skipping the Project Manager and channel id

Start Mode should not use the user's channel id. In fact, we realized we could completely skip creating the project manager before calling setProjectAndLevelData in lab2Redux.ts. Fortunately, things already seem to be set up well to handle things if we don't have a project manager or channel. For this context, it means that levelbuilders can edit, run, and save start sources code without using or impacting their personal project code. Because we don't have a Start Over button., this may make it feel a little broken right now. However, this gives us parity with the normal levelbuilder experience of resetting your workspace after editing start code.

Save Button

To enabling saving start sources, I added one useEffect to track any updates to the active lab source. I re-used the button from JavaLab here.

Links

Jira: https://codedotorg.atlassian.net/browse/CT-548
Spec: https://docs.google.com/document/d/1TYuIUXuIBQCf-_JgZl_Q94MhP8c85YrAW6Qf4LgTacs/edit?pli=1

Testing story

Deployment strategy

Follow-up work

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

@mikeharv mikeharv marked this pull request as ready for review May 3, 2024 20:13
@mikeharv mikeharv requested a review from a team May 3, 2024 20:13
Copy link
Contributor

@molly-moen molly-moen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, a couple small comments!

apps/src/lab2/lab2Redux.ts Outdated Show resolved Hide resolved
@@ -123,6 +126,18 @@ export const setUpWithLevel = createAsyncThunk(
return;
}

// Start mode doesn't use channel ids so we can skip creating
// a project manager and just set the level data.
const appOptions = getScriptData('appoptions') as PartialAppOptions;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mind moving the app options related logic into lab2/utils.ts? We have similar methods there. Then we can have a isStartMode method that's a little cleaner.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created a getAppOptionsEditBlocks that returns the string if it's available. Also created a START_SOURCES constant for use in the components. This seemed to be more useful than just creating getAppOptionsStartSources or something, since Lab2 will likely need to work with other modes at some point too.

apps/src/lab2/types.ts Show resolved Hide resolved
useEffect(() => {
if (isStartMode) {
header.showLevelBuilderSaveButton(() => {
return {source: sourceRef.current};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why can't we use source here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, it turns out we can. At one point, because I had never worked with useEffect before, I was trying to add all of this logic to a single useEffect call like this:

useEffect(() => {
  if (isStartMode) {
    header.showLevelBuilderSaveButton(() => {
      return {source};
    });
  }
  dispatch(setSource(initialSources?.source as MultiFileSource));
}, [channelId, dispatch, initialSources, isStartMode, source]);

This was causing me to not be able to edit the code in the editor at all. I then tried the sourceRef trick, so that the above code didn't need to depend on source at all, which seemed to work. Later, I moved the isStartMode block to it's own useEffect call since that felt cleaner. Apparently if I had started with this, it would have just worked. tl;dr: React rookie

@mikeharv mikeharv merged commit d879878 into staging May 6, 2024
2 checks passed
@mikeharv mikeharv deleted the mike/start-mode-python branch May 6, 2024 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants