fix(submission): scribing canvas render order and unrecorded Voice teardown

client/app/bundles/course/assessment/submission/components/ScribingView/ScribingCanvas.tsx

@@ -1331,13 +1331,23 @@ const ScribingCanvas = forwardRef<ScribingCanvasRef, ScribingCanvasProps>(
         className="flex justify-center-safe bg-neutral-300 m-0 w-full outline-none items-center"
         style={{ minWidth: 800 }}
       >
-        {!scribingState[answerId]?.isCanvasLoaded ? <LoadingIndicator /> : null}
         <canvas
           ref={htmlCanvasRef}
           data-testid={`canvas-${answerId}`}
           id={`canvas-${answerId}`}
           style={styles.canvas}
         />
+        {/*
+         * Rendered AFTER the canvas, never before: once Fabric.js constructs the
+         * canvas, it wraps the raw <canvas> node in its own container div,
+         * re-parenting it out from under this div's direct children. A sibling
+         * rendered BEFORE the canvas would need React to `insertBefore` relative
+         * to that no-longer-direct-child node on every toggle (e.g. a redundant
+         * re-initialize resetting isCanvasLoaded) — which throws. Appending a
+         * trailing sibling never needs a reference node, so it's safe regardless
+         * of what Fabric has done to the canvas by then.
+         */}
+        {!scribingState[answerId]?.isCanvasLoaded ? <LoadingIndicator /> : null}
       </div>
     );
   },

client/app/bundles/course/assessment/submission/components/ScribingView/__test__/index.test.tsx

@@ -1,12 +1,37 @@
 import { dispatch } from 'store';
-import { act, render } from 'test-utils';
+import { act, render, waitFor } from 'test-utils';
 import { QuestionType } from 'types/course/assessment/question';
 import { ScribingAnswerData } from 'types/course/assessment/submission/answer/scribing';
 
 import ScribingView from 'course/assessment/submission/containers/ScribingView';
+import { LOADING_INDICATOR_TEST_ID } from 'lib/components/core/LoadingIndicator';
 
 import { scribingActions } from '../../../reducers/scribing';
 
+/**
+ * jsdom never actually fetches/decodes images, so a real <img>'s `load` event
+ * never fires for a fake asset URL — ScribingCanvas's own image-load effect
+ * (where it actually constructs the Fabric.js canvas) never runs under the
+ * default Image. This stub returns a REAL HTMLImageElement (so Fabric/canvas's
+ * `instanceof` checks on drawImage's source still pass), with `src` patched to
+ * fire `load` asynchronously, like a real browser would.
+ */
+function StubImage(): HTMLImageElement {
+  const img = document.createElement('img');
+  Object.defineProperty(img, 'width', { value: 100, configurable: true });
+  Object.defineProperty(img, 'height', { value: 100, configurable: true });
+  let currentSrc = '';
+  Object.defineProperty(img, 'src', {
+    configurable: true,
+    get: () => currentSrc,
+    set: (value: string) => {
+      currentSrc = value;
+      setTimeout(() => img.dispatchEvent(new Event('load')), 0);
+    },
+  });
+  return img;
+}
+
 const assessmentId = 1;
 const submissionId = 2;
 const answerId = 3;
@@ -74,4 +99,76 @@ describe('ScribingView', () => {
       await page.findByTestId(`canvas-${answerId}`, {}, { timeout: 5000 }),
     ).toBeVisible();
   });
