Skip to content

Show a code playground's data files as read-only tabs - #1171

Open
TomNaessens wants to merge 1 commit into
feature/shared-output-renderingfrom
chore/component-cleanup
Open

TomNaessens wants to merge 1 commit into
feature/shared-output-renderingfrom
chore/component-cleanup

Conversation

@TomNaessens

@TomNaessens TomNaessens commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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

This pull request lets a code playground declare data files, shown as read-only tabs next to the code, as reviewers asked on dodona-edu/dodona#9211. It is step 4 of 5; see #1168 for the overview.

Stacked on #1170. Dodona side: dodona-edu/dodona#9305 passes the files attribute on.

  • <p-code-playground files="media/grades.txt media/other.csv">: space-separated paths, resolved against the page's base URL and stored in the workspace under their base name. Every Run starts from a workspace holding exactly these files (runner.start(RunMode.Run, files) from Let a run start from a workspace with only the given files #1159), so a run gives the same result every time and two playgrounds can use different files with the same name.
    • A file's preview loads when its tab is first opened. On Run, the playground claims the runtime right away and runner.start() waits for the files (its files option also takes a promise): Stop works while they load and other playgrounds wait. A file that fails to load shows an alert and the run ends without running the code; the next Run or reopening the tab retries.
    • Duplicate base names show the configuration error card.
  • loadFile(url) (state/Files.ts): fetches once per URL (a failure is retried), rejects on an HTTP error, and returns text for valid UTF-8, otherwise base64.
  • readonly on p-editor-tabs, p-editor-tab and p-file-viewer: no add, rename or close, and a read-only editor. With a url, p-file-viewer shows an "Open file" link to it for binary files and text over 100 kB. The sandbox keeps the editable defaults.

Deliberately left out:

  • The sandbox's own URL loading (fetchAndAddUrl, and provideFiles, which still writes HTTP error pages into files): a separate change.
  • Showing files the program writes, and editable file tabs.

Manual testing instructions

  1. yarn vitest run --exclude '**/.claude/**'
  2. Through dodona-edu/dodona#9305: a playground with files="media/grades.txt" shows a grades.txt tab with the file's contents and prints it on Run. A PNG shows the open link; a missing file shows the alert without running.
Screenshots (Dodona test page)
File tab (light) File tab (dark) Binary file
File tab in light mode File tab in dark mode Binary file with open link

Checklist

  • Tests: FileLoader.test.ts, EditorTabs.test.ts and FileViewer.test.ts (read-only modes), the playground files cases (including a real Pyodide run reading grades.txt); full suite green locally (511)
  • Accessibility: reuses the existing tab markup; the file panel is a tabpanel
  • Docs: README "Code playgrounds"
  • AI: Claude Code wrote the change

🤖 Generated with Claude Code

@TomNaessens TomNaessens added the chore Repository/build/dependency maintenance label Sep 29, 2026
@TomNaessens TomNaessens self-assigned this Sep 29, 2026
@TomNaessens
TomNaessens added this pull request to stack #1184 October 7, 2026 11:05
@TomNaessens
TomNaessens force-pushed the chore/component-cleanup branch from df35fb9 to 896f856 Compare October 7, 2026 11:17
@TomNaessens TomNaessens changed the title Share more code between the sandbox and playground components Show a code playground's data files as read-only tabs Oct 7, 2026
@TomNaessens TomNaessens removed the chore Repository/build/dependency maintenance label Oct 7, 2026
@TomNaessens
TomNaessens force-pushed the chore/component-cleanup branch from 896f856 to bf59f6e Compare October 7, 2026 12:28
@TomNaessens
TomNaessens removed this pull request from stack #1184 October 7, 2026 12:38
@TomNaessens
TomNaessens force-pushed the chore/component-cleanup branch from bf59f6e to 1a98c08 Compare October 7, 2026 12:39
@TomNaessens
TomNaessens added this pull request to stack #1186 October 7, 2026 12:39
Comment thread src/frontend/components/CodePlayground.ts Outdated
Comment thread src/frontend/components/CodePlayground.ts Outdated
Comment thread src/frontend/state/Files.ts
@TomNaessens
TomNaessens force-pushed the chore/component-cleanup branch from 1a98c08 to dce80a6 Compare October 7, 2026 13:42
@TomNaessens
TomNaessens force-pushed the chore/component-cleanup branch from dce80a6 to d2c5330 Compare October 7, 2026 13:55
@TomNaessens
TomNaessens marked this pull request as ready for review October 7, 2026 14:32
`;
}
const readonly = this.papyros.debugger.active;
const readonly = this.readonly || this.papyros.debugger.active;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤖 [P2] Ignore editor change events in read-only previews

readonly prevents typing, but changing .value on a reused p-file-editor still dispatches change. Switching between two loaded text previews therefore reaches onEditorChange, which checks only debugger.active, and writes the preview to the backend after 300 ms. If another playground is paused in input(), this can overwrite its files on the shared runtime. I reproduced the running playground printing from B instead of its original from A after switching B's read-only preview. Return early from onEditorChange when this.readonly is true, and cover preview switching without backend writes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤖 Fixed in 85ccb80: onEditorChange returns early when the viewer is read-only, so setting .value on the reused editor while switching previews no longer writes anything to the workspace. Tests: switching between two read-only previews makes no updateFile or updateFileContent call (fails without the guard), and an editable viewer still writes after a switch.

@bmesuere bmesuere left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤖 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
TomNaessens force-pushed the chore/component-cleanup branch from d2c5330 to 85ccb80 Compare October 7, 2026 16:02
@TomNaessens
TomNaessens removed this pull request from stack #1186 October 7, 2026 20:49
@TomNaessens

Copy link
Copy Markdown
Contributor Author

🤖 Rebase notes for #1194 (reviewed at 910fb15). The notes on #1169 and #1168 cover the files below this PR; these are the parts specific to the read-only data-file tabs.

@TomNaessens

Copy link
Copy Markdown
Contributor Author

🤖 todo (non-blocking): once #1196 is merged, it adds test/__tests__/components/FileTabs.test.ts, which tests that Escape in the rename input (p-editor-tab) and in the add-file input (p-add-file-button) is cancelled. These are the same components that EditorTabs.test.ts here covers. When rebasing, fold those two tests into EditorTabs.test.ts and drop FileTabs.test.ts.

No other change is needed. Read-only tabs render neither the rename input nor the add-file button, so they add no Escape that needs cancelling, and the preventDefault() that #1196 adds to onRenameKeydown merges cleanly next to the editable change here.

Comment thread src/frontend/components/FileViewer.ts Outdated
Comment thread src/frontend/components/FileViewer.ts Outdated
Comment thread src/frontend/components/FileViewer.ts Outdated
Comment thread src/frontend/components/playground/files.ts Outdated
@TomNaessens

Copy link
Copy Markdown
Contributor Author

🤖 Rebased onto main as 61e88a6:

@TomNaessens
TomNaessens force-pushed the chore/component-cleanup branch from 61e88a6 to 1663574 Compare October 13, 2026 14:41
@TomNaessens

Copy link
Copy Markdown
Contributor Author

🤖 Simplify pass after the rebase (now 1663574):

  • p-file-viewer: url alone decides between the "Open file" link and the download button; the strings are now generic (Papyros.file_not_previewable, Papyros.open_file); downloadBinary is main's again.
  • p-editor-tabs computes locked = readonly || debugger.active once and passes it to each tab; p-editor-tab no longer reads the debugger.
  • The playground's tab bar had a double line under it (its own hairline plus a border); one left.
  • Main's Runner.filenameFromUrl uses the shared fileNameFromUrl for the decode.

@TomNaessens
TomNaessens force-pushed the chore/component-cleanup branch from 1663574 to 105c39c Compare October 16, 2026 11:12
@TomNaessens
TomNaessens force-pushed the chore/component-cleanup branch from 105c39c to 785a36c Compare October 16, 2026 11:44

@TomNaessens TomNaessens left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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


Generated by Claude Code

Comment thread src/frontend/state/Runner.ts
Comment thread src/frontend/components/FileViewer.ts

Copy link
Copy Markdown
Contributor Author

🤖 Minor findings. Plain review (no code-review skill) with a checkout of the PR head. Covers the full PR against its base #1170 (step 4 of the stack in dodona-edu/dodona#9402).

Two inline comments: Stop during a slow file load ends in a worker restart after 5 s, and the "Open file" link text. Checked and fine:

  • loadFile drops failed fetches from its cache, so a retry really fetches again;
  • strict UTF-8 detection, and chunked base64 for large files;
  • file names and contents render as text (no XSS path);
  • the sandbox keeps its editable tabs;
  • the 100 kB cutoff only applies with a url.

Generated by Claude Code

@TomNaessens
TomNaessens force-pushed the chore/component-cleanup branch from 785a36c to 29353fe Compare October 26, 2026 13:14
@TomNaessens

Copy link
Copy Markdown
Contributor Author

🤖 note: Changed since the approval (29353fe): fixes for two review-routine findings.

  • Stop while the data files still load ends the run at once instead of restarting the shared worker after 5 s.
  • The "Open file" link names the file and says it opens in a new tab (en + nl).

<p-code-playground> takes a files attribute: space-separated paths, resolved
against the document's base URL, that every run starts with. They show as
read-only tabs above the editor and load when their tab is first opened or
when Run needs them; a file that fails to load shows an alert, stops the run
and is retried when its tab is opened again or on the next Run, and
duplicate or invalid names (such as `media/`) are a configuration error. p-editor-tabs, p-editor-tab and
p-file-viewer get a readonly mode for this; the sandbox keeps editable tabs.
Binary files and text over 100 kB are not previewed and link to the file
instead; the link names the file and says it opens in a new tab.

The files option of Runner.start() also takes a promise, so Run claims the
runtime at the click: the run waits for the files in its Loading phase, and
ends without running when they fail to load. Stop ends a run that has not
reached the worker yet on the spot, and the run ignores its files when they
arrive, so a slow file never holds the runtime or restarts the worker.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@TomNaessens
TomNaessens force-pushed the chore/component-cleanup branch from 29353fe to 3184fc0 Compare October 26, 2026 13:29

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants