-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
result related #2 #7
Conversation
src/components/Questions/index.js
Outdated
|
||
const { index, end, userAnswers, currentQuestion, score } = this.state | ||
|
||
if (target.value === currentQuestion.answer) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use object destructing for target.value, or you can just pass the value from onClick function instead of destructing here as you only need the value from the target object.
userAnswers: userAnswers.concat({ index, status: "false" }) | ||
}) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would be more clear if you separate this into another method and call it here, you could name it addProgress or addUserAnswers
userAnswers: userAnswers.concat({ index, status: "false" }) | ||
}) | ||
} | ||
|
||
index !== data.length - 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also here you can move this code into a separate method and call it e.g (moveToNextQuestion) or just nextQuestion but then you have to change the current function name as it already has this name, and give it a more descriptive name
src/components/Questions/index.js
Outdated
{options.map((option, i) => { | ||
return ( | ||
<div key={i}> | ||
{" "} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need for space here, remove it, please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
really nice work 👍
I left some comments to do some changes
src/components/Questions/index.js
Outdated
this.setState({ selected: target.value }) | ||
|
||
const { index, end, currentQuestion, score } = this.state | ||
const { value } = target |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still this need to be moved to first of the function so you can use it here too
this.setState({ selected: target.value })
No description provided.