From 6d562b0c26648237ec4b00b47b4abfac14fecb46 Mon Sep 17 00:00:00 2001 From: Bryan Jennings Date: Sat, 1 Oct 2022 11:57:26 -0700 Subject: [PATCH 1/8] Add DOJO exercise filter checkbox --- pages/exercises/[lessonSlug].tsx | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/pages/exercises/[lessonSlug].tsx b/pages/exercises/[lessonSlug].tsx index 50cfaab8e..8bca25b9d 100644 --- a/pages/exercises/[lessonSlug].tsx +++ b/pages/exercises/[lessonSlug].tsx @@ -24,6 +24,7 @@ const Exercises: React.FC> = ({ }) => { const { lessons, alerts, exercises, exerciseSubmissions } = queryData const router = useRouter() + const [hideAnswered, setHideAnswered] = useState(false) const [exerciseIndex, setExerciseIndex] = useState(-1) const [addExerciseSubmission] = useAddExerciseSubmissionMutation() const [userAnswers, setUserAnswers] = useState>({}) @@ -70,6 +71,9 @@ const Exercises: React.FC> = ({ explanation: exercise.explanation || '', userAnswer: userAnswers[exercise.id] ?? null })) + .filter( + exercise => !hideAnswered || exercise.userAnswer !== exercise.answer + ) const exercise = currentExercises[exerciseIndex] @@ -95,6 +99,8 @@ const Exercises: React.FC> = ({ tabs={tabs} setExerciseIndex={setExerciseIndex} lessonTitle={currentLesson.title} + hideAnswered={hideAnswered} + setHideAnswered={setHideAnswered} exercises={currentExercises} /> )} @@ -185,6 +191,8 @@ type ExerciseListProps = { tabs: { text: string; url: string }[] setExerciseIndex: React.Dispatch> lessonTitle: string + hideAnswered: boolean + setHideAnswered: (hideAnswered: boolean) => void exercises: { moduleName: string problem: string @@ -197,6 +205,8 @@ const ExerciseList = ({ tabs, setExerciseIndex, lessonTitle, + hideAnswered, + setHideAnswered, exercises }: ExerciseListProps) => { return ( @@ -208,7 +218,19 @@ const ExerciseList = ({ />
-

{lessonTitle}

+
+

{lessonTitle}

+ +
@@ -225,7 +247,7 @@ const ExerciseList = ({ ))} From 81607cdd8af895a995c22169a303a75dc74069de Mon Sep 17 00:00:00 2001 From: Bryan Jennings Date: Sat, 1 Oct 2022 13:15:14 -0700 Subject: [PATCH 2/8] Move React state down to Exercise component so it can handle filtered exercises --- pages/exercises/[lessonSlug].tsx | 111 ++++++++++++++++++++----------- 1 file changed, 71 insertions(+), 40 deletions(-) diff --git a/pages/exercises/[lessonSlug].tsx b/pages/exercises/[lessonSlug].tsx index 8bca25b9d..a4e3fd25c 100644 --- a/pages/exercises/[lessonSlug].tsx +++ b/pages/exercises/[lessonSlug].tsx @@ -24,8 +24,8 @@ const Exercises: React.FC> = ({ }) => { const { lessons, alerts, exercises, exerciseSubmissions } = queryData const router = useRouter() + const [solvingExercise, setSolvingExercise] = useState(false) const [hideAnswered, setHideAnswered] = useState(false) - const [exerciseIndex, setExerciseIndex] = useState(-1) const [addExerciseSubmission] = useAddExerciseSubmissionMutation() const [userAnswers, setUserAnswers] = useState>({}) useEffect(() => { @@ -75,29 +75,31 @@ const Exercises: React.FC> = ({ exercise => !hideAnswered || exercise.userAnswer !== exercise.answer ) - const exercise = currentExercises[exerciseIndex] - return ( - {exercise ? ( + {solvingExercise ? ( 0} - hasNext={exerciseIndex < currentExercises.length - 1} - submitUserAnswer={(userAnswer: string) => { - setUserAnswers({ ...userAnswers, [exercise.id]: userAnswer }) + exercises={currentExercises} + userAnswers={userAnswers} + onExit={localUserAnswers => { + setUserAnswers({ ...userAnswers, ...localUserAnswers }) + setSolvingExercise(false) + }} + submitUserAnswer={(exerciseId, userAnswer) => { addExerciseSubmission({ - variables: { exerciseId: exercise.id, userAnswer } + variables: { exerciseId, userAnswer } }) }} /> ) : ( { + if (currentExercises.length > 0) { + setSolvingExercise(true) + } + }} lessonTitle={currentLesson.title} hideAnswered={hideAnswered} setHideAnswered={setHideAnswered} @@ -110,42 +112,48 @@ const Exercises: React.FC> = ({ } type ExerciseData = { + id: number problem: string answer: string explanation: string } type ExerciseProps = { - exercise: ExerciseData - setExerciseIndex: React.Dispatch> lessonTitle: string - hasPrevious: boolean - hasNext: boolean - submitUserAnswer: (userAnswer: string) => void + exercises: ExerciseData[] + userAnswers: Record + submitUserAnswer: (exerciseId: number, userAnswer: string) => void + onExit: (userAnswers: Record) => void } const Exercise = ({ - exercise, - setExerciseIndex, lessonTitle, - hasPrevious, - hasNext, - submitUserAnswer + exercises, + userAnswers, + submitUserAnswer, + onExit }: ExerciseProps) => { const [answerShown, setAnswerShown] = useState(false) const [message, setMessage] = useState(Message.EMPTY) + const [exerciseIndex, setExerciseIndex] = useState(0) + const [localUserAnswers, setLocalUserAnswers] = useState(userAnswers) + const exercise = exercises[exerciseIndex] + + const hasPrevious = exerciseIndex > 0 + const hasNext = exerciseIndex < exercises.length - 1 return (

{lessonTitle}

{ + setLocalUserAnswers({ + ...localUserAnswers, + [exercise.id]: userAnswer + }) + submitUserAnswer(exercise.id, userAnswer) + }} />
{hasPrevious ? (
{ + if (exercise.userAnswer === exercise.answer) return 'ANSWERED' + if (exercise.userAnswer) return 'INCORRECT' + return 'NOT ANSWERED' + })()} problem={exercise.problem} /> ))} From 5833acc517874e1556feb5f7db430cdea9f3f091 Mon Sep 17 00:00:00 2001 From: Bryan Jennings Date: Sat, 1 Oct 2022 14:20:53 -0700 Subject: [PATCH 4/8] Add tests for DOJO exercises show-incomplete-only checkbox --- .../pages/exercises/[lessonSlug].test.js | 109 ++++++++++++++++++ pages/exercises/[lessonSlug].tsx | 6 +- 2 files changed, 110 insertions(+), 5 deletions(-) diff --git a/__tests__/pages/exercises/[lessonSlug].test.js b/__tests__/pages/exercises/[lessonSlug].test.js index 738271e14..1ae6f3a78 100644 --- a/__tests__/pages/exercises/[lessonSlug].test.js +++ b/__tests__/pages/exercises/[lessonSlug].test.js @@ -111,6 +111,115 @@ describe('Exercises page', () => { expect(queryByRole('button', { name: 'SKIP' })).not.toBeInTheDocument() }) + test('Renders different exercises depending on whether the show-only-incompleted checkbox is checked', async () => { + const mocks = [ + { + request: { query: GET_EXERCISES }, + result: { + data: getExercisesData + } + }, + { + request: { + query: ADD_EXERCISE_SUBMISSION, + variables: { + exerciseId: 2, + userAnswer: 'blah blah' + } + }, + result: { + data: { + addExerciseSubmissions: { + id: 1, + exerciseId: 2, + userId: 4, + userAnswer: 'blah blah' + } + } + } + }, + { + request: { + query: ADD_EXERCISE_SUBMISSION, + variables: { + exerciseId: 3, + userAnswer: '-1' + } + }, + result: { + data: { + addExerciseSubmissions: { + id: 1, + exerciseId: 3, + userId: 4, + userAnswer: '-1' + } + } + } + } + ] + + const { findByLabelText, findAllByText, findByRole } = render( + + + + ) + + let checkbox = await findByLabelText('Show incomplete exercises only') + + let exercisePreviews = await findAllByText('Problem') + expect(exercisePreviews.length).toBe(3) + let notAnsweredExercisePreviews = await findAllByText('NOT ANSWERED') + expect(notAnsweredExercisePreviews.length).toBe(2) + + fireEvent.click(checkbox) + + exercisePreviews = await findAllByText('Problem') + expect(exercisePreviews.length).toBe(2) + notAnsweredExercisePreviews = await findAllByText('NOT ANSWERED') + expect(notAnsweredExercisePreviews.length).toBe(2) + + const solveExercisesButton = await findByRole('button', { + name: 'SOLVE EXERCISES' + }) + fireEvent.click(solveExercisesButton) + + let inputBox = await findByLabelText('User answer') + fireEvent.change(inputBox, { + target: { value: 'blah blah' } + }) + let submitButton = await findByRole('button', { name: 'SUBMIT' }) + fireEvent.click(submitButton) + + const skipButton = await findByRole('button', { name: 'SKIP' }) + fireEvent.click(skipButton) + + inputBox = await findByLabelText('User answer') + fireEvent.change(inputBox, { + target: { value: '-1' } + }) + submitButton = await findByRole('button', { name: 'SUBMIT' }) + fireEvent.click(submitButton) + + const nextQuestionButton = await findByRole('button', { + name: 'NEXT QUESTION' + }) + fireEvent.click(nextQuestionButton) + + exercisePreviews = await findAllByText('Problem') + expect(exercisePreviews.length).toBe(1) + let incorrectExercisePreviews = await findAllByText('INCORRECT') + expect(incorrectExercisePreviews.length).toBe(1) + + checkbox = await findByLabelText('Show incomplete exercises only') + fireEvent.click(checkbox) + + exercisePreviews = await findAllByText('Problem') + expect(exercisePreviews.length).toBe(3) + incorrectExercisePreviews = await findAllByText('INCORRECT') + expect(incorrectExercisePreviews.length).toBe(1) + }) + test('Should not render lessons nav card tab if lesson docUrl is null', async () => { const mocks = [ { diff --git a/pages/exercises/[lessonSlug].tsx b/pages/exercises/[lessonSlug].tsx index 73d3c09e7..23a0036bf 100644 --- a/pages/exercises/[lessonSlug].tsx +++ b/pages/exercises/[lessonSlug].tsx @@ -95,11 +95,7 @@ const Exercises: React.FC> = ({ ) : ( { - if (currentExercises.length > 0) { - setSolvingExercise(true) - } - }} + onClickSolveExercises={() => setSolvingExercise(true)} lessonTitle={currentLesson.title} hideAnswered={hideAnswered} setHideAnswered={setHideAnswered} From 1488552389ab3604b3eeada076428f71d278e63f Mon Sep 17 00:00:00 2001 From: Bryan Jennings Date: Sat, 1 Oct 2022 14:38:32 -0700 Subject: [PATCH 5/8] Only show SOLVE EXERCISES button if there is at least 1 exercise --- pages/exercises/[lessonSlug].tsx | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/pages/exercises/[lessonSlug].tsx b/pages/exercises/[lessonSlug].tsx index 23a0036bf..2bf9c1b45 100644 --- a/pages/exercises/[lessonSlug].tsx +++ b/pages/exercises/[lessonSlug].tsx @@ -261,13 +261,15 @@ const ExerciseList = ({ Show incomplete exercises only
-
- - SOLVE EXERCISES - -
+ {exercises.length > 0 && ( +
+ + SOLVE EXERCISES + +
+ )}
{exercises.map((exercise, i) => ( From 36855fbc4914e7f26a6c9bc60fa056c67b8e4b9b Mon Sep 17 00:00:00 2001 From: Bryan Jennings Date: Sat, 1 Oct 2022 15:04:26 -0700 Subject: [PATCH 6/8] Use expect(a).toHaveLength(b) instead of expect(a.length).toBe(b) for better error messages --- __tests__/pages/exercises/[lessonSlug].test.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/__tests__/pages/exercises/[lessonSlug].test.js b/__tests__/pages/exercises/[lessonSlug].test.js index 1ae6f3a78..203c24bcd 100644 --- a/__tests__/pages/exercises/[lessonSlug].test.js +++ b/__tests__/pages/exercises/[lessonSlug].test.js @@ -168,16 +168,16 @@ describe('Exercises page', () => { let checkbox = await findByLabelText('Show incomplete exercises only') let exercisePreviews = await findAllByText('Problem') - expect(exercisePreviews.length).toBe(3) + expect(exercisePreviews).toHaveLength(3) let notAnsweredExercisePreviews = await findAllByText('NOT ANSWERED') - expect(notAnsweredExercisePreviews.length).toBe(2) + expect(notAnsweredExercisePreviews).toHaveLength(2) fireEvent.click(checkbox) exercisePreviews = await findAllByText('Problem') - expect(exercisePreviews.length).toBe(2) + expect(exercisePreviews).toHaveLength(2) notAnsweredExercisePreviews = await findAllByText('NOT ANSWERED') - expect(notAnsweredExercisePreviews.length).toBe(2) + expect(notAnsweredExercisePreviews).toHaveLength(2) const solveExercisesButton = await findByRole('button', { name: 'SOLVE EXERCISES' @@ -207,17 +207,17 @@ describe('Exercises page', () => { fireEvent.click(nextQuestionButton) exercisePreviews = await findAllByText('Problem') - expect(exercisePreviews.length).toBe(1) + expect(exercisePreviews).toHaveLength(1) let incorrectExercisePreviews = await findAllByText('INCORRECT') - expect(incorrectExercisePreviews.length).toBe(1) + expect(incorrectExercisePreviews).toHaveLength(1) checkbox = await findByLabelText('Show incomplete exercises only') fireEvent.click(checkbox) exercisePreviews = await findAllByText('Problem') - expect(exercisePreviews.length).toBe(3) + expect(exercisePreviews).toHaveLength(3) incorrectExercisePreviews = await findAllByText('INCORRECT') - expect(incorrectExercisePreviews.length).toBe(1) + expect(incorrectExercisePreviews).toHaveLength(1) }) test('Should not render lessons nav card tab if lesson docUrl is null', async () => { From cefee5ec751d8661d051da7c245a1fdfa2076ab6 Mon Sep 17 00:00:00 2001 From: Bryan Jennings Date: Sat, 1 Oct 2022 15:09:33 -0700 Subject: [PATCH 7/8] Replace await waitFor(() => getBy*) with await findBy* --- .../pages/exercises/[lessonSlug].test.js | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/__tests__/pages/exercises/[lessonSlug].test.js b/__tests__/pages/exercises/[lessonSlug].test.js index 203c24bcd..e7a17d734 100644 --- a/__tests__/pages/exercises/[lessonSlug].test.js +++ b/__tests__/pages/exercises/[lessonSlug].test.js @@ -1,5 +1,5 @@ import React from 'react' -import { render, waitFor, screen, fireEvent } from '@testing-library/react' +import { render, screen, fireEvent } from '@testing-library/react' import '@testing-library/jest-dom' import Exercises from '../../../pages/exercises/[lessonSlug]' import { useRouter } from 'next/router' @@ -28,9 +28,7 @@ describe('Exercises page', () => { ) - await waitFor(() => - screen.getByRole('heading', { name: /Foundations of JavaScript/i }) - ) + await screen.findByRole('heading', { name: /Foundations of JavaScript/i }) screen.getByRole('link', { name: 'CHALLENGES' }) screen.getByRole('link', { name: 'EXERCISES' }) @@ -66,15 +64,13 @@ describe('Exercises page', () => { } ] - const { getByRole, queryByRole, getByLabelText } = render( + const { getByRole, findByRole, queryByRole, getByLabelText } = render( ) - await waitFor(() => - getByRole('heading', { name: /Foundations of JavaScript/i }) - ) + await findByRole('heading', { name: /Foundations of JavaScript/i }) const solveExercisesButton = getByRole('button', { name: 'SOLVE EXERCISES' @@ -242,9 +238,7 @@ describe('Exercises page', () => { ) - await waitFor(() => - screen.getByRole('heading', { name: /Foundations of JavaScript/i }) - ) + await screen.findByRole('heading', { name: /Foundations of JavaScript/i }) screen.getByRole('link', { name: 'CHALLENGES' }) screen.getByRole('link', { name: 'EXERCISES' }) @@ -270,7 +264,7 @@ describe('Exercises page', () => { ) - await waitFor(() => screen.getByRole('heading', { name: /500 Error/i })) + await screen.findByRole('heading', { name: /500 Error/i }) }) test('Should render a 404 error page if the lesson is not found', async () => { @@ -292,7 +286,7 @@ describe('Exercises page', () => { ) - await waitFor(() => screen.getByRole('heading', { name: /404 Error/i })) + await screen.findByRole('heading', { name: /404 Error/i }) }) test('Should render a loading spinner if useRouter is not ready', async () => { @@ -314,6 +308,6 @@ describe('Exercises page', () => { ) - await waitFor(() => screen.getByText('Loading...')) + await screen.findByText('Loading...') }) }) From 4159728f8a62c1b691857be02d96046d8824c570 Mon Sep 17 00:00:00 2001 From: Bryan Jennings Date: Sat, 1 Oct 2022 16:32:22 -0700 Subject: [PATCH 8/8] Add state property to exercise data --- pages/exercises/[lessonSlug].tsx | 35 +++++++++++++++++++------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/pages/exercises/[lessonSlug].tsx b/pages/exercises/[lessonSlug].tsx index 2bf9c1b45..5d2585741 100644 --- a/pages/exercises/[lessonSlug].tsx +++ b/pages/exercises/[lessonSlug].tsx @@ -12,7 +12,9 @@ import Error, { StatusCode } from '../../components/Error' import LoadingSpinner from '../../components/LoadingSpinner' import AlertsDisplay from '../../components/AlertsDisplay' import NavCard from '../../components/NavCard' -import ExercisePreviewCard from '../../components/ExercisePreviewCard' +import ExercisePreviewCard, { + ExercisePreviewCardProps +} from '../../components/ExercisePreviewCard' import { NewButton } from '../../components/theme/Button' import ExerciseCard, { Message } from '../../components/ExerciseCard' import { ArrowLeftIcon } from '@primer/octicons-react' @@ -63,14 +65,22 @@ const Exercises: React.FC> = ({ const currentExercises = exercises .filter(exercise => exercise?.module.lesson.slug === slug) - .map(exercise => ({ - id: exercise.id, - moduleName: exercise.module.name, - problem: exercise.description, - answer: exercise.answer, - explanation: exercise.explanation || '', - userAnswer: userAnswers[exercise.id] ?? null - })) + .map(exercise => { + const userAnswer = userAnswers[exercise.id] ?? null + return { + id: exercise.id, + moduleName: exercise.module.name, + problem: exercise.description, + answer: exercise.answer, + explanation: exercise.explanation || '', + userAnswer, + state: ((): ExercisePreviewCardProps['state'] => { + if (userAnswer === exercise.answer) return 'ANSWERED' + if (userAnswer) return 'INCORRECT' + return 'NOT ANSWERED' + })() + } + }) .filter( exercise => !hideAnswered || exercise.userAnswer !== exercise.answer ) @@ -220,6 +230,7 @@ type ExerciseItem = { problem: string answer: string userAnswer: string | null + state: ExercisePreviewCardProps['state'] } type ExerciseListProps = { @@ -276,11 +287,7 @@ const ExerciseList = ({ { - if (exercise.userAnswer === exercise.answer) return 'ANSWERED' - if (exercise.userAnswer) return 'INCORRECT' - return 'NOT ANSWERED' - })()} + state={exercise.state} problem={exercise.problem} /> ))}