fix: update lower jaw on ctrl enter (#47463)

* fix: update jaw on all ctrl-enter presses

* fix: show running tests while hiding feedback

Screenreaders still announcing running tests and then the output, and
the display now stops showing a flash of the new feedback message

* refactor: camelCase

* refactor: clean up and reduce renders

previousHint isn't really state, insofar as it changing should not
trigger a new render - a ref does the trick.

* chore: remove log

* fix: use ref to track latest attempts prop

* fix: allow lower jaw to trigger viewzone updates

React.render's callback cannot be relied on. It does not necessarily
get called on every update to the rendered element.

updateContainer is called on every render, so the editor will be
updated.

* refactor: attemptsNumber -> attempts

* fix: do not render on attempt changes

* refactor: more camelCase

* fix: try to prevent previous hint rendering

Co-authored-by:
Manabu Matsumoto <mmatsumoto1026@gmail.com>

* test: resetting of lower jaw

* fix: reset lower jaw when step is reset

* test: check congrats message appears on completion

* fix: hide feedback after reset

* fix: leave focus in the editor after passing tests

This is an attempt to work around a JAWS issue whereby the submit
shortcut (ctrl+enter) is ignored after the user passes all tests.

* test: submit button receives focus when tests pass

* fix: focus submit button when tests pass

Co-authored-by: Bruce Blaser <bbsmooth@gmail.com>

Co-authored-by: Bruce Blaser <bbsmooth@gmail.com>

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

@@ -59,7 +59,9 @@ import {
   stopResetting,
   isProjectPreviewModalOpenSelector,
   openModal,
-  isChallengeCompletedSelector
+  isChallengeCompletedSelector,
+  attemptsSelector,
+  resetAttempts
 } from '../redux';
 import GreenPass from '../../../assets/icons/green-pass';
 import LowerJaw from './lower-jaw';
@@ -70,6 +72,7 @@ import './editor.css';
 const MonacoEditor = Loadable(() => import('react-monaco-editor'));
 
 interface EditorProps {
+  attempts: number;
   canFocus: boolean;
   challengeFiles: ChallengeFiles;
   challengeType: number;
@@ -97,6 +100,7 @@ interface EditorProps {
   setEditorFocusability: (isFocusable: boolean) => void;
   submitChallenge: () => void;
   stopResetting: () => void;
+  resetAttempts: () => void;
   tests: Test[];
   theme: Themes;
   title: string;
@@ -128,6 +132,7 @@ interface EditorProperties {
 }
 
 const mapStateToProps = createSelector(
+  attemptsSelector,
   canFocusEditorSelector,
   challengeMetaSelector,
   consoleOutputSelector,
@@ -139,6 +144,7 @@ const mapStateToProps = createSelector(
   challengeTestsSelector,
   isChallengeCompletedSelector,
   (
+    attempts: number,
     canFocus: boolean,
     { challengeType }: { challengeType: number },
     output: string[],
@@ -150,6 +156,7 @@ const mapStateToProps = createSelector(
     tests: [{ text: string; testString: string }],
     isChallengeCompleted: boolean
   ) => ({
+    attempts,
     canFocus: open ? false : canFocus,
     challengeType,
     previewOpen,
@@ -173,6 +180,7 @@ const mapDispatchToProps = {
   submitChallenge,
   initTests,
   stopResetting,
+  resetAttempts,
   openHelpModal: () => openModal('help'),
   openResetModal: () => openModal('reset')
 };
@@ -232,7 +240,7 @@ const initialData: EditorProperties = {
 
 const Editor = (props: EditorProps): JSX.Element => {
   const { t } = useTranslation();
-  const { editorRef, initTests } = props;
+  const { editorRef, initTests, resetAttempts } = props;
   // These refs are used during initialisation of the editor as well as by
   // callbacks.  Since they have to be initialised before editorWillMount and
   // editorDidMount are called, we cannot use useState.  Reason being that will
@@ -258,18 +266,14 @@ const Editor = (props: EditorProps): JSX.Element => {
     noteIndex: 0,
     shouldPlay: store.get('fcc-sound') as boolean | undefined
   });
-  const attemptRef = useRef<{ attempts: number }>({ attempts: 0 });
 
   // since editorDidMount runs once with the initial props object, it keeps a
   // reference to *those* props. If we want it to use the latest props, we can
   // use a ref, since it will be updated on every render.
   const testRef = useRef<Test[]>([]);
   testRef.current = props.tests;
-
-  // TENATIVE PLAN: create a typical order [html/jsx, css, js], put the
-  // available files into that order.  i.e. if it's just one file it will
-  // automatically be first, but  if there's jsx and js (for some reason) it
-  //  will be [jsx, js].
+  const attemptsRef = useRef<number>(0);
+  attemptsRef.current = props.attempts;
 
   const options: editor.IStandaloneEditorConstructionOptions = {
     fontSize: 18,
@@ -376,6 +380,7 @@ const Editor = (props: EditorProps): JSX.Element => {
     if (hasEditableRegion()) {
       initializeDescriptionAndOutputWidgets();
       addContentChangeListener();
+      resetAttempts();
       showEditableRegion(editor);
     }
 
@@ -597,15 +602,18 @@ const Editor = (props: EditorProps): JSX.Element => {
 
   function tryToExecuteChallenge() {
     props.executeChallenge();
-    attemptRef.current.attempts++;
   }
 
   const tryToSubmitChallenge = submitChallengeDebounceRef.current;
 
-  function createLowerJaw(outputNode: HTMLElement, callback?: () => void) {
+  function createLowerJaw(
+    outputNode: HTMLDivElement,
+    editor: editor.IStandaloneCodeEditor
+  ) {
     const { output } = props;
     const isChallengeComplete = challengeIsComplete();
     const isEditorInFocus = document.activeElement?.tagName === 'TEXTAREA';
+
     ReactDOM.render(
       <LowerJaw
         openHelpModal={props.openHelpModal}
@@ -613,15 +621,14 @@ const Editor = (props: EditorProps): JSX.Element => {
         tryToExecuteChallenge={tryToExecuteChallenge}
         hint={output[1]}
         testsLength={props.tests.length}
-        attemptsNumber={attemptRef.current.attempts}
+        attempts={attemptsRef.current}
         challengeIsCompleted={isChallengeComplete}
-        challengeHasErrors={challengeHasErrors()}
         tryToSubmitChallenge={tryToSubmitChallenge}
         isEditorInFocus={isEditorInFocus}
         isSignedIn={props.isSignedIn}
+        updateContainer={() => updateOutputViewZone(outputNode, editor)}
       />,
-      outputNode,
-      callback
+      outputNode
     );
   }
 
@@ -630,13 +637,11 @@ const Editor = (props: EditorProps): JSX.Element => {
     if (!editor || !dataRef.current.outputNode) return;
 
     const outputNode = dataRef.current.outputNode;
-    createLowerJaw(outputNode, () => {
-      if (dataRef.current.outputNode) {
-        updateOutputViewZone(outputNode, editor);
-      }
-    });
+    createLowerJaw(outputNode, editor);
   };
 
+  // TODO: there's a potential performance gain to be had by only updating when
+  // the outputViewZone has actually changed.
   const updateOutputViewZone = (
     outputNode: HTMLDivElement,
     editor: editor.IStandaloneCodeEditor
@@ -1071,14 +1076,11 @@ const Editor = (props: EditorProps): JSX.Element => {
     return tests.every(test => test.pass && !test.err);
   }
 
-  function challengeHasErrors() {
-    const tests = testRef.current;
-    return tests.some(test => test.err);
-  }
-
-  function resetAttampts() {
-    attemptRef.current.attempts = 0;
-  }
+  // We need to set initialize the tests, but only once
+  useEffect(() => {
+    initTests(props.initialTests);
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, []);
 
   // runs every update to the editor and when the challenge is reset
   useEffect(() => {
@@ -1097,17 +1099,11 @@ const Editor = (props: EditorProps): JSX.Element => {
       focusIfTargetEditor();
     }
 
-    // Once a challenge has been completed, we don't want changes to the content
-    // to reset the tests since the user is already done with the challenge.
-    if (props.initialTests && !challengeIsComplete())
-      initTests(props.initialTests);
-
     if (hasEditableRegion() && editor) {
       if (props.isResetting) {
         initializeDescriptionAndOutputWidgets();
         updateDescriptionZone();
         showEditableRegion(editor);
-        resetAttampts();
         resetMarginDecorations();
       }
     }

client/src/templates/Challenges/classic/lower-jaw.tsx

@@ -14,46 +14,58 @@ interface LowerJawProps {
   openHelpModal: () => void;
   tryToExecuteChallenge: () => void;
   tryToSubmitChallenge: () => void;
-  showFeedback?: boolean;
   isEditorInFocus?: boolean;
-  challengeHasErrors?: boolean;
   testsLength?: number;
-  attemptsNumber?: number;
+  attempts: number;
   openResetModal: () => void;
   isSignedIn: boolean;
+  updateContainer: () => void;
 }
 
 const LowerJaw = ({
   openHelpModal,
   challengeIsCompleted,
-  challengeHasErrors,
   hint,
   tryToExecuteChallenge,
   tryToSubmitChallenge,
-  attemptsNumber,
+  attempts,
   testsLength,
   isEditorInFocus,
   openResetModal,
-  isSignedIn
+  isSignedIn,
+  updateContainer
 }: LowerJawProps): JSX.Element => {
-  const [previousHint, setpreviousHint] = useState('');
+  const hintRef = React.useRef('');
   const [runningTests, setRunningTests] = useState(false);
-  const [testFeedbackheight, setTestFeedbackheight] = useState(0);
+  const [testFeedbackHeight, setTestFeedbackHeight] = useState(0);
+  const [currentAttempts, setCurrentAttempts] = useState(attempts);
   const [isFeedbackHidden, setIsFeedbackHidden] = useState(false);
-  const [testBtnariaHidden, setTestBtnariaHidden] = useState(false);
+  const [testBtnAriaHidden, setTestBtnAriaHidden] = useState(false);
   const { t } = useTranslation();
   const submitButtonRef = React.createRef<HTMLButtonElement>();
   const testFeedbackRef = React.createRef<HTMLDivElement>();
 
   useEffect(() => {
-    if (attemptsNumber && attemptsNumber > 0) {
+    // prevent unnecessary updates:
+    if (attempts === currentAttempts) return;
+    // Attempts should only be zero when the step is reset, so we should reset
+    // the state here.
+    if (attempts === 0) {
+      setCurrentAttempts(0);
+      setRunningTests(false);
+      setTestBtnAriaHidden(false);
+      setIsFeedbackHidden(false);
+      hintRef.current = '';
+    } else if (attempts > 0 && hint) {
       //hide the feedback from SR until the "Running tests" are displayed and removed.
       setIsFeedbackHidden(true);
-
-      //allow the lower jaw height to be picked up by the editor.
-      setTimeout(() => {
-        setRunningTests(true);
-      }, 200);
+      setRunningTests(true);
+      //to prevent the changing attempts value from immediately triggering a new
+      //render, the rendered component only depends on currentAttempts. Since
+      //currentAttempts is updated with when the feedback is hidden, the screen
+      //reader should only read out the new message.
+      setCurrentAttempts(attempts);
+      hintRef.current = hint;
 
       //display the test feedback contents.
       setTimeout(() => {
@@ -61,50 +73,35 @@ const LowerJaw = ({
         setIsFeedbackHidden(false);
       }, 300);
     }
-
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [attemptsNumber]);
+  }, [attempts, hint, currentAttempts]);
 
   useEffect(() => {
-    // only save error hints
-    if (challengeHasErrors && hint && previousHint !== hint) {
-      setpreviousHint(hint);
-    }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [challengeHasErrors, hint]);
-
-  useEffect(() => {
-    if (challengeIsCompleted && submitButtonRef?.current) {
-      submitButtonRef.current.focus();
+    if (challengeIsCompleted) {
+      if (!isEditorInFocus) submitButtonRef?.current?.focus();
       setTimeout(() => {
-        setTestBtnariaHidden(true);
+        setTestBtnAriaHidden(true);
       }, 500);
     }
 
-    setTestBtnariaHidden(challengeIsCompleted);
-
+    setTestBtnAriaHidden(challengeIsCompleted);
+    // Since submitButtonRef changes every render, we have to ignore it here or,
+    // once the challenges is completed, every render (including ones triggered
+    // by typing in the editor) will focus the button.
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [challengeIsCompleted]);
 
   // eslint-disable-next-line react-hooks/exhaustive-deps
   useEffect(() => {
     if (testFeedbackRef.current) {
-      setTestFeedbackheight(testFeedbackRef.current.clientHeight);
+      setTestFeedbackHeight(testFeedbackRef.current.clientHeight);
     }
+    // Every render could change the shape of the jaw, so this effect will let
+    // monaco know it might need to resize
+    updateContainer();
   });
 
-  /*
-    Return early in lifecycle based on the earliest available conditions to help the editor
-    calculate the correct editor gap for the lower jaw.
-
-    For consistency, use the persisted version if the conditions has been met before.
-  */
-  const earliestAvailableHint = hint || previousHint;
-
   const renderTestFeedbackContainer = () => {
-    if (attemptsNumber === 0) {
-      return '';
-    } else if (runningTests) {
+    if (runningTests) {
       return <span className='sr-only'>{t('aria.running-tests')}</span>;
     } else if (challengeIsCompleted) {
       const submitKeyboardInstructions = isEditorInFocus ? (
@@ -128,13 +125,17 @@ const LowerJaw = ({
           </div>
         </div>
       );
-    } else if (earliestAvailableHint) {
-      const hintDescription = `<h2 class="hint">${t(
-        'learn.hint'
-      )}</h2> ${earliestAvailableHint}`;
+    } else if (hintRef.current) {
+      const hintDescription = `<h2 class="hint">${t('learn.hint')}</h2> ${
+        hintRef.current
+      }`;
       return (
         <>
-          <div className='test-status fade-in' aria-hidden={isFeedbackHidden}>
+          <div
+            data-cy='failing-test-feedback'
+            className='test-status fade-in'
+            aria-hidden={isFeedbackHidden}
+          >
             <div className='status-icon' aria-hidden='true'>
               <span>
                 <Fail />
@@ -158,6 +159,8 @@ const LowerJaw = ({
           </div>
         </>
       );
+    } else {
+      return null;
     }
   };
 
@@ -169,16 +172,14 @@ const LowerJaw = ({
       'learn.sorry-hang-in-there',
       'learn.sorry-dont-giveup'
     ];
-    return attemptsNumber
-      ? sentenceArray[attemptsNumber % sentenceArray.length]
-      : sentenceArray[0];
+    return sentenceArray[currentAttempts % sentenceArray.length];
   };
 
   const renderContextualActionRow = () => {
     const isAttemptsLargerThanTest =
-      attemptsNumber &&
+      currentAttempts &&
       testsLength &&
-      (attemptsNumber >= testsLength || attemptsNumber >= 3);
+      (currentAttempts >= testsLength || currentAttempts >= 3);
 
     if (isAttemptsLargerThanTest && !challengeIsCompleted) {
       return (
@@ -225,8 +226,9 @@ const LowerJaw = ({
           ) : null}
           <button
             id='test-button'
+            data-cy='run-tests-button'
             className={`btn-block btn ${challengeIsCompleted ? 'sr-only' : ''}`}
-            aria-hidden={testBtnariaHidden}
+            aria-hidden={testBtnAriaHidden}
             onClick={tryToExecuteChallenge}
           >
             {showDesktopButton
@@ -235,6 +237,7 @@ const LowerJaw = ({
           </button>
           <button
             id='submit-button'
+            data-cy='submit-button'
             aria-hidden={!challengeIsCompleted}
             className='btn-block btn'
             onClick={tryToSubmitChallenge}
@@ -251,7 +254,7 @@ const LowerJaw = ({
     <div className='action-row-container'>
       {renderButtons()}
       <div
-        style={runningTests ? { height: `${testFeedbackheight}px` } : {}}
+        style={runningTests ? { height: `${testFeedbackHeight}px` } : {}}
         className={`test-feedback`}
         id='test-feedback'
         aria-live='assertive'

client/src/templates/Challenges/components/ResetModal.tsx

@@ -70,6 +70,7 @@ function ResetModal({ reset, close, isOpen }: ResetModalProps): JSX.Element {
       </Modal.Body>
       <Modal.Footer className='reset-modal-footer'>
         <Button
+          data-cy='reset-modal-confirm'
           block={true}
           bsSize='large'
           bsStyle='danger'

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

@@ -41,6 +41,7 @@ export const actionTypes = createTypes(
     'resetChallenge',
     'stopResetting',
     'submitChallenge',
+    'resetAttempts',
 
     'setEditorFocusability',
     'toggleVisibleEditor'

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

@@ -17,6 +17,7 @@ export { ns };
 
 const initialState = {
   canFocusEditor: true,
+  attempts: 0,
   visibleEditors: {},
   challengeFiles: [],
   challengeMeta: {
@@ -127,6 +128,7 @@ export const executeChallenge = createAction(actionTypes.executeChallenge);
 export const resetChallenge = createAction(actionTypes.resetChallenge);
 export const stopResetting = createAction(actionTypes.stopResetting);
 export const submitChallenge = createAction(actionTypes.submitChallenge);
+export const resetAttempts = createAction(actionTypes.resetAttempts);
 
 export const setEditorFocusability = createAction(
   actionTypes.setEditorFocusability
@@ -215,6 +217,7 @@ export const challengeDataSelector = state => {
   return challengeData;
 };
 
+export const attemptsSelector = state => state[ns].attempts;
 export const canFocusEditorSelector = state => state[ns].canFocusEditor;
 export const visibleEditorsSelector = state => state[ns].visibleEditors;
 
@@ -308,9 +311,14 @@ export const reducer = handleActions(
           testString
         })),
         consoleOut: [],
-        isResetting: true
+        isResetting: true,
+        attempts: 0
       };
     },
+    [actionTypes.resetAttempts]: state => ({
+      ...state,
+      attempts: 0
+    }),
     [actionTypes.stopResetting]: state => ({
       ...state,
       isResetting: false
@@ -361,7 +369,8 @@ export const reducer = handleActions(
     }),
     [actionTypes.executeChallenge]: state => ({
       ...state,
-      currentTab: 3
+      currentTab: 3,
+      attempts: state.attempts + 1
     }),
     [actionTypes.setEditorFocusability]: (state, { payload }) => ({
       ...state,

cypress/integration/default/learn/challenges/multifile.js

@@ -2,9 +2,10 @@ import translations from '../../../../../client/i18n/locales/english/translation
 const location =
   '/learn/2022/responsive-web-design/learn-accessibility-by-building-a-quiz/step-2';
 const selectors = {
-  testButton: '#test-button',
+  testButton: '[data-cy=run-tests-button]',
   monacoTabs: '.monaco-editor-tabs',
-  signInButton: '#action-buttons-container a[href$="/signin"]'
+  signInButton: '#action-buttons-container a[href$="/signin"]',
+  submitButton: '[data-cy=submit-button]'
 };
 
 describe('Challenge with multifile editor', () => {
@@ -27,13 +28,34 @@ describe('Challenge with multifile editor', () => {
       .contains('Check Your Code');
   });
 
+  it('resets the lower jaw when prompted', () => {
+    cy.get('[data-cy=failing-test-feedback]').should('not.exist');
+
+    cy.contains('Check Your Code').click();
+    cy.get('[data-cy=failing-test-feedback]').should('be.visible');
+    cy.contains('Restart Step').click();
+    cy.get('[data-cy=reset-modal-confirm').click();
+
+    cy.get('[data-cy=failing-test-feedback]').should('not.exist');
+  });
+
   // Page will change after this test. If your test requires page used in previous
   // tests make sure it is above this one
   it('prompts unauthenticated user to sign in to save progress', () => {
+    cy.visit(location);
     cy.focused().type('{end}{enter}<meta charset="UTF-8" />{ctrl+enter}');
     cy.get(selectors.signInButton).contains(translations.learn['sign-in-save']);
+    cy.contains(translations.learn['congratulations']);
     cy.get(selectors.signInButton).click();
     cy.go('back');
     cy.get(selectors.signInButton).should('not.exist');
   });
+
+  it('focuses the submit button after testing a valid solution', () => {
+    cy.visit(location);
+    cy.focused().type('{end}{enter}<meta charset="UTF-8" />');
+    cy.get(selectors.submitButton).should('not.be.focused');
+    cy.get(selectors.testButton).click();
+    cy.get(selectors.submitButton).should('be.focused');
+  });
 });