+
+  describe('with a real Fabric.js canvas', () => {
+    const OriginalImage = global.Image;
+
+    beforeAll(() => {
+      // @ts-expect-error — minimal stub, see StubImage above
+      global.Image = StubImage;
+    });
+
+    afterAll(() => {
+      global.Image = OriginalImage;
+    });
+
+    it('survives a redundant re-initialize after the canvas has already loaded', async () => {
+      // React reports this crash as an uncaught commit-phase error (via a
+      // dispatched DOM event), not as a rejected promise — `dispatch`/`act`
+      // above it don't throw. Catch it directly so the test fails clearly
+      // instead of just timing out waiting for a DOM update that never comes.
+      const uncaughtErrors: string[] = [];
+      const onWindowError = (event: ErrorEvent): void => {
+        uncaughtErrors.push(event.error?.message ?? event.message);
+      };
+      window.addEventListener('error', onWindowError);
+
+      try {
+        await act(() =>
+          dispatch(
+            scribingActions.initialize({ answers: mockSubmission.answers }),
+          ),
+        );
+
+        const url = `/courses/${global.courseId}/assessments/${assessmentId}/submissions/${submissionId}/edit`;
+        const page = render(<ScribingView answerId={answerId} />, {
+          at: [url],
+        });
+
+        // Wait for the component's own image-load effect to construct the real
+        // Fabric.js canvas — this is what wraps the <canvas> in Fabric's own
+        // container div, re-parenting it out from under React's tree.
+        await waitFor(
+          () =>
+            expect(
+              page.queryByTestId(LOADING_INDICATOR_TEST_ID),
+            ).not.toBeInTheDocument(),
+          { timeout: 5000 },
+        );
+
+        // A duplicate FETCH_SUBMISSION_SUCCESS re-runs scribing/initialize,
+        // resetting isCanvasLoaded to false for an answer whose canvas Fabric.js
+        // has already taken over. Toggling the loading indicator back on then
+        // requires React to insertBefore a sibling relative to that
+        // no-longer-direct-child <canvas> node — must not throw.
+        await act(() =>
+          dispatch(
+            scribingActions.initialize({ answers: mockSubmission.answers }),
+          ),
+        );
+
+        expect(uncaughtErrors).toEqual([]);
+        expect(
+          await page.findByTestId(
+            LOADING_INDICATOR_TEST_ID,
+            {},
+            { timeout: 5000 },
+          ),
+        ).toBeVisible();
+        expect(page.getByTestId(`canvas-${answerId}`)).toBeInTheDocument();
+      } finally {
+        window.removeEventListener('error', onWindowError);
+      }
+    });
+  });
 });

client/app/bundles/course/assessment/submission/reducers/__test__/recorder.test.js

@@ -0,0 +1,59 @@
+import recorderHelper from '../../../utils/recorderHelper';
+import actionTypes from '../../constants';
+import reducer from '../recorder';
+
+jest.mock('../../../utils/recorderHelper');
+
+describe('recorder reducer', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  describe('RECORDER_COMPONENT_UNMOUNT', () => {
+    it('does not try to stop the recorder when nothing was recording', () => {
+      recorderHelper.isRecording.mockReturnValue(false);
+      const state = {
+        recording: false,
+        recorderComponentsCount: 1,
+        recordingComponentId: '',
+      };
+
+      reducer(state, { type: actionTypes.RECORDER_COMPONENT_UNMOUNT });
+
+      expect(recorderHelper.stopRecord).not.toHaveBeenCalled();
+    });
+
+    it('stops the recorder when the user navigates away mid-recording', () => {
+      recorderHelper.isRecording.mockReturnValue(true);
+      recorderHelper.stopRecord.mockResolvedValue(new File([], 'test.wav'));
+      const state = {
+        recording: true,
+        recorderComponentsCount: 1,
+        recordingComponentId: 'voice_response_1',
+      };
+
+      reducer(state, { type: actionTypes.RECORDER_COMPONENT_UNMOUNT });
+
+      expect(recorderHelper.stopRecord).toHaveBeenCalled();
+    });
+
+    it('does not decrement below the last unmount and resets recording state', () => {
+      recorderHelper.isRecording.mockReturnValue(false);
+      const state = {
+        recording: true,
+        recorderComponentsCount: 1,
+        recordingComponentId: 'voice_response_1',
+      };
+
+      const nextState = reducer(state, {
+        type: actionTypes.RECORDER_COMPONENT_UNMOUNT,
+      });
+
+      expect(nextState).toEqual({
+        recording: false,
+        recorderComponentsCount: 0,
+        recordingComponentId: 'voice_response_1',
+      });
+    });
+  });
+});

client/app/bundles/course/assessment/submission/reducers/recorder.js

@@ -38,10 +38,10 @@ export default function (state = initialState, action) {
       recording = false;
 
       /**
-       * When the user navigate to other path without stopping the recorder
-       * We need to help the user to stop
+       * When the user navigates to another path without stopping the recorder,
+       * help them stop it, but only if it was actually recording.
        */
-      if (recorderComponentsCount === 0) {
+      if (recorderComponentsCount === 0 && recorderHelper.isRecording()) {
         recorderHelper.stopRecord();
       }
       return {