Make DOJO exercises not nullable and fix React key bug #2336
Conversation
|
@bryanjenningz is attempting to deploy a commit to the c0d3-prod Team on Vercel. A member of the Team first needs to authorize it. |
Codecov Report
@@ Coverage Diff @@
## master #2336 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 181 181
Lines 3179 3180 +1
Branches 850 851 +1
=========================================
+ Hits 3179 3180 +1
|
| @@ -66,6 +66,7 @@ const Exercises: React.FC<QueryDataProps<GetExercisesQuery>> = ({ | |||
| <Layout title={currentLesson.title}> | |||
| {exercise ? ( | |||
| <Exercise | |||
| key={exerciseIndex} | |||
There was a problem hiding this comment.
This line fixes a small React key bug which previously would make it possible to press "Show Answer" (to show the current exercise answer) then "Skip" (to skip to the next exercise) which caused the next exercise's answer to be shown.
| challengeName: exercise.module.name, | ||
| problem: exercise.description, | ||
| answer: exercise.answer, | ||
| explanation: exercise.explanation || '' |
There was a problem hiding this comment.
Exercise explanations are nullable so we have to handle it here by defauting to an empty string, which works fine and looks good (see pull request description for a picture of what it looks like).
| fireEvent.click(skipButton) | ||
| expect(queryByRole('button', { name: 'PREVIOUS' })).toBeInTheDocument() | ||
|
|
||
| skipButton = getByRole('button', { name: 'SKIP' }) |
There was a problem hiding this comment.
The reason why we need to reassign skipButton to the current skip button in the DOM is because we are using a React key prop which makes it so the Exercise component rerenders.
| @@ -87,7 +87,7 @@ const getExercisesData: GetExercisesQuery = { | |||
| }, | |||
| description: '```\nlet a = 1\na -= 2\n// what is a?\n```', | |||
| answer: '-1', | |||
| explanation: '`a -= 2` is a shorter way to write `a = a - 2`' | |||
| explanation: null | |||
There was a problem hiding this comment.
Adding a null exercise explanation to the mock exercise data so we can get full test coverage.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This pull request doesn't affect any user-facing pages.
This pull request makes the exercises type definition on the backend non-nullable (i.e.
[Exercise!]!) which makes the data easier to deal with. This won't cause any problems because we never return any null values in the exercises array.This pull request also fixes a small React key bug which made it possible to previously press "Show Answer" then "Skip" which would make it go to the next exercise but the next exercise would already be showing the answer. Now if you press "Show Answer" then "Skip" the next answer will not be showing because we added a
key={exerciseIndex}property to the Exercise component which causes a rerender whenexerciseIndexchanges.We also safely handle the case where there is no exercise explanation (notice the bottom right is empty because this exercise happens to not have an explanation):
How to test:
This pull request is a part of #2253