fix(client): retain editableContents through reload (#59573)

client/src/templates/Challenges/classic/editor.tsx

@@ -108,7 +108,7 @@ export interface EditorProps {
   previewOpen: boolean;
   updateFile: (object: {
     fileKey: string;
-    editorValue: string;
+    contents: string;
     editableRegionBoundaries?: number[];
   }) => void;
   usesMultifileEditor: boolean;
@@ -846,7 +846,7 @@ const Editor = (props: EditorProps): JSX.Element => {
     }
   }
 
-  const onChange = (editorValue: string) => {
+  const onChange = (contents: string) => {
     const { updateFile, fileKey, isResetting } = props;
     if (isResetting) return;
     // TODO: now that we have getCurrentEditableRegion, should the overlays
@@ -873,7 +873,7 @@ const Editor = (props: EditorProps): JSX.Element => {
         }
       });
     }
-    updateFile({ fileKey, editorValue, editableRegionBoundaries });
+    updateFile({ fileKey, contents, editableRegionBoundaries });
   };
 
   function createBreadcrumb(): HTMLElement {

client/src/templates/Challenges/redux/action-types.js

@@ -23,7 +23,6 @@ export const actionTypes = createTypes(
     'cancelTests',
     'logsToConsole',
     'disableBuildOnError',
-    'storedCodeFound',
     'noStoredCodeFound',
     'saveEditorContent',
     'setShowPreviewPane',

client/src/templates/Challenges/redux/actions.js

@@ -1,24 +1,8 @@
 import { createAction } from 'redux-actions';
 
-import { getLines } from '../../../../../shared/utils/get-lines';
 import { actionTypes } from './action-types';
 
-export const createFiles = createAction(
-  actionTypes.createFiles,
-  challengeFiles =>
-    challengeFiles.map(challengeFile => ({
-      ...challengeFile,
-      seed: challengeFile.contents.slice(),
-      editableContents: getLines(
-        challengeFile.contents,
-        challengeFile.editableRegionBoundaries
-      ),
-      editableRegionBoundaries:
-        challengeFile.editableRegionBoundaries?.slice() ?? [],
-      seedEditableRegionBoundaries:
-        challengeFile.editableRegionBoundaries?.slice() ?? []
-    }))
-);
+export const createFiles = createAction(actionTypes.createFiles);
 
 export const createQuestion = createAction(actionTypes.createQuestion);
 export const initTests = createAction(actionTypes.initTests);
@@ -50,7 +34,6 @@ export const logsToConsole = createAction(actionTypes.logsToConsole);
 export const disableBuildOnError = createAction(
   actionTypes.disableBuildOnError
 );
-export const storedCodeFound = createAction(actionTypes.storedCodeFound);
 export const noStoredCodeFound = createAction(actionTypes.noStoredCodeFound);
 export const saveEditorContent = createAction(actionTypes.saveEditorContent);
 export const setIsAdvancing = createAction(actionTypes.setIsAdvancing);

client/src/templates/Challenges/redux/code-storage-epic.js

@@ -9,7 +9,7 @@ import { FlashMessages } from '../../../components/Flash/redux/flash-messages';
 import { savedChallengesSelector } from '../../../redux/selectors';
 import { actionTypes as appTypes } from '../../../redux/action-types';
 import { actionTypes } from './action-types';
-import { noStoredCodeFound, storedCodeFound } from './actions';
+import { noStoredCodeFound, updateFile } from './actions';
 import { challengeFilesSelector, challengeMetaSelector } from './selectors';
 
 const legacyPrefixes = [
@@ -200,7 +200,9 @@ function loadCodeEpic(action$, state$) {
         }
       }
       if (finalFiles) {
-        return of(storedCodeFound(finalFiles));
+        // update the contents, rather than replacing the entire file, so that
+        // we do not lose the seed values.
+        return of(...finalFiles.map(file => updateFile(file)));
       }
       return of(noStoredCodeFound());
     })

client/src/templates/Challenges/redux/index.js

@@ -2,7 +2,6 @@ import { isEmpty } from 'lodash-es';
 import { handleActions } from 'redux-actions';
 
 import { getLines } from '../../../../../shared/utils/get-lines';
