[TomNaessens]

Review map: dodona-edu/dodona#9402 (step 3 of 7: what each pull request does, the review order and live status)

This pull request adds <p-code-playground>: a small runnable editor for code blocks in Dodona's reading activities, moved from Dodona's <dw-code-playground> into Papyros. It is step 3 of 5; see #1168 for the overview.

Stacked on #1168. Dodona side: dodona-edu/dodona#9304 replaces its component with a thin wrapper around this one.

How to review

Most of CodePlayground.ts is a port of Dodona's component, reviewed in dodona-edu/dodona#8648 (including a WCAG pass). The useful view is the diff against that original:

git -C ../dodona show bfa2d95c4:app/assets/javascript/components/code_playground/code_playground.ts > /tmp/dodona-playground.ts
git diff --no-index /tmp/dodona-playground.ts src/frontend/components/CodePlayground.ts

About 200 of its added lines are the styles, moved from Dodona's code_playground.css.scss into the component and switched from --d-* to the --md-sys-color-* tokens Dodona already maps.

What changed in the port:

• Each playground is its own Papyros instance on a shared runtime (Let several Papyros instances share one runtime #1168), so Dodona's scheduler is gone: output comes from the instance's own io, and "another playground is running" is read from the runtime.
• The editor is the sandbox's p-code, bound to the instance's runner.code; errors and images render through the shared helpers from Share rendering and helpers between the sandbox components #1169; the status line shows runner.stateMessage.
• Shadow DOM instead of Dodona's light DOM.
• Typed input is not echoed into the output, the same as in the sandbox. Dodona's version did echo it; that was a design choice at the time, not a requirement.
• Launching when the first playground comes into view is ported from dodona-edu/dodona#9214 (playground/preload.ts).

Kept as in Dodona: Run/Stop with aria-disabled (never native disabled, which drops focus), the transcript as role="log", focus moves, the reserved output height so the page doesn't jump, and scroll-follow only when already at the bottom (through p-output's FollowEnd controller from #1169, so an image that gets its height late no longer stops it).

Manual testing instructions

1. yarn vitest run --exclude '**/.claude/**'
2. Through dodona-edu/dodona#9304 with a local build of this branch: a page with several playgrounds runs each block, gates the others while one runs, and Stop, Reset, input() and tracebacks work.

Screenshot (Dodona test page)

Checklist

• Tests: CodePlayground.test.ts (ported from Dodona, plus the preload); full suite green locally (466)
• Accessibility: behaviour and its tests ported from the audited Dodona component
• Docs: README "Code playgrounds"
• AI: Claude Code wrote the change

🤖 Generated with Claude Code

[bmesuere]

🤖 Reviewed this PR against its stack base. One reproducible issue is described inline. The additional regression check reproduces it with a real Python run. Validation of the complete stack at 530ba26: 334 existing tests passed; TypeScript checking, ESLint, and the app build passed.

[TomNaessens]

🤖 Rebase notes for #1194 (reviewed at 910fb15). The conflicts at this level come from #1168 and #1169; the notes there have the details.

• CodePlayground.ts calls start(RunMode.Run, []). Once start() takes one options object (the note on Let several Papyros instances share one runtime #1168), this becomes start(RunMode.Run, { files: [] }).
• Library.ts: keep Move the run buttons into the tab bar, add a host tab and compact the input #1194's exports (CODE_TAB, HOST_TAB, type RunOptions) next to CodePlayground.
• The playground has no p-code-runner, so Move the run buttons into the tab bar, add a host tab and compact the input #1194's papyros-code-runner container queries and the host tab don't apply to it.

[TomNaessens]

🤖 Rebase notes for #1195 (reviewed at a5eedf7). The conflicts at this level come from #1169; the note there has the details. This one is about a design overlap, not a conflict.

• Two ways to follow the end of the output. PlaygroundOutput reads whether the log is at the end in hostUpdate() (tolerance 2 px) and scrolls to the end in hostUpdated(). Make the output a growing panel and put the debugger stepper in a header bar #1195 gives p-output a different mechanism: a following flag that only the reader's own scrolling changes, plus a ResizeObserver on the content and its children.
• The playground likely has the bug Make the output a growing panel and put the debugger stepper in a header bar #1195 fixed. bmesuere's review on Make the output a growing panel and put the debugger stepper in a header bar #1195 reproduced it in p-output: an inline image gets its height after the render that inserts it, so the scroll to the end lands short, and the next render reads "not at the end" and stops following. The playground renders matplotlib images inline through renderImage in a .output capped at 300px and only scrolls at render time, so it most likely does the same. Not reproduced in the playground yet.
• The other way round: Make the output a growing panel and put the debugger stepper in a header bar #1195's flag is never reset when a run empties the output (left as a finding on Make the output a growing panel and put the debugger stepper in a header bar #1195), which the playground's read-before-render approach avoids.
• Suggestion: once Make the output a growing panel and put the debugger stepper in a header bar #1195 is in, move one follow-the-end helper into output/renderOutput.ts (or a small reactive controller next to it) and use it in both p-output and PlaygroundOutput: read "at the end" before the render, as the playground does, and keep the ResizeObserver from Make the output a growing panel and put the debugger stepper in a header bar #1195 for late image heights.

[TomNaessens]

🤖 todo (non-blocking): once #1196 is merged, the README has a host contract for Escape: Papyros calls preventDefault() on the Escape keydowns it acts on (Escape followed by Tab out of the code editor, and the rename and add-file inputs), so a host can close its own UI only on an Escape whose defaultPrevented is false.

<p-code-playground> embeds p-code-editor, so the contract applies to it too, and Dodona's playgrounds in descriptions (#9304, #9305) will rely on it. When rebasing:

1. If Cancel the Escape keydowns that Papyros acts on #1196 still documents the contract under <p-code-runner> (I suggested moving it to a page-level section there), mention it in the "Code playgrounds" section too.
2. Check that a playground's editor cancels its Escape, like the sandbox editor (the listener lives on p-code-editor, so this should hold as is).
3. The playground's own input (onInputKeydown) doesn't act on Escape, so it should stay uncancelled. Nothing to change there.

[TomNaessens]

🤖 Rebased onto main as 89c1195:

• Move the run buttons into the tab bar, add a host tab and compact the input #1194: start(RunMode.Run, { files: [] }); exports kept.
• Cancel the Escape keydowns that Papyros acts on #1196: the README's "Code playgrounds" section mentions the Escape contract, and a test checks that the playground editor cancels its Escape.
• Make the output a growing panel and put the debugger stepper in a header bar #1195 (follow the end): not part of the rebase; it's next, in a simplify pass. The late-image problem does reproduce in the playground (40 lines plus a 200px image: the log stops 200px short and stops following).

[TomNaessens]

🤖 Simplify pass after the rebase (now 2aa73de):

• The playground follows the end through FollowEnd (Share rendering and helpers between the sandbox components #1169), so an image that gets its height after the render no longer stops it (new test; it failed on the old code).
• The reserved height floor lives in CSS (min-height: max(22px, var(--reserved-height, 0px))).
• The transcript renders its chunks as an array, so a new chunk adds a text node instead of rebuilding the whole text.
• .error-title moved into outputStyles with the markup it styles; dead :disabled selectors and the test-only outputs getter removed.

[TomNaessens]

🤖 note: Papyros.unsupported_browser now reads "Python could not start in this browser. Close other tabs, update your browser or use another one." (nl: "Python kon niet starten in deze browser. Sluit andere tabbladen, werk je browser bij of gebruik een andere browser.").

Dodona reworded its playground message to this on main (a boot failure can also be a device out of memory), and with this PR the playground shows Papyros's own string instead of Dodona's. The sandbox uses the same string, so it changes there too.

[TomNaessens]

🤖 Automated review on Tom's behalf; summary below.

---

Generated by Claude Code

[TomNaessens]

🤖 Minor findings. Plain review (no code-review skill) with a checkout of the PR head. Covers the full PR against its base #1168 (step 3 of the stack in dodona-edu/dodona#9402), compared with Dodona's original component at bfa2d95c4.

Two inline comments: the status spinner ignores reduced motion (a regression from the original), and a default-instance playground drops its errors. Checked and fine:

• listener, observer and FollowEnd cleanup;
• the deferred dispose on removal, and moves within the page;
• the Reset focus hand-off;
• aria-describedby/labelledby ids and role="alert" placement.

Of the original tests, only "keeps transient status out of the toolbar" wasn't ported. The vitest suite wasn't run here: the installed Chromium build doesn't match the one this Playwright expects.

---

Generated by Claude Code

[TomNaessens]

🤖 note: Changed since the approval (4b838eb): fixes for three review-routine findings.

• The status line shows no spinner under prefers-reduced-motion: reduce.
• The playground's input row ignores Enter with keyCode 229 (Safari IME).
• The README says what a playground without .papyros runs with.