Make it easier to work with instructions - #1580
Merged
Merged
Conversation
The markdown string format is what code classroom uses, the array with content is used by the projects site and has pre-rendered html
At the moment these instructions can only be passed as a web component attribute rather than as project props. Decouple the type of instruction from where it came from.
Previously WebComponentProject converted project instructions to HTML with marked.parse before storing them in the instructions slice, so redux only ever held pre-rendered markup. That made the raw source harder to work with for editing since it led to a more complex chain of state updates. This change stores the unconverted markdown string in the instructions slice instead, and moves the marked.parse call (along with its custom target=_blank link renderer) into InstructionsPanel, which now converts to HTML immediately before rendering each step. Pre-rendered HTML content from authored lesson JSON continues to pass through marked.parse unchanged, so no other call sites needed updating. Updated WebComponentProject and InstructionsPanel tests to match: the dispatched payload now carries raw markdown, and the target=_blank link conversion is verified where the conversion now happens.
Previously every instruction step went through marked.parse in InstructionsPanel, relying on markdown parsing being a safe no-op passthrough for pre-rendered HTML from authored lesson JSON. That coupling was accidental: there was no explicit signal for which steps needed markdown conversion and which were already HTML. This change renames the key used for the single-string project instructions case from `content` to `markdown_content` in WebComponentProject, and teaches InstructionsPanel to branch on the key present on a step: `content` is displayed as-is, while `markdown_content` is run through marked.parse. This makes the distinction explicit rather than relying on marked's HTML passthrough behaviour.
Previously WebComponentProject held a useEffect that dispatched setInstructions into the instructions slice whenever editor.project.instructions changed. Since editing instructions writes to editor.project.instructions on every keystroke, this fired a redux action on every keystroke too, just to keep a derived copy of the same data in sync. This change replaces that effect with selectInstructionSteps, a memoized reselect selector (via @reduxjs/toolkit) that computes the steps to display directly from editor.project.instructions, instructions.permitOverride, and any steps already loaded into the slice (e.g. by WebComponentLoader for pre-authored lessons). Nothing is dispatched to derive it, so InstructionsPanel and ProgressBar can both read the same selector without WebComponentProject needing to run first or re-run on every edit. Alternatively the sync logic could have moved into InstructionsPanel directly, but ProgressBar (and any future reader) needs the same derived steps, so a shared selector avoids duplicating the branching logic per component.
Previously InstructionsPanel mixed two concerns in one component: the
surrounding UI (buttons, tabs, empty state, progress bar) and current
step management, alongside the actual rendering of a step's HTML
(markdown conversion, syntax highlighting, scratchblocks, quiz-ready
signalling) into a ref'd DOM node. That made the file large and meant
quiz questions were rendered through ad-hoc branching rather than a
reusable path.
This change extracts InstructionsStep, a component responsible only
for rendering a single step (or a quiz question, passed in the same
{content} shape) into its own DOM node. InstructionsPanel now computes
which step to show (the real current step, or a synthesized
{content: quiz.questions[...]} step while a quiz is active) and passes
it down, keeping its own responsibility to managing state and the
current step.
This also let two pieces of incidental complexity go: react-tabs
mounts each InstructionsStep instance fresh when its tab is selected,
so the instructionsTab dependency previously needed to force the
content effect to re-run is no longer necessary, and Prism's one-time
config now lives with the only component that renders highlighted code.
Splitting out a dedicated step component also sets up the next step:
editing a single instruction, rather than the whole project
instructions string, will live in InstructionsStep rather than
requiring changes to the surrounding panel.
Test coverage for step rendering (markdown conversion, syntax
highlighting, scratchblocks, quiz-ready signalling) moved to a new
InstructionsStep.test.jsx; InstructionsPanel.test.jsx keeps a mix of
panel-level tests plus one representative example of each moved
behaviour to confirm the wiring still works end-to-end.
zetter-rpf
force-pushed
the
instructions-refactor
branch
from
August 7, 2026 13:18
739561f to
d870ae4
Compare
zetter-rpf
temporarily deployed
to
previews/1580/merge
August 7, 2026 13:18 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Sidebar decided whether to show the instructions menu option by reading state.instructions.project.steps directly, rather than through selectInstructionSteps. That state is only populated when steps are loaded pre-authored (e.g. by WebComponentLoader); for a single-page project whose instructions are a markdown string in editor.project.instructions, it stays empty, so Sidebar concluded there were no instructions and hid the panel entirely. This change points Sidebar at selectInstructionSteps, the same derived source InstructionsPanel and ProgressBar already use, so all three agree on whether instructions exist. Also hardened the selector itself with optional chaining on state.instructions, since several existing tests render Sidebar/Project/MobileProject without an instructions slice in their mock store at all. Added a regression test reproducing the exact scenario (markdown string instructions, default permitOverride) and confirmed it fails without the Sidebar fix and passes with it.
zetter-rpf
marked this pull request as ready for review
August 7, 2026 14:16
zetter-rpf
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors instruction selection and rendering to support Markdown strings and authored multi-step HTML instructions.
Changes:
- Adds a memoized instruction-step selector.
- Extracts instruction rendering into a tested component.
- Corrects example-project instruction formats.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/redux/InstructionsSlice.js |
Adds the instruction-step selector. |
src/redux/InstructionsSlice.test.js |
Tests instruction selection behavior. |
src/projects/cool-scratch.json |
Corrects instruction format. |
src/projects/cool-python.json |
Corrects Markdown instruction format. |
src/projects/cool-html.json |
Demonstrates multi-step HTML instructions. |
src/components/WebComponentProject/WebComponentProject.jsx |
Removes instruction preprocessing. |
src/components/WebComponentProject/WebComponentProject.test.jsx |
Removes obsolete preprocessing tests. |
src/components/Menus/Sidebar/InstructionsPanel/ProgressBar/ProgressBar.jsx |
Uses selected instruction steps. |
src/components/Menus/Sidebar/InstructionsPanel/ProgressBar/ProgressBar.test.jsx |
Updates test state. |
src/components/Menus/Sidebar/InstructionsPanel/InstructionsStep/InstructionsStep.jsx |
Adds instruction rendering component. |
src/components/Menus/Sidebar/InstructionsPanel/InstructionsStep/InstructionsStep.test.jsx |
Tests rendering and integrations. |
src/components/Menus/Sidebar/InstructionsPanel/InstructionsPanel.jsx |
Delegates step rendering. |
src/components/Menus/Sidebar/InstructionsPanel/InstructionsPanel.test.jsx |
Updates panel-level tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+6
to
+8
| window.HTMLElement.prototype.scrollTo = vi.fn(); | ||
| vi.mock("../../../../../utils/scratchblocks", () => ({ | ||
| scratchblocksInit: vi.fn(), |
Contributor
Author
There was a problem hiding this comment.
This is wrong - we are migrating
| if (!stepContent.current || !step) return; | ||
|
|
||
| stepContent.current.parentElement?.scrollTo({ top: 0 }); | ||
| stepContent.current.innerHTML = getStepHtml(step); |
Contributor
Author
There was a problem hiding this comment.
yes, this is an existing problem that I plan on fixing seperately
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Done to make https://github.com/RaspberryPiFoundation/digital-editor-issues/issues/1681 easier
This are some refactors and improvements to instructions that:
See commits for more
Made with help from Claude