Add warning banner when editing start blocks for a level with a template #50082
Conversation
apps/i18n/common/en_us.json
Outdated
| @@ -2071,6 +2071,7 @@ | |||
| "standardsReportHeader": "Class Standards Report", | |||
| "standardsReportNoUnpluggedLessons": "There are no unplugged lessons in this course.", | |||
| "standardsReportLessonLengthInfo": "*Lessons in this course offer between 45 and 65 minutes of instruction", | |||
| "startBlocksTemplateWarning": "WARNING: You are editing start blocks for a level with a template. Any start blocks defined here will be overwritten by the template.", | |||
There was a problem hiding this comment.
Any start blocks defined here will be overwritten by the template.
Strictly, I think this is only true when there are start blocks defined in the template. It's possible to define per-level start blocks in a templated progression if you leave them out of the template, but we don't think that you should do that.
Possible alternative:
"WARNING: You are editing start blocks for a level with a template. Start blocks should be defined in the template level."
There was a problem hiding this comment.
Should there still be a notification that if start blocks are defined in the template, per-level start blocks are over-written?
There was a problem hiding this comment.
I actually don't think we should say "overwritten" anywhere because that implies to me that we'll be erasing something. If start blocks are defined in both places, we simply ignore any that are defined in the non-template levels.
There was a problem hiding this comment.
Then should we remind levelbuilders that the level start blocks will be ignored if there are start blocks defined in the template?
There was a problem hiding this comment.
We do this already near the field on level edit pages where templates are added:
I think this banner is primarily meant to be a reminder that you're editing start blocks (rather than working in the level as a student), and secondarily as an extra warning to be careful if you're using a template. As long as the message we add to the banner is accurate and helpful, I don't think it has to explain every detail about the behavior.
There was a problem hiding this comment.
👍 I like your text Mike, I could only make the string so long without it spilling onto a second line and making the banner too big. Hopefully the clarification on the edit page is enough. This is mostly to give a reminder.
| @@ -39,7 +40,8 @@ class CodeWorkspace extends React.Component { | |||
| showMakerToggle: PropTypes.bool, | |||
| autogenerateML: PropTypes.func, | |||
| closeWorkspaceAlert: PropTypes.func, | |||
| workspaceAlert: PropTypes.object | |||
| workspaceAlert: PropTypes.object, | |||
| isProjectTemplateLevel: PropTypes.bool | |||
There was a problem hiding this comment.
I know you didn't choose the name for this property, but to me it suggests that the level is a template (rather than that it uses one).
I feel like hasTemplate or levelHasTemplate better describes what I think this property means.
There was a problem hiding this comment.
yep I agree, I'm going to leave as-is because changing it would be pretty high risk
If a level has a template, we will now show a warning banner when editing start blocks or start sources. In this previous PR I added a banner to indicate that you are in start blocks mode in Blockly. That banner will now also show up in droplet labs and in Java Lab (although the styling is slightly different in Java Lab.
Screenshots
Start Blocks, Blockly (unchanged)
Start Blocks with template, Blockly
Start Blocks, Droplet
Looks the same as Blockly, but it required a minor CSS change to push down the workspace so I'm including a screenshot
Start Blocks with template, Droplet
Start Sources, Java Lab
The Java Lab banners use the same styling as the code review warning banner, which is a darker yellow
Start Sources with template, Java Lab
Links
Testing story
Tested locally
Follow-up work
As of now this is the only improvement planned in this space. We had discussed making start blocks readonly if the level had a template, but this opened up a can of worms and didn't work for every lab. Slack thread with more details.
PR Checklist: