Mimic autosave in Scratch Editor#1469
Conversation
abcampo-iry
temporarily deployed
to
previews/1469/merge
GitHub Actions
Inactive
abcampo-iry
force-pushed
the
issues/1212-autosave
branch
from
637b154 to
6a84bbe
Compare
abcampo-iry
temporarily deployed
to
previews/1469/merge
GitHub Actions
Inactive
abcampo-iry
force-pushed
the
issues/1212-autosave
branch
from
6a84bbe to
d6b7a86
Compare
abcampo-iry
temporarily deployed
to
previews/1469/merge
GitHub Actions
Inactive
abcampo-iry
force-pushed
the
issues/1212-autosave
branch
from
d6b7a86 to
0da7d2b
Compare
abcampo-iry
temporarily deployed
to
previews/1469/merge
GitHub Actions
Inactive
abcampo-iry
force-pushed
the
issues/1212-autosave
branch
from
0da7d2b to
dbfdb88
Compare
abcampo-iry
temporarily deployed
to
previews/1469/merge
GitHub Actions
Inactive
abcampo-iry
force-pushed
the
issues/1212-autosave
branch
from
dbfdb88 to
1145479
Compare
abcampo-iry
temporarily deployed
to
previews/1469/merge
GitHub Actions
Inactive
abcampo-iry
force-pushed
the
issues/1212-autosave
branch
from
1145479 to
b205bb8
Compare
abcampo-iry
temporarily deployed
to
previews/1469/merge
GitHub Actions
Inactive
There was a problem hiding this comment.
Pull request overview
This PR adds Scratch autosave by forwarding Scratch VM change events from the iframe to the main editor and reusing the shared Redux save-status state/UI.
Changes:
- Adds Scratch save lifecycle Redux actions and tests.
- Updates Scratch save hooks and project bar behavior to hide manual Save once autosave is eligible.
- Adds Scratch iframe change-event forwarding and updates unit/Cypress coverage for the new autosave flow.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/redux/EditorSlice.js |
Adds Scratch-specific save state actions. |
src/redux/EditorSlice.test.js |
Covers Scratch save timestamp and lifecycle reducers. |
src/hooks/useScratchSaveState.js |
Implements Scratch autosave scheduling and save message handling. |
src/hooks/useScratchSaveState.test.js |
Tests autosave debounce, queueing, and Redux state updates. |
src/hooks/useScratchSave.js |
Enables autosave only for eligible Scratch projects. |
src/components/ScratchEditor/ScratchIntegrationHOC.jsx |
Emits project-changed events from Scratch VM changes/uploads. |
src/components/ScratchEditor/ScratchIntegrationHOC.test.jsx |
Tests Scratch VM project-changed event forwarding. |
src/components/ProjectBar/ScratchProjectBar.jsx |
Switches saved Scratch projects from manual Save to SaveStatus. |
src/components/ProjectBar/ScratchProjectBar.test.js |
Tests Scratch save button/status/autosave behavior. |
cypress/e2e/spec-scratch.cy.js |
Updates Scratch remix flow E2E coverage for autosave. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
abcampo-iry
temporarily deployed
to
previews/1469/merge
GitHub Actions
Inactive
abcampo-iry
temporarily deployed
to
previews/1469/merge
GitHub Actions
Inactive
|
I see no issues but due to the impact of this change should have an another engineer review. non-blocking: We should discuss how we are handling the auto-save time. See Thread . |
| this.props.vm?.on?.("PROJECT_CHANGED", this.handleProjectChanged); | ||
| this.props.vm?.on?.("targetsUpdate", this.handleTargetsUpdate); |
There was a problem hiding this comment.
What is a target in this context and when does it fire (and a project change doesn't?)
Also I can see your guarding this for when the vm doesn't exist, or it has no on function - why might this happen? May autosave not work in some cases?
There was a problem hiding this comment.
I think you are right, on the "target in this context", I was thinking it needed a wider scope that's why i binded both I think I can make it with only project_changed.
On the on, its more defensive, but I would bet is probably ok without.
There was a problem hiding this comment.
On the on, its more defensive, but I would bet is probably ok without.
Sometimes being defensive is less helpful - it can mask unexpected errors and hide important states to cover. For example here, if vm didn't exist, the code would mean that this bit ran fine, but I wasn't sure if the auto save would work or if the user could save at all. If this could happen, it would be useful to have more tests around.
zetter-rpf
left a comment
zetter-rpf
left a comment
There was a problem hiding this comment.
Nice one,
I did find the changes in the project bar and hooks complex and haven't gone through them line by line, but I can see it works well and has good test coverage.
I just had couple of questions to help my understanding that might be making sure are clearer in the commit/pr/code.
This change adds autosave for Scratch projects and keeps the behaviour close to the existing Python and HTML editors. Python and HTML already autosave after a short delay when the user owns a saved project. Scratch is different because project changes happen inside the Scratch iframe, so we listen to Scratch VM change events and pass those changes back to the main app. Autosave only starts once the Scratch project has been saved or remixed and has an identifier. New Scratch projects, and projects that need to be remixed first, still show the manual Save button. After that first save/remix, the manual Save button is hidden and the header shows the autosave status instead. The autosave delay is 2 seconds. If a save is already running, we do not start another one immediately; we queue the next autosave until the current save finishes. Scratch save events now update the shared Redux editor save state: saving, lastSavedTime, and lastSaveAutosave. This is the same state used by the existing save status UI, so Scratch can reuse SaveStatus instead of keeping a separate local save-status flow in the Scratch hook. The Scratch save/remix tests have also been updated to cover the new flow: first save/remix, identifier update, hiding the manual Save button, and triggering autosave once the project is eligible. As Teacher: https://github.com/user-attachments/assets/ee9010bc-0ae8-4cb7-953e-eeaea70253ec As Student (first time): https://github.com/user-attachments/assets/60bb464b-6bb6-4fc9-9131-d9c84994496d
Closes: https://github.com/RaspberryPiFoundation/digital-editor-issues/issues/1212
This change adds autosave for Scratch projects and keeps the behaviour close to the existing Python and HTML editors.
Python and HTML already autosave after a short delay when the user owns a saved project. Scratch is different because project changes happen inside the Scratch iframe, so we listen to Scratch VM change events and pass those changes back to the main app.
Autosave only starts once the Scratch project has been saved or remixed and has an identifier. New Scratch projects, and projects that need to be remixed first, still show the manual Save button. After that first save/remix, the manual Save button is hidden and the header shows the autosave status instead.
The autosave delay is 2 seconds. If a save is already running, we do not start another one immediately; we queue the next autosave until the current save finishes.
Scratch save events now update the shared Redux editor save state: saving, lastSavedTime, and lastSaveAutosave. This is the same state used by the existing save status UI, so Scratch can reuse SaveStatus instead of keeping a separate local save-status flow in the Scratch hook.
The Scratch save/remix tests have also been updated to cover the new flow: first save/remix, identifier update, hiding the manual Save button, and triggering autosave once the project is eligible.
As Teacher:
Screen.Recording.2026-05-15.at.15.57.12.mov
As Student (first time):
Screen.Recording.2026-05-15.at.15.58.36.mov