[feature] UEPR-252 Implement manual update of project thumbnails #264
Conversation
There was a problem hiding this comment.
Pull Request Overview
Enable optional manual control over when project thumbnails are captured and saved, reducing automatic thumbnail updates.
- Extracted thumbnail capture and storage logic into a reusable module (
store-project-thumbnail.js). - Updated project saver HOC to respect a
manuallySaveThumbnailsflag and delegate to the new module. - Added a “Set Thumbnail” button in the stage header (with throttle) and propagated the manual-save prop through GUI components.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/scratch-gui/src/lib/store-project-thumbnail.js | Added storeProjectThumbnail and getProjectThumbnail helpers. |
| packages/scratch-gui/src/lib/project-saver-hoc.jsx | Switched to new thumbnail module, removed old methods, added flag. |
| packages/scratch-gui/src/components/stage-wrapper/stage-wrapper.jsx | Passed manuallySaveThumbnails and onUpdateProjectThumbnail props to header. |
| packages/scratch-gui/src/components/stage-header/stage-header.jsx | Introduced "Set Thumbnail" button, throttle logic, and new props. |
| packages/scratch-gui/src/components/stage-header/stage-header.css | Added styling for the thumbnail button under .rightSection. |
| packages/scratch-gui/src/components/gui/gui.jsx | Propagated manuallySaveThumbnails flag and update handler props. |
Comments suppressed due to low confidence (3)
packages/scratch-gui/src/lib/project-saver-hoc.jsx:85
- dataURItoBlob is used here but not imported in this file, which will cause a ReferenceError at runtime. Please add
import dataURItoBlob from '../lib/data-uri-to-blob';(or correct relative path) at the top.
dataURItoBlob(dataURI)
packages/scratch-gui/src/components/stage-header/stage-header.css:82
- [nitpick] This CSS file uses nested selectors (e.g.,
.setThumbnailButtoninside.rightSection) which isn’t valid in plain CSS. Flatten or ensure your build supports nested syntax to guarantee these rules are applied.
.rightSection {
packages/scratch-gui/src/components/gui/gui.jsx:492
- [nitpick] Consider adding a defaultProp for onUpdateProjectThumbnail (e.g., a no-op function) to avoid undefined checks and ensure the handler is always callable downstream.
onUpdateProjectThumbnail: PropTypes.func,
| @@ -128,6 +129,7 @@ const GUIComponent = props => { | |||
| onTelemetryModalCancel, | |||
| onTelemetryModalOptIn, | |||
| onTelemetryModalOptOut, | |||
| onUpdateProjectThumbnail, | |||
There was a problem hiding this comment.
Thumbnail saves are currently done through the Storage interface - there is a saveProjectThumbnail method that is currently called. Is that still called to save the thumbnail to the server? And this is just to notify that the user saved it, right?
There was a problem hiding this comment.
As discussed offline, the store's saveProjectThumbnail is used as fallback when the onUpdate callback isn't passed - which is the case in NGP
|
|
||
| export const storeProjectThumbnail = (vm, callback) => { | ||
| try { | ||
| getProjectThumbnail(vm, callback); |
There was a problem hiding this comment.
"store" calls "get"? Hm?
Does this also save the project thumbnail to the backend?
There was a problem hiding this comment.
No, the callback saves it, the vm makes a snapshot of the current state.
|
TODO: revert c48f364, as passing the Storage.saveThumbnail default in the projectSaverHOC should be enough to propagate it to the stage header |
Resolves
https://scratchfoundation.atlassian.net/browse/UEPR-252
Proposed Changes
Update logic, so that thumbnails are now updated manually
Reason for Changes
This will hopefully allow us to reduce monthly costs connected with thumbnail updates