Skip to content

Refactor to split scratch.jsx into clearer parts#1439

Merged
zetter-rpf merged 2 commits into
mainfrom
split-scratch-jsx-refactor
Apr 21, 2026
Merged

Refactor to split scratch.jsx into clearer parts#1439
zetter-rpf merged 2 commits into
mainfrom
split-scratch-jsx-refactor

Conversation

@zetter-rpf

@zetter-rpf zetter-rpf commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

Related to: https://github.com/RaspberryPiFoundation/digital-editor-issues/issues/1197

Refactor to split scratch.jsx now that it's responsibilities have grown and simplify tests

In https://github.com/RaspberryPiFoundation/digital-editor-issues/issues/1197 I'll need to update the access token that's passed into Scratch. This should make that easier.

I've kept scratch.jsx as responsible for initializing the page and setting up authentication. ScratchEditor is responsible for rendering the Scratch editor component and setting up the handlers it needs.

I've also extracted WrappedScratchGui as it makes the tests easier to set up when you don't need to mock the process of setting up the higher order component.

@zetter-rpf zetter-rpf temporarily deployed to previews/1439/merge April 17, 2026 13:20 — with GitHub Actions Inactive
@zetter-rpf zetter-rpf force-pushed the split-scratch-jsx-refactor branch from 4e212ba to b522686 Compare April 17, 2026 13:20
@zetter-rpf zetter-rpf temporarily deployed to previews/1439/merge April 17, 2026 13:21 — with GitHub Actions Inactive
@zetter-rpf zetter-rpf marked this pull request as ready for review April 20, 2026 08:14
Copilot AI review requested due to automatic review settings April 20, 2026 08:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Refactors the Scratch iframe entrypoint by extracting Scratch editor rendering, GUI wrapper setup, and parent-window postMessage helpers into dedicated modules to simplify future auth-token changes and reduce test mocking complexity.

Changes:

  • Split Scratch GUI setup into ScratchEditor + WrappedScratchGui components.
  • Extracted postScratchGuiEvent into a shared events.js helper for iframe-to-parent messaging.
  • Updated/refocused tests: handshake tests stay in scratch.test.js, while save/metadata behavior moves to new ScratchEditor tests.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/scratch.jsx Entry-point now mounts ScratchEditor and imports shared postMessage helper.
src/scratch.test.js Simplifies mocks and asserts accessToken is passed into ScratchEditor.
src/components/ScratchEditor/ScratchEditor.jsx New component encapsulating Scratch GUI props, event wiring, and save behavior.
src/components/ScratchEditor/ScratchEditor.test.jsx New unit tests for editor render and save/metadata wiring.
src/components/ScratchEditor/WrappedScratchGui.jsx New wrapper composing Scratch GUI HOCs and setting modal app element.
src/components/ScratchEditor/events.js New helper for posting Scratch GUI events to the parent origin.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/components/ScratchEditor/ScratchEditor.jsx Outdated
Comment thread src/components/ScratchEditor/ScratchEditor.jsx Outdated
Comment thread src/components/ScratchEditor/ScratchEditor.test.jsx Outdated
Comment thread src/components/ScratchEditor/WrappedScratchGui.jsx Outdated
Refactor to split scratch.jsx now that it's responsibilities have grown.

I've kept scratch.jsx as responsible for initializing the page and setting up authentication. ScratchEditor is responsible for rendering the Scratch editor component and setting up the handlers it needs.

I've also extracted WrappedScratchGui as it makes the tests a lot easier to set up when you don't need to mock the process of setting up the higher order component.
@zetter-rpf zetter-rpf force-pushed the split-scratch-jsx-refactor branch from c58497c to 035e918 Compare April 20, 2026 10:33
@zetter-rpf zetter-rpf temporarily deployed to previews/1439/merge April 20, 2026 10:33 — with GitHub Actions Inactive
Now I've pushed this into the component, it is safer to store it as a ref rather than in a variable.

@DNR500 DNR500 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this, this looks cleaner

@zetter-rpf zetter-rpf merged commit b152adf into main Apr 21, 2026
7 checks passed
@zetter-rpf zetter-rpf deleted the split-scratch-jsx-refactor branch April 21, 2026 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants