diff --git a/public/translations/en-US.json b/public/translations/en-US.json index b621ff7f7..c144481ba 100644 --- a/public/translations/en-US.json +++ b/public/translations/en-US.json @@ -259,7 +259,8 @@ }, "saveStatus": { "saving": "Saving", - "saved": "Saved" + "saved": "Saved", + "failed": "Offline - changes not saved" }, "runners": { "HtmlOutput": "HTML output preview" @@ -312,4 +313,4 @@ "common": { "or": "or" } -} \ No newline at end of file +} diff --git a/public/translations/en.json b/public/translations/en.json index 584618c9f..d08c77875 100644 --- a/public/translations/en.json +++ b/public/translations/en.json @@ -259,7 +259,8 @@ }, "saveStatus": { "saving": "Saving", - "saved": "Saved" + "saved": "Saved", + "failed": "Offline - changes not saved" }, "runners": { "HtmlOutput": "HTML Output Preview" diff --git a/src/components/ProjectBar/ScratchProjectBar.jsx b/src/components/ProjectBar/ScratchProjectBar.jsx index 157b40d23..71d3e6856 100644 --- a/src/components/ProjectBar/ScratchProjectBar.jsx +++ b/src/components/ProjectBar/ScratchProjectBar.jsx @@ -8,6 +8,7 @@ import ProjectName from "../ProjectName/ProjectName"; import DownloadButton from "../DownloadButton/DownloadButton"; import UploadButton from "../UploadButton/UploadButton"; import DesignSystemButton from "../DesignSystemButton/DesignSystemButton"; +import SaveStatus from "../SaveStatus/SaveStatus"; import "../../assets/stylesheets/ProjectBar.scss"; import { useScratchSave } from "../../hooks/useScratchSave"; @@ -20,14 +21,24 @@ const ScratchProjectBar = ({ nameEditable = true }) => { const readOnly = useSelector((state) => state.editor.readOnly); const showScratchSaveButton = Boolean(user && !readOnly); const { + isScratchSaveFailed, isScratchSaving, saveScratchProject, - scratchSaveLabelKey, + scratchLastSavedTime, shouldRemixOnSave, } = useScratchSave({ enabled: showScratchSaveButton, }); - const scratchSaveLabel = t(scratchSaveLabelKey); + const hasScratchSaveStatus = Boolean( + isScratchSaving || isScratchSaveFailed || scratchLastSavedTime, + ); + const showScratchSaveStatus = showScratchSaveButton && hasScratchSaveStatus; + let scratchSaveStatus = "success"; + if (isScratchSaveFailed) { + scratchSaveStatus = "failed"; + } else if (isScratchSaving) { + scratchSaveStatus = "pending"; + } if (loading !== "success") { return null; @@ -60,7 +71,7 @@ const ScratchProjectBar = ({ nameEditable = true }) => { saveScratchProject({ shouldRemixOnSave })} - text={scratchSaveLabel} + text={t("header.save")} textAlways icon={} type="primary" @@ -68,6 +79,13 @@ const ScratchProjectBar = ({ nameEditable = true }) => { /> )} + {showScratchSaveStatus && ( + + )} ); diff --git a/src/components/ProjectBar/ScratchProjectBar.test.js b/src/components/ProjectBar/ScratchProjectBar.test.js index d37352580..18997b98a 100644 --- a/src/components/ProjectBar/ScratchProjectBar.test.js +++ b/src/components/ProjectBar/ScratchProjectBar.test.js @@ -17,6 +17,8 @@ jest.mock("react-router-dom", () => ({ useNavigate: () => jest.fn(), })); +jest.useFakeTimers(); + const scratchProject = { name: "Hello world", identifier: "hello-world-project", @@ -59,6 +61,22 @@ const renderScratchProjectBar = (state) => { ); }; +const renderSignedInScratchProjectBar = ({ + project = scratchProject, + editor = {}, + auth = {}, +} = {}) => + renderScratchProjectBar({ + editor: { + project, + ...editor, + }, + auth: { + user, + ...auth, + }, + }); + const getScratchOrigin = () => process.env.ASSETS_URL || window.location.origin; const dispatchScratchMessage = (type, origin = getScratchOrigin()) => { @@ -76,28 +94,26 @@ beforeEach(() => { jest.clearAllMocks(); }); -describe("When project is Scratch", () => { - beforeEach(() => { - postMessageToScratchIframe.mockClear(); - renderScratchProjectBar({ - editor: { - project: scratchProject, - }, - auth: { - user, - }, - }); - }); +afterEach(() => { + jest.clearAllTimers(); +}); +describe("When project is Scratch", () => { test("Upload button shown", () => { + renderSignedInScratchProjectBar(); + expect(screen.queryByText("header.upload")).toBeInTheDocument(); }); test("Download button shown", () => { + renderSignedInScratchProjectBar(); + expect(screen.queryByText("header.download")).toBeInTheDocument(); }); test("clicking Save sends scratch-gui-save message", () => { + renderSignedInScratchProjectBar(); + fireEvent.click(screen.getByRole("button", { name: "header.save" })); expect(postMessageToScratchIframe).toHaveBeenCalledTimes(1); @@ -107,79 +123,90 @@ describe("When project is Scratch", () => { }); test("clicking Save remixes a non-owner Scratch project on the first save", () => { - renderScratchProjectBar({ - editor: { - project: { - ...scratchProject, - user_id: "teacher-id", - }, - }, - auth: { - user, + renderSignedInScratchProjectBar({ + project: { + ...scratchProject, + user_id: "teacher-id", }, }); - fireEvent.click(screen.getAllByRole("button", { name: "header.save" })[1]); + fireEvent.click(screen.getByRole("button", { name: "header.save" })); expect(postMessageToScratchIframe).toHaveBeenCalledWith({ type: "scratch-gui-remix", }); }); + + test("auto-saves after a Scratch project change", () => { + renderSignedInScratchProjectBar(); + + dispatchScratchMessage("scratch-gui-project-changed"); + + act(() => { + jest.advanceTimersByTime(2000); + }); + + expect(postMessageToScratchIframe).toHaveBeenCalledWith({ + type: "scratch-gui-save", + }); + }); }); describe("Additional Scratch manual save states", () => { test("shows the saving state from the scratch save hook", () => { - renderScratchProjectBar({ - editor: { - project: scratchProject, - }, - auth: { - user, - }, - }); + renderSignedInScratchProjectBar(); dispatchScratchMessage("scratch-gui-saving-started"); - expect( - screen.getByRole("button", { name: "saveStatus.saving" }), - ).toBeDisabled(); + expect(screen.getByRole("button", { name: "header.save" })).toBeDisabled(); + expect(screen.getByText(/saveStatus.saving/)).toBeInTheDocument(); }); test("shows the saved state from the scratch save hook", () => { - renderScratchProjectBar({ - editor: { - project: scratchProject, - }, - auth: { - user, - }, - }); + renderSignedInScratchProjectBar(); dispatchScratchMessage("scratch-gui-saving-succeeded"); - expect( - screen.getByRole("button", { name: "saveStatus.saved" }), - ).toBeInTheDocument(); + expect(screen.getByText("saveStatus.saved now")).toBeInTheDocument(); + }); + + test("shows the failed state from the scratch save hook", () => { + renderSignedInScratchProjectBar(); + + dispatchScratchMessage("scratch-gui-saving-failed"); + + expect(screen.getByText("saveStatus.failed")).toBeInTheDocument(); }); test("shows the saving state during a Scratch remix", () => { - renderScratchProjectBar({ - editor: { - project: { - ...scratchProject, - user_id: "teacher-id", - }, - }, - auth: { - user, + renderSignedInScratchProjectBar({ + project: { + ...scratchProject, + user_id: "teacher-id", }, }); dispatchScratchMessage("scratch-gui-remixing-started"); - expect( - screen.getByRole("button", { name: "saveStatus.saving" }), - ).toBeDisabled(); + expect(screen.getByRole("button", { name: "header.save" })).toBeDisabled(); + expect(screen.getByText(/saveStatus.saving/)).toBeInTheDocument(); + }); + + test("does not auto-save a non-owner Scratch project before the first remix save", () => { + renderSignedInScratchProjectBar({ + project: { + ...scratchProject, + user_id: "teacher-id", + }, + }); + + dispatchScratchMessage("scratch-gui-project-changed"); + + act(() => { + jest.advanceTimersByTime(2000); + }); + + expect(postMessageToScratchIframe).not.toHaveBeenCalled(); }); test("does not show save for logged-out Scratch users", () => { diff --git a/src/components/SaveStatus/SaveStatus.jsx b/src/components/SaveStatus/SaveStatus.jsx index 7f504b3c9..82da6febb 100644 --- a/src/components/SaveStatus/SaveStatus.jsx +++ b/src/components/SaveStatus/SaveStatus.jsx @@ -5,26 +5,46 @@ import classNames from "classnames"; import CloudTickIcon from "../../assets/icons/cloud_tick.svg"; import CloudUploadIcon from "../../assets/icons/cloud_upload.svg"; +import InfoIcon from "../../assets/icons/info.svg"; import "../../assets/stylesheets/SaveStatus.scss"; import { formatRelativeTime } from "../../utils/formatRelativeTime"; -const SaveStatus = ({ isMobile = false }) => { +const SaveStatus = ({ + isMobile = false, + saving: savingOverride, + lastSavedTime: lastSavedTimeOverride, + loading: loadingOverride, +}) => { const { t, i18n } = useTranslation(); const locale = i18n.language; const fallbackLng = i18n.options.fallbackLng; - const lastSavedTime = useSelector((state) => state.editor.lastSavedTime); - const saving = useSelector((state) => state.editor.saving); + const reduxLastSavedTime = useSelector((state) => state.editor.lastSavedTime); + const reduxSaving = useSelector((state) => state.editor.saving); const [time, setTime] = useState(Date.now()); - const loading = useSelector((state) => state.editor.loading); + const reduxLoading = useSelector((state) => state.editor.loading); + const lastSavedTime = + lastSavedTimeOverride === undefined + ? reduxLastSavedTime + : lastSavedTimeOverride; + const saving = savingOverride === undefined ? reduxSaving : savingOverride; + const loading = + loadingOverride === undefined ? reduxLoading : loadingOverride; const isPending = saving === "pending"; + const isFailed = saving === "failed"; + const hasStatusToShow = Boolean(isPending || isFailed || lastSavedTime); + const shouldRender = hasStatusToShow && loading === "success"; useEffect(() => { setTime(Date.now()); + if (!lastSavedTime) { + return undefined; + } + const statusTick = setInterval(() => { setTime(Date.now()); }, 10000); @@ -32,36 +52,45 @@ const SaveStatus = ({ isMobile = false }) => { return () => clearInterval(statusTick); }, [lastSavedTime]); + if (!shouldRender) { + return null; + } + return ( - lastSavedTime && - loading === "success" && ( -
- {isPending ? ( - <> -
- -
-
- {t("saveStatus.saving")}… -
- - ) : ( - <> -
- -
-
- {t("saveStatus.saved")}{" "} - {formatRelativeTime(lastSavedTime, time, locale, fallbackLng)} -
- - )} -
- ) +
+ {isFailed ? ( + <> +
+ +
+
{t("saveStatus.failed")}
+ + ) : isPending ? ( + <> +
+ +
+
+ {t("saveStatus.saving")}… +
+ + ) : ( + <> +
+ +
+
+ {t("saveStatus.saved")}{" "} + {formatRelativeTime(lastSavedTime, time, locale, fallbackLng)} +
+ + )} +
); }; diff --git a/src/components/SaveStatus/SaveStatus.test.js b/src/components/SaveStatus/SaveStatus.test.js index e359c0a7e..6127c0572 100644 --- a/src/components/SaveStatus/SaveStatus.test.js +++ b/src/components/SaveStatus/SaveStatus.test.js @@ -36,4 +36,46 @@ describe("With a save button", () => { test("Renders save button", () => { expect(saveStatus).toBeInTheDocument(); }); + + test("Renders pending status when provided by props", () => { + const middlewares = []; + const mockStore = configureStore(middlewares); + store = mockStore({ + editor: { + project: project, + loading: "success", + lastSavedTime: null, + saving: "idle", + }, + }); + + render( + + + , + ); + + expect(screen.queryByText(/saveStatus.saving/)).toBeInTheDocument(); + }); + + test("Renders failed status when provided by props", () => { + const middlewares = []; + const mockStore = configureStore(middlewares); + store = mockStore({ + editor: { + project: project, + loading: "success", + lastSavedTime: null, + saving: "idle", + }, + }); + + render( + + + , + ); + + expect(screen.queryByText("saveStatus.failed")).toBeInTheDocument(); + }); }); diff --git a/src/components/ScratchEditor/ScratchIntegrationHOC.jsx b/src/components/ScratchEditor/ScratchIntegrationHOC.jsx index 22d4b1575..51ef47af6 100644 --- a/src/components/ScratchEditor/ScratchIntegrationHOC.jsx +++ b/src/components/ScratchEditor/ScratchIntegrationHOC.jsx @@ -8,6 +8,10 @@ import { setStageSize, } from "@scratch/scratch-gui"; import { allowedIframeHost } from "../../utils/iframeUtils"; +import { postScratchGuiEvent } from "./events.js"; + +const targetIdsFor = (targets = []) => + targets.map((target) => target.id).join("|"); const ScratchIntegrationHOC = function (WrappedComponent) { class ScratchIntegrationComponent extends React.Component { @@ -19,13 +23,26 @@ const ScratchIntegrationHOC = function (WrappedComponent) { this.handleUpload = this.handleUpload.bind(this); this.handleRemix = this.handleRemix.bind(this); this.handleSave = this.handleSave.bind(this); + this.handleProjectChanged = this.handleProjectChanged.bind(this); + this.handleTargetsUpdate = this.handleTargetsUpdate.bind(this); } componentDidMount() { window.addEventListener("message", this.handleMessage); + this.props.vm?.on?.("PROJECT_CHANGED", this.handleProjectChanged); + this.props.vm?.on?.("targetsUpdate", this.handleTargetsUpdate); + this.previousTargetIds = targetIdsFor(this.props.vm?.runtime?.targets); this.props.setStageSize(); } componentWillUnmount() { window.removeEventListener("message", this.handleMessage); + this.props.vm?.removeListener?.( + "PROJECT_CHANGED", + this.handleProjectChanged, + ); + this.props.vm?.removeListener?.( + "targetsUpdate", + this.handleTargetsUpdate, + ); } handleMessage(event) { @@ -40,7 +57,7 @@ const ScratchIntegrationHOC = function (WrappedComponent) { return; } - switch (event.data.type) { + switch (event.data?.type) { case "scratch-gui-download": this.handleDownload(event); break; @@ -69,9 +86,10 @@ const ScratchIntegrationHOC = function (WrappedComponent) { } handleUpload(event) { const file = event.data.file; - file?.arrayBuffer()?.then((arrayBuffer) => { - this.props.loadProject(arrayBuffer); - }); + file + ?.arrayBuffer() + ?.then((arrayBuffer) => this.props.loadProject(arrayBuffer)) + ?.then(this.handleProjectChanged); } handleRemix() { this.props.onClickRemix(); @@ -79,6 +97,20 @@ const ScratchIntegrationHOC = function (WrappedComponent) { handleSave() { this.props.onClickSave(); } + handleProjectChanged() { + postScratchGuiEvent("scratch-gui-project-changed"); + } + handleTargetsUpdate({ targetList } = {}) { + const targetIds = targetIdsFor(targetList); + if (!targetIds) { + return; + } + + if (this.previousTargetIds && this.previousTargetIds !== targetIds) { + this.handleProjectChanged(); + } + this.previousTargetIds = targetIds; + } render() { const { loadProject, @@ -99,6 +131,7 @@ const ScratchIntegrationHOC = function (WrappedComponent) { state.scratchGui.vm, ), loadProject: state.scratchGui.vm.loadProject.bind(state.scratchGui.vm), + vm: state.scratchGui.vm, }); const mapDispatchToProps = (dispatch) => ({ @@ -113,6 +146,7 @@ const ScratchIntegrationHOC = function (WrappedComponent) { onClickRemix: PropTypes.func, onClickSave: PropTypes.func, setStageSize: PropTypes.func, + vm: PropTypes.object, }; return connect( mapStateToProps, diff --git a/src/components/ScratchEditor/ScratchIntegrationHOC.test.jsx b/src/components/ScratchEditor/ScratchIntegrationHOC.test.jsx index 4f1889f33..d49bae8a4 100644 --- a/src/components/ScratchEditor/ScratchIntegrationHOC.test.jsx +++ b/src/components/ScratchEditor/ScratchIntegrationHOC.test.jsx @@ -2,18 +2,27 @@ const React = require("react"); const { render, waitFor } = require("@testing-library/react"); const { Provider } = require("react-redux"); const configureStore = require("redux-mock-store").default; -const ScratchIntegrationHOC = require("./ScratchIntegrationHOC").default; jest.mock("file-saver", () => ({ saveAs: jest.fn() })); +jest.mock("./events.js", () => ({ postScratchGuiEvent: jest.fn() })); jest.mock("@scratch/scratch-gui", () => ({ remixProject: () => ({ type: "remix" }), manualUpdateProject: () => ({ type: "manualUpdate" }), setStageSize: () => ({ type: "setStageSize" }), })); +const ScratchIntegrationHOC = require("./ScratchIntegrationHOC").default; +const { postScratchGuiEvent } = require("./events.js"); + describe("ScratchIntegrationHOC", () => { const mockSaveProjectSb3 = jest.fn(); const mockLoadProject = jest.fn(); + const mockVm = { + saveProjectSb3: mockSaveProjectSb3, + loadProject: mockLoadProject, + on: jest.fn(), + removeListener: jest.fn(), + }; const allowedOrigin = "https://editor.example.com"; let store; let Wrapped; @@ -27,14 +36,17 @@ describe("ScratchIntegrationHOC", () => { } mockSaveProjectSb3.mockClear(); mockLoadProject.mockClear(); + mockVm.on.mockClear(); + mockVm.removeListener.mockClear(); + mockVm.runtime = { + targets: [{ id: "stage" }, { id: "sprite-1" }], + }; + postScratchGuiEvent.mockClear(); process.env.REACT_APP_ALLOWED_IFRAME_ORIGINS = allowedOrigin; const mockStore = configureStore([]); store = mockStore({ scratchGui: { - vm: { - saveProjectSb3: mockSaveProjectSb3, - loadProject: mockLoadProject, - }, + vm: mockVm, }, }); const Dummy = () => @@ -46,6 +58,11 @@ describe("ScratchIntegrationHOC", () => { delete process.env.REACT_APP_ALLOWED_IFRAME_ORIGINS; }); + const getVmHandler = (eventName) => + mockVm.on.mock.calls.find( + ([registeredEventName]) => registeredEventName === eventName, + )?.[1]; + describe("scratch-gui-download message", () => { it("calls saveProjectSb3 and saveAs with blob and filename", async () => { const mockBlob = new Blob(["x"], { @@ -80,6 +97,7 @@ describe("ScratchIntegrationHOC", () => { const file = { arrayBuffer: jest.fn().mockResolvedValue(arrayBuffer), }; + mockLoadProject.mockResolvedValue(); render( React.createElement(Provider, { store }, React.createElement(Wrapped)), @@ -98,6 +116,53 @@ describe("ScratchIntegrationHOC", () => { await waitFor(() => { expect(file.arrayBuffer).toHaveBeenCalledTimes(1); expect(mockLoadProject).toHaveBeenCalledWith(arrayBuffer); + expect(postScratchGuiEvent).toHaveBeenCalledWith( + "scratch-gui-project-changed", + ); + }); + }); + + describe("Scratch VM project changes", () => { + it("posts a project-changed event to the parent window", () => { + render( + React.createElement( + Provider, + { store }, + React.createElement(Wrapped), + ), + ); + + getVmHandler("PROJECT_CHANGED")(); + + expect(postScratchGuiEvent).toHaveBeenCalledWith( + "scratch-gui-project-changed", + ); + }); + + it("posts a project-changed event when the target list changes", () => { + render( + React.createElement( + Provider, + { store }, + React.createElement(Wrapped), + ), + ); + + const targetsUpdateHandler = getVmHandler("targetsUpdate"); + const targetList = [ + { id: "stage" }, + { id: "sprite-1" }, + { id: "sprite-2" }, + ]; + + targetsUpdateHandler({ targetList }); + expect(postScratchGuiEvent).toHaveBeenCalledWith( + "scratch-gui-project-changed", + ); + + postScratchGuiEvent.mockClear(); + targetsUpdateHandler({ targetList }); + expect(postScratchGuiEvent).not.toHaveBeenCalled(); }); }); }); diff --git a/src/hooks/useScratchSave.js b/src/hooks/useScratchSave.js index f72687773..4561e82a3 100644 --- a/src/hooks/useScratchSave.js +++ b/src/hooks/useScratchSave.js @@ -22,18 +22,25 @@ export const useScratchSave = ({ enabled = true } = {}) => { projectOwner, scratchIframeProjectIdentifier, }); - const { isScratchSaving, saveScratchProject, scratchSaveLabelKey } = - useScratchSaveState({ - enabled: enableScratchSaveState, - }); + const autoSaveEnabled = Boolean(project?.identifier && !shouldRemixOnSave); + const { + isScratchSaveFailed, + isScratchSaving, + saveScratchProject, + scratchLastSavedTime, + } = useScratchSaveState({ + enabled: enableScratchSaveState, + autoSaveEnabled, + }); return { enableScratchSaveState, + isScratchSaveFailed, isScratchSaving, loading, projectOwner, saveScratchProject, - scratchSaveLabelKey, + scratchLastSavedTime, shouldRemixOnSave, user, }; diff --git a/src/hooks/useScratchSaveState.js b/src/hooks/useScratchSaveState.js index 91d7d81ff..01c0e521c 100644 --- a/src/hooks/useScratchSaveState.js +++ b/src/hooks/useScratchSaveState.js @@ -1,52 +1,126 @@ -import { useEffect, useRef, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import { getScratchAllowedOrigin, postMessageToScratchIframe, } from "../utils/scratchIframe"; -const SCRATCH_SAVE_LABEL_KEYS = { - idle: "header.save", - saving: "saveStatus.saving", - saved: "saveStatus.saved", -}; +const SCRATCH_AUTOSAVE_DELAY_MS = 2000; const SCRATCH_SAVE_MINIMUM_SAVING_DURATION_MS = 1000; const SCRATCH_SAVE_RESET_DELAY_MS = 5000; -export const useScratchSaveState = ({ enabled = false } = {}) => { - const scratchSaveTimeoutRef = useRef(null); - const scratchSavingStartedAtRef = useRef(null); - const [scratchSaveState, setScratchSaveState] = useState("idle"); +export const useScratchSaveState = ({ + enabled = false, + autoSaveEnabled = false, +} = {}) => { + const saveStateTimeoutRef = useRef(null); + const autoSaveTimeoutRef = useRef(null); + const savingStartedAtRef = useRef(null); + const saveInFlightRef = useRef(false); + const autoSaveEnabledRef = useRef(false); + const autoSaveQueuedAfterSaveRef = useRef(false); + const projectChangedAtRef = useRef(null); + const [saveState, setSaveState] = useState("idle"); + const [lastSavedTime, setLastSavedTime] = useState(null); + + const clearSaveStateTimeout = useCallback(() => { + if (saveStateTimeoutRef.current) { + clearTimeout(saveStateTimeoutRef.current); + saveStateTimeoutRef.current = null; + } + }, []); + + const clearAutoSaveTimeout = useCallback(() => { + if (autoSaveTimeoutRef.current) { + clearTimeout(autoSaveTimeoutRef.current); + autoSaveTimeoutRef.current = null; + } + }, []); + + const postSaveRequest = useCallback(() => { + postMessageToScratchIframe({ + type: "scratch-gui-save", + }); + }, []); + + const scheduleAutoSave = useCallback( + (delay = SCRATCH_AUTOSAVE_DELAY_MS) => { + clearAutoSaveTimeout(); + + autoSaveTimeoutRef.current = setTimeout(() => { + autoSaveTimeoutRef.current = null; + + if (!autoSaveEnabledRef.current) { + return; + } + + if (saveInFlightRef.current) { + autoSaveQueuedAfterSaveRef.current = true; + return; + } + + autoSaveQueuedAfterSaveRef.current = false; + postSaveRequest(); + }, delay); + }, + [clearAutoSaveTimeout, postSaveRequest], + ); + + const scheduleQueuedAutoSave = useCallback(() => { + if (!autoSaveQueuedAfterSaveRef.current) { + return; + } + + autoSaveQueuedAfterSaveRef.current = false; + + if (!autoSaveEnabledRef.current) { + return; + } + + const lastChangedAt = projectChangedAtRef.current; + const remainingDebounceTime = + lastChangedAt == null + ? 0 + : Math.max(0, lastChangedAt + SCRATCH_AUTOSAVE_DELAY_MS - Date.now()); + + scheduleAutoSave(remainingDebounceTime); + }, [scheduleAutoSave]); useEffect(() => { - const clearScratchSaveTimeout = () => { - if (scratchSaveTimeoutRef.current) { - clearTimeout(scratchSaveTimeoutRef.current); - scratchSaveTimeoutRef.current = null; - } - }; + autoSaveEnabledRef.current = Boolean(enabled && autoSaveEnabled); - const resetScratchSaveState = () => { - clearScratchSaveTimeout(); - scratchSavingStartedAtRef.current = null; - setScratchSaveState("idle"); + if (!autoSaveEnabledRef.current) { + clearAutoSaveTimeout(); + autoSaveQueuedAfterSaveRef.current = false; + } + }, [autoSaveEnabled, clearAutoSaveTimeout, enabled]); + + useEffect(() => { + const resetSaveState = ({ clearLastSavedTime = false } = {}) => { + clearSaveStateTimeout(); + savingStartedAtRef.current = null; + saveInFlightRef.current = false; + setSaveState("idle"); + if (clearLastSavedTime) { + setLastSavedTime(null); + } }; - const transitionScratchSaveStateToSaved = () => { - scratchSaveTimeoutRef.current = null; - scratchSavingStartedAtRef.current = null; - setScratchSaveState("saved"); - scratchSaveTimeoutRef.current = setTimeout(() => { - scratchSaveTimeoutRef.current = null; - setScratchSaveState("idle"); + const showSavedState = () => { + saveStateTimeoutRef.current = null; + savingStartedAtRef.current = null; + setSaveState("saved"); + saveStateTimeoutRef.current = setTimeout(() => { + saveStateTimeoutRef.current = null; + setSaveState("idle"); }, SCRATCH_SAVE_RESET_DELAY_MS); }; - const scheduleScratchSaveStateToSaved = () => { - clearScratchSaveTimeout(); + const scheduleSavedState = () => { + clearSaveStateTimeout(); - const savingStartedAt = scratchSavingStartedAtRef.current; + const savingStartedAt = savingStartedAtRef.current; if (savingStartedAt == null) { - transitionScratchSaveStateToSaved(); + showSavedState(); return; } @@ -55,18 +129,21 @@ export const useScratchSaveState = ({ enabled = false } = {}) => { const remainingSavingTime = savingCanFinishAt - Date.now(); if (remainingSavingTime <= 0) { - transitionScratchSaveStateToSaved(); + showSavedState(); return; } - scratchSaveTimeoutRef.current = setTimeout( - transitionScratchSaveStateToSaved, + saveStateTimeoutRef.current = setTimeout( + showSavedState, remainingSavingTime, ); }; if (!enabled) { - resetScratchSaveState(); + resetSaveState({ + clearLastSavedTime: true, + }); + clearAutoSaveTimeout(); return undefined; } @@ -77,19 +154,39 @@ export const useScratchSaveState = ({ enabled = false } = {}) => { } switch (event.data?.type) { + case "scratch-gui-project-changed": + if (!autoSaveEnabledRef.current) { + break; + } + projectChangedAtRef.current = Date.now(); + if (saveInFlightRef.current) { + autoSaveQueuedAfterSaveRef.current = true; + break; + } + scheduleAutoSave(); + break; case "scratch-gui-saving-started": case "scratch-gui-remixing-started": - clearScratchSaveTimeout(); - scratchSavingStartedAtRef.current = Date.now(); - setScratchSaveState("saving"); + clearSaveStateTimeout(); + saveInFlightRef.current = true; + savingStartedAtRef.current = Date.now(); + setSaveState("saving"); break; case "scratch-gui-saving-succeeded": case "scratch-gui-remixing-succeeded": - scheduleScratchSaveStateToSaved(); + saveInFlightRef.current = false; + setLastSavedTime(Date.now()); + scheduleSavedState(); + scheduleQueuedAutoSave(); break; case "scratch-gui-saving-failed": case "scratch-gui-remixing-failed": - resetScratchSaveState(); + saveInFlightRef.current = false; + autoSaveQueuedAfterSaveRef.current = false; + clearAutoSaveTimeout(); + clearSaveStateTimeout(); + savingStartedAtRef.current = null; + setSaveState("failed"); break; default: break; @@ -100,21 +197,33 @@ export const useScratchSaveState = ({ enabled = false } = {}) => { return () => { window.removeEventListener("message", handleScratchMessage); - clearScratchSaveTimeout(); + clearSaveStateTimeout(); + clearAutoSaveTimeout(); }; - }, [enabled]); - - const saveScratchProject = ({ shouldRemixOnSave = false } = {}) => { - postMessageToScratchIframe({ - type: shouldRemixOnSave ? "scratch-gui-remix" : "scratch-gui-save", - }); - }; + }, [ + clearAutoSaveTimeout, + clearSaveStateTimeout, + enabled, + scheduleAutoSave, + scheduleQueuedAutoSave, + ]); + + const saveScratchProject = useCallback( + ({ shouldRemixOnSave = false } = {}) => { + clearAutoSaveTimeout(); + autoSaveQueuedAfterSaveRef.current = false; + postMessageToScratchIframe({ + type: shouldRemixOnSave ? "scratch-gui-remix" : "scratch-gui-save", + }); + }, + [clearAutoSaveTimeout], + ); return { - scratchSaveState, - scratchSaveLabelKey: - SCRATCH_SAVE_LABEL_KEYS[scratchSaveState] || SCRATCH_SAVE_LABEL_KEYS.idle, - isScratchSaving: scratchSaveState === "saving", + scratchSaveState: saveState, + isScratchSaving: saveState === "saving", + isScratchSaveFailed: saveState === "failed", + scratchLastSavedTime: lastSavedTime, saveScratchProject, }; }; diff --git a/src/hooks/useScratchSaveState.test.js b/src/hooks/useScratchSaveState.test.js index 630f3c876..7115f225f 100644 --- a/src/hooks/useScratchSaveState.test.js +++ b/src/hooks/useScratchSaveState.test.js @@ -15,9 +15,8 @@ jest.useFakeTimers(); const scratchOrigin = "https://assets.example.com"; -const assertScratchSaveState = (result, state, labelKey, isSaving) => { +const assertScratchSaveState = (result, state, isSaving) => { expect(result.current.scratchSaveState).toBe(state); - expect(result.current.scratchSaveLabelKey).toBe(labelKey); expect(result.current.isScratchSaving).toBe(isSaving); }; @@ -70,11 +69,105 @@ describe("useScratchSaveState", () => { }); }); + test("does not auto-save project changes until auto-save is enabled", () => { + renderHook(() => + useScratchSaveState({ enabled: true, autoSaveEnabled: false }), + ); + + dispatchScratchMessage("scratch-gui-project-changed"); + + act(() => { + jest.advanceTimersByTime(2000); + }); + + expect(postMessageToScratchIframe).not.toHaveBeenCalled(); + }); + + test("auto-saves 2 seconds after a Scratch project change", () => { + renderHook(() => + useScratchSaveState({ enabled: true, autoSaveEnabled: true }), + ); + + dispatchScratchMessage("scratch-gui-project-changed"); + + act(() => { + jest.advanceTimersByTime(1999); + }); + + expect(postMessageToScratchIframe).not.toHaveBeenCalled(); + + act(() => { + jest.advanceTimersByTime(1); + }); + + expect(postMessageToScratchIframe).toHaveBeenCalledWith({ + type: "scratch-gui-save", + }); + }); + + test("debounces repeated Scratch project changes", () => { + renderHook(() => + useScratchSaveState({ enabled: true, autoSaveEnabled: true }), + ); + + dispatchScratchMessage("scratch-gui-project-changed"); + + act(() => { + jest.advanceTimersByTime(1000); + }); + + dispatchScratchMessage("scratch-gui-project-changed"); + + act(() => { + jest.advanceTimersByTime(1999); + }); + + expect(postMessageToScratchIframe).not.toHaveBeenCalled(); + + act(() => { + jest.advanceTimersByTime(1); + }); + + expect(postMessageToScratchIframe).toHaveBeenCalledTimes(1); + expect(postMessageToScratchIframe).toHaveBeenCalledWith({ + type: "scratch-gui-save", + }); + }); + + test("queues auto-save when a Scratch project change happens during an in-flight save", () => { + renderHook(() => + useScratchSaveState({ enabled: true, autoSaveEnabled: true }), + ); + + dispatchScratchMessage("scratch-gui-saving-started"); + dispatchScratchMessage("scratch-gui-project-changed"); + + act(() => { + jest.advanceTimersByTime(1000); + }); + + dispatchScratchMessage("scratch-gui-saving-succeeded"); + + act(() => { + jest.advanceTimersByTime(999); + }); + + expect(postMessageToScratchIframe).not.toHaveBeenCalled(); + + act(() => { + jest.advanceTimersByTime(1); + }); + + expect(postMessageToScratchIframe).toHaveBeenCalledWith({ + type: "scratch-gui-save", + }); + }); + test("keeps saving visible for at least 1 second before resetting 5 seconds after saved", () => { const { result } = renderHook(() => useScratchSaveState({ enabled: true })); dispatchScratchMessage("scratch-gui-saving-started"); - assertScratchSaveState(result, "saving", "saveStatus.saving", true); + assertScratchSaveState(result, "saving", true); dispatchScratchMessage("scratch-gui-saving-succeeded"); @@ -82,32 +175,32 @@ describe("useScratchSaveState", () => { jest.advanceTimersByTime(999); }); - assertScratchSaveState(result, "saving", "saveStatus.saving", true); + assertScratchSaveState(result, "saving", true); act(() => { jest.advanceTimersByTime(1); }); - assertScratchSaveState(result, "saved", "saveStatus.saved", false); + assertScratchSaveState(result, "saved", false); act(() => { jest.advanceTimersByTime(4999); }); - assertScratchSaveState(result, "saved", "saveStatus.saved", false); + assertScratchSaveState(result, "saved", false); act(() => { jest.advanceTimersByTime(1); }); - assertScratchSaveState(result, "idle", "header.save", false); + assertScratchSaveState(result, "idle", false); }); test("tracks remixing messages with the same save state lifecycle", () => { const { result } = renderHook(() => useScratchSaveState({ enabled: true })); dispatchScratchMessage("scratch-gui-remixing-started"); - assertScratchSaveState(result, "saving", "saveStatus.saving", true); + assertScratchSaveState(result, "saving", true); dispatchScratchMessage("scratch-gui-remixing-succeeded"); @@ -115,16 +208,17 @@ describe("useScratchSaveState", () => { jest.advanceTimersByTime(1000); }); - assertScratchSaveState(result, "saved", "saveStatus.saved", false); + assertScratchSaveState(result, "saved", false); }); - test("resets to idle after a remix failure", () => { + test("shows a failed state after a remix failure", () => { const { result } = renderHook(() => useScratchSaveState({ enabled: true })); dispatchScratchMessage("scratch-gui-remixing-started"); dispatchScratchMessage("scratch-gui-remixing-failed"); - assertScratchSaveState(result, "idle", "header.save", false); + assertScratchSaveState(result, "failed", false); + expect(result.current.isScratchSaveFailed).toBe(true); }); test("ignores messages from the wrong origin", () => { @@ -135,7 +229,7 @@ describe("useScratchSaveState", () => { "https://other.example.com", ); - assertScratchSaveState(result, "idle", "header.save", false); + assertScratchSaveState(result, "idle", false); }); test("accepts messages when ASSETS_URL contains a path", () => { @@ -146,6 +240,6 @@ describe("useScratchSaveState", () => { dispatchScratchMessage("scratch-gui-saving-started"); - assertScratchSaveState(result, "saving", "saveStatus.saving", true); + assertScratchSaveState(result, "saving", true); }); });