Repository: freeCodeCamp/freeCodeCamp
File: client/src/templates/Challenges/quiz/show.tsx (TypeScript)

Code before:
    );

    if (unansweredQuestions.length) {
      setErrorMessage(
        t('learn.quiz.unanswered-questions', {
          unansweredQuestions: unansweredQuestions.join(', ')
        })
      );
      return;
    }

    validateAnswers();
    setHasSubmitted(true);
  };

  const handleSubmitAndGo = () => {
    openCompletionModal();
  };

  // Handle error message display on submit.
  // We keep the logic in a useEffect hook in order to subscribe to the `correctAnswerCount` change
  // as `correctAnswerCount` is a React state and its value is updated asynchronously,
  useEffect(() => {
    if (!hasSubmitted) {
      return;
    }

    setErrorMessage(
      t('learn.quiz.have-n-correct-questions', {
        correctAnswerCount,
        total: quiz.length
      })
    );

    if (correctAnswerCount === quiz.length) {
      openCompletionModal();
      setIsPassed(true);
    }
  }, [correctAnswerCount, quiz.length, hasSubmitted, openCompletionModal, t]);

  return (
    <Hotkeys
      executeChallenge={!isPassed ? handleAnswersCheck : handleSubmitAndGo}
      containerRef={container}
      nextChallengePath={nextChallengePath}
      prevChallengePath={prevChallengePath}
    >
      <LearnLayout>
        <Helmet
          title={`${blockNameTitle} | ${t('learn.learn')} | freeCodeCamp.org`}
        />

Review comment (bug):
`useEffect` feels like the wrong tool for this. We're using it to handle a click event, but in a roundabout way.

As I understand what's happening this is the sequence

1. User fills in the quiz (maybe partially)
2. If partially complete, we ask them to finish
3. If complete we validate
4. If it fails we update the state returned by `useQuiz` and do some logic in the show component i.e. call a function (let's call it `onSuccess`)
5. If it succeeds we again update the `useQuiz` state and do some different logic (`onFailure`, say) in the show component

Everything is fine, except 4. and 5. Reason being, we shouldn't have to look at the state to know to call `onSuccess`  i.e. checking `hasSubmitted`'s value and comparing `correctAnswerCount` to `quiz.length`. `onSuccess` should be called immediately in response to the click, rather than indirectly when the state changes.

If that makes sense, I think we can implement it by passing `onSuccess` and  `onFailure` into `useQuiz`, so that it can call them when `validateAnswers` is called.

What do you reckon?



Code after:
    const unansweredList = quizData.reduce<number[]>(
      (acc, curr, id) => (curr.selectedAnswer == null ? [...acc, id + 1] : acc),
      []
    );

    if (unansweredList.length > 0) {
      return t('learn.quiz.unanswered-questions', {
        unansweredQuestions: unansweredList.join(', ')
      });
    }

    return t('learn.quiz.have-n-correct-questions', {
      correctAnswerCount,
      total: quiz.length
    });
  }

  const errorMessage = getErrorMessage();

  return (
    <Hotkeys
      executeChallenge={!isPassed ? handleAnswersCheck : handleSubmitAndGo}
      containerRef={container}
      nextChallengePath={nextChallengePath}
      prevChallengePath={prevChallengePath}
    >
      <LearnLayout>
        <Helmet
          title={`${blockNameTitle} | ${t('learn.learn')} | freeCodeCamp.org`}
        />
        <Container className='quiz-challenge-container'>
          <Row>
            <Spacer size='medium' />
            <ChallengeTitle
              isCompleted={isChallengeCompleted}
              translationPending={translationPending}
            >
              {title}
            </ChallengeTitle>

            <Col md={8} mdOffset={2} sm={10} smOffset={1} xs={12}>
              <ChallengeDescription description={description} />
              <ObserveKeys>
                <Quiz questions={quizData} disabled={hasSubmitted} />
              </ObserveKeys>
              <Spacer size='medium' />
              <div aria-live='polite' aria-atomic='true'>
                {errorMessage}
              </div>
              <Spacer size='medium' />
              {/*