Skip to content

feat: [UEPR-252] add manual save of thumnails #9638

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Jul 22, 2025
Merged

feat: [UEPR-252] add manual save of thumnails #9638

merged 13 commits into from
Jul 22, 2025

Conversation

KManolov3
Copy link
Contributor

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

@KManolov3 KManolov3 requested review from Copilot, MiroslavDionisiev and cwillisf and removed request for Copilot and MiroslavDionisiev July 15, 2025 09:14
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a manual thumbnail-saving flow by adding a feature flag, an informational modal on first use, and routing the thumbnail update through a custom handler.

  • Add MANUALLY_SAVE_THUMBNAILS env var in Webpack and pass it through to the client
  • Create a tooltip and UpdateThumbnailInfoModal to explain manual saves on first use
  • Update Preview and PreviewPresentation to use a handleManualThumbnailUpdate path and pass the new flag

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
webpack.config.js Inject MANUALLY_SAVE_THUMBNAILS default into client env
src/views/preview/update-thumbnail-info-modal.scss Styles for the first-use info modal
src/views/preview/update-thumbnail-info-modal.jsx New modal component explaining manual thumbnail saves
src/views/preview/project-view.scss Tooltip styling and driver.js overrides for manual-save hint
src/views/preview/project-view.jsx Wire up handleManualThumbnailUpdate, tooltip, modal, and flag
src/views/preview/presentation.jsx Pass manuallySaveThumbnails (and userOwnsProject) to GUI
src/views/preview/l10n.json Add localized strings for tooltip and info modal
Comments suppressed due to low confidence (1)

src/views/preview/project-view.jsx:852

  • The new handleManualThumbnailUpdate flow, including first-time tooltip and modal logic, doesn’t have any unit or integration tests. Adding tests for both the initial modal display and subsequent updates will help catch regressions.
    handleManualThumbnailUpdate (id, blob) {

Copy link
Contributor

@cwillisf cwillisf left a comment

Choose a reason for hiding this comment

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

I commented on a couple nits, but overall this looks great!

@@ -414,6 +415,8 @@ const PreviewPresentation = ({
onUpdateProjectId={onUpdateProjectId}
onUpdateProjectThumbnail={onUpdateProjectThumbnail}
shouldStopProject={shouldStopProject}
manuallySaveThumbnails={manuallySaveThumbnails}
userOwnsProject={userOwnsProject}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this flag actually used? I don't see it mentioned anywhere else in this PR...

Copy link
Contributor Author

@KManolov3 KManolov3 Jul 18, 2025

Choose a reason for hiding this comment

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

It's used in the editor to make sure we only show the update thumbnail button if the project is created by us:
scratchfoundation/scratch-editor#264

@KManolov3 KManolov3 merged commit 7213efc into develop Jul 22, 2025
5 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants