From cda00d47709682e549ecb822c006bf1a96ad6574 Mon Sep 17 00:00:00 2001
From: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
Date: Thu, 25 Jul 2024 17:45:03 +0200
Subject: [PATCH] feat(client): save challenge layout in local storage (again)
 (#55621)

---
 .../Challenges/classic/desktop-layout.tsx     | 17 +++++++-
 .../src/templates/Challenges/classic/show.tsx | 40 +++++++------------
 e2e/multifile-editor.spec.ts                  | 30 ++++++++++++++
 3 files changed, 60 insertions(+), 27 deletions(-)

diff --git a/client/src/templates/Challenges/classic/desktop-layout.tsx b/client/src/templates/Challenges/classic/desktop-layout.tsx
index 31db47a66ac..00d299d2b7a 100644
--- a/client/src/templates/Challenges/classic/desktop-layout.tsx
+++ b/client/src/templates/Challenges/classic/desktop-layout.tsx
@@ -215,6 +215,7 @@ const DesktopLayout = (props: DesktopLayoutProps): JSX.Element => {
           <ReflexElement
             flex={instructionPane.flex}
             {...resizeProps}
+            name='instructionPane'
             data-playwright-test-label='instruction-pane'
           >
             {instructions}
@@ -226,6 +227,7 @@ const DesktopLayout = (props: DesktopLayoutProps): JSX.Element => {
 
         <ReflexElement
           flex={editorPane.flex}
+          name='editorPane'
           {...resizeProps}
           data-playwright-test-label='editor-pane'
         >
@@ -236,6 +238,7 @@ const DesktopLayout = (props: DesktopLayoutProps): JSX.Element => {
             >
               <ReflexElement
                 flex={codePane.flex}
+                name='codePane'
                 {...reflexProps}
                 {...resizeProps}
               >
@@ -258,17 +261,26 @@ const DesktopLayout = (props: DesktopLayoutProps): JSX.Element => {
         </ReflexElement>
         {displayNotes && <ReflexSplitter propagate={true} {...resizeProps} />}
         {displayNotes && (
-          <ReflexElement flex={notesPane.flex} {...resizeProps}>
+          <ReflexElement
+            name='notesPane'
+            flex={notesPane.flex}
+            {...resizeProps}
+          >
             <Notes notes={notes} />
           </ReflexElement>
         )}
 
         {(displayPreviewPane || displayPreviewConsole) && (
-          <ReflexSplitter propagate={true} {...resizeProps} />
+          <ReflexSplitter
+            data-playwright-test-label='preview-left-splitter'
+            propagate={true}
+            {...resizeProps}
+          />
         )}
         {(displayPreviewPane || displayPreviewConsole) && (
           <ReflexElement
             flex={previewPane.flex}
+            name='previewPane'
             {...resizeProps}
             data-playwright-test-label='preview-pane'
           >
@@ -279,6 +291,7 @@ const DesktopLayout = (props: DesktopLayoutProps): JSX.Element => {
               )}
               {displayPreviewConsole && (
                 <ReflexElement
+                  name='testsPane'
                   {...(displayPreviewPane && { flex: testsPane.flex })}
                   {...resizeProps}
                 >
diff --git a/client/src/templates/Challenges/classic/show.tsx b/client/src/templates/Challenges/classic/show.tsx
index b77250e958e..bd3032d5414 100644
--- a/client/src/templates/Challenges/classic/show.tsx
+++ b/client/src/templates/Challenges/classic/show.tsx
@@ -244,18 +244,16 @@ function ShowClassic({
     challengeTypes.python
   ].includes(challengeType);
   const getLayoutState = () => {
-    const reflexLayout = store.get(REFLEX_LAYOUT) as ReflexLayout;
-
-    // Validate if user has not done any resize of the panes
-    if (!reflexLayout) return BASE_LAYOUT;
+    const reflexLayout = store.get(REFLEX_LAYOUT) as ReflexLayout | null;
 
     // Check that the layout values stored are valid (exist in base layout). If
     // not valid, it will fallback to the base layout values and be set on next
     // user resize.
-    const isValidLayout = isContained(
-      Object.keys(BASE_LAYOUT),
-      Object.keys(reflexLayout)
-    );
+    const isValidLayout =
+      reflexLayout &&
+      isContained(Object.keys(BASE_LAYOUT), Object.keys(reflexLayout));
+
+    if (!isValidLayout) store.remove(REFLEX_LAYOUT);
 
     return isValidLayout ? reflexLayout : BASE_LAYOUT;
   };
@@ -266,25 +264,17 @@ function ShowClassic({
   const [layout, setLayout] = useState(getLayoutState());
 
   const onStopResize = (event: HandlerProps) => {
+    setResizing(false);
+    // 'name' is used to identify the Elements whose layout is stored.
     const { name, flex } = event.component.props;
 
-    // Only interested in tracking layout updates for ReflexElement's
-    if (!name) {
-      setResizing(false);
-      return;
-    }
-
-    // Forcing a state update with the value of each panel since on stop resize
-    // is executed per each panel.
-    if (typeof layout === 'object') {
-      setLayout({
-        ...layout,
-        [name]: { flex }
-      });
-    }
-    setResizing(false);
-
-    store.set(REFLEX_LAYOUT, layout);
+    // onStopResize can be called multiple times before the state changes, so
+    // we need an updater function to ensure all updates are applied.
+    setLayout(l => {
+      const newLayout = name ? { ...l, [name]: { flex } } : l;
+      store.set(REFLEX_LAYOUT, newLayout);
+      return newLayout;
+    });
   };
 
   const setHtmlHeight = () => {
diff --git a/e2e/multifile-editor.spec.ts b/e2e/multifile-editor.spec.ts
index 8fbeb117baa..bb1745490cd 100644
--- a/e2e/multifile-editor.spec.ts
+++ b/e2e/multifile-editor.spec.ts
@@ -42,4 +42,34 @@ test.describe('MultifileEditor Component', () => {
       index++;
     }
   });
+
+  test('Reloading should preserve the editor layout', async ({
+    page,
+    isMobile
+  }) => {
+    test.skip(
+      isMobile,
+      'The mobile layout does not have resizable panes, so this test is not applicable.'
+    );
+
+    const desktopLayout = page.getByTestId('desktop-layout');
+    const splitter = desktopLayout.getByTestId('preview-left-splitter');
+    const editorPane = desktopLayout.getByTestId('editor-pane');
+    const initialStyle = await editorPane.getAttribute('style');
+    expect(initialStyle).toContain('flex: 1');
+
+    // Drag the splitter to resize the editor pane
+    await splitter.hover();
+    await page.mouse.down();
+    await page.mouse.move(100, 100);
+    await page.mouse.up();
+
+    const newStyle = await editorPane.getAttribute('style');
+    // It doesn't matter where it's dragged to, just that it's different:
+    expect(newStyle).not.toContain('flex: 1');
+
+    await page.reload();
+
+    expect(await editorPane.getAttribute('style')).toBe(newStyle);
+  });
 });