-import { mergeChallengeFiles } from '../classic/saved-challenges';
 import { getTargetEditor } from '../utils/get-target-editor';
 import { actionTypes, ns } from './action-types';
 import codeStorageEpic from './code-storage-epic';
@@ -84,23 +83,31 @@ export const reducer = handleActions(
     }),
     [actionTypes.createFiles]: (state, { payload }) => ({
       ...state,
-      challengeFiles: payload
+      challengeFiles: payload.map(challengeFile => ({
+        ...challengeFile,
+        seed: challengeFile.contents.slice(),
+        editableContents: getLines(
+          challengeFile.contents,
+          challengeFile.editableRegionBoundaries
+        ),
+        editableRegionBoundaries:
+          challengeFile.editableRegionBoundaries?.slice() ?? [],
+        seedEditableRegionBoundaries:
+          challengeFile.editableRegionBoundaries?.slice() ?? []
+      }))
     }),
     [actionTypes.updateFile]: (
       state,
-      { payload: { fileKey, editorValue, editableRegionBoundaries } }
+      { payload: { fileKey, contents, editableRegionBoundaries } }
     ) => {
       const updates = {};
       // if a given part of the payload is null, we leave that part of the state
       // unchanged
       if (editableRegionBoundaries !== null)
         updates.editableRegionBoundaries = editableRegionBoundaries;
-      if (editorValue !== null) updates.contents = editorValue;
-      if (editableRegionBoundaries !== null && editorValue !== null)
-        updates.editableContents = getLines(
-          editorValue,
-          editableRegionBoundaries
-        );
+      if (contents !== null) updates.contents = contents;
+      if (editableRegionBoundaries !== null && contents !== null)
+        updates.editableContents = getLines(contents, editableRegionBoundaries);
       return {
         ...state,
         challengeFiles: state.challengeFiles.map(challengeFile =>
@@ -111,12 +118,6 @@ export const reducer = handleActions(
         isBuildEnabled: true
       };
     },
-    [actionTypes.storedCodeFound]: (state, { payload }) => ({
-      ...state,
-      challengeFiles: state.challengeFiles.length
-        ? mergeChallengeFiles(state.challengeFiles, payload)
-        : payload
-    }),
     [actionTypes.initTests]: (state, { payload }) => ({
       ...state,
       challengeTests: payload

e2e/challenge.spec.ts

@@ -0,0 +1,47 @@
+import { test, expect } from '@playwright/test';
+
+import translations from '../client/i18n/locales/english/translations.json';
+import { clearEditor, focusEditor, getEditors } from './utils/editor';
+
+test.describe('when reloading the page', () => {
+  test.beforeEach(async ({ page }) => {
+    const pageUsingEditableRegionInTests =
+      '/learn/2022/responsive-web-design/learn-basic-css-by-building-a-cafe-menu/step-14';
+    await page.goto(pageUsingEditableRegionInTests);
+  });
+  // This is quite brittle. If it breaks, try to come up with a unit test instead.
+
+  test('should keep the editable content for testing', async ({
+    page,
+    isMobile,
+    browserName
+  }) => {
+    await focusEditor({ page, isMobile });
+    await clearEditor({ page, browserName });
+    // For some reason, fill doesn't work properly on firefox if there are new lines
+    // in the text, hence one line:
+    const solution = `h1, h2, p { text-align: center; }`;
+
+    await getEditors(page).fill(solution);
+    const editorTextLocator = page
+      .getByTestId('editor-container-stylescss')
+      .getByText(solution);
+    await expect(editorTextLocator).toBeVisible();
+
+    // save the code
+    await page.keyboard.down('Control');
+    await page.keyboard.press('S');
+
+    await page.reload();
+
+    await expect(editorTextLocator).toBeVisible();
+
+    // check the tests pass
+    await page.keyboard.down('Control');
+    await page.keyboard.press('Enter');
+
+    await expect(
+      page.getByText(translations.learn.congratulations)
+    ).toBeVisible();
+  });
+});