Skip to content

Test ends as we complete the paragraph #4

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.

Sign up for GitHub

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

Open
Divyansh0811 wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Test ends as we complete the paragraph #4

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ec4073d
Test ends as we complete the paragraph fixed
Divyansh0811 Jun 9, 2021
7c77791
files updated redo
Divyansh0811 Jun 14, 2021
435d989
changes done
Divyansh0811 Jun 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"@testing-library/jest-dom": "^5.11.4",
"@testing-library/react": "^11.1.0",
"@testing-library/user-event": "^12.1.10",
"dependencies": "0.0.1",
Copy link
Copy Markdown
Member

@MadhavBahl MadhavBahl Jun 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this for?

Copy link
Copy Markdown
Author

@Divyansh0811 Divyansh0811 Jun 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgive me for this, earlier i used to type "npm install dependencies" instead of "npm install" after cloning a repo. Learned this thing yesterday

Copy link
Copy Markdown
Member

@MadhavBahl MadhavBahl Jun 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha, no worries, it happens ✨

Also, you don't need to apologize for these small mistakes, you're contributing to open source in your free time, so I should be the one thanking you,

Just delete this line, and it should be alright

"gh-pages": "^3.1.0",
"react": "^17.0.1",
"react-dom": "^17.0.1",
Expand Down
9 changes: 9 additions & 0 deletions src/components/App/App.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,15 @@ class App extends React.Component {
return;
}

if (index === this.state.selectedParagraph.length - 1) {
this.setState({
characters,
words,
timeRemaining: 0,
});
return;
}

Comment on lines +165 to +181
Copy link
Copy Markdown
Member

@MadhavBahl MadhavBahl Jun 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic looks good, but we should probably also update the score here

Since - if the user finishes the tests before time, the time won't be 60 seconds, and his/her wpm will be more

In case you wish to work on this in this PR itself, it'd be cool

But, If you want, this can be taken up in a separate PR/issue, open another issue and add a comment here with a todo something like this -

// TODO: Update the test score for this case, here the time taken isn't 60 seconds, so the wpm should be modified

Copy link
Copy Markdown
Author

@Divyansh0811 Divyansh0811 Jun 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will the update WPM will be calculated using the below formula?
wpm = wordsTyped * totalTime/(totalTime - timeRemaining)

For eg:
1)A person types 45 words in 15 sec, his answer will be = 45 x 60/(60-45) = 180wpm.
2)A person types 20 words in 10 sec, his answer will be = 20 x 60/(60-40) = 120wpm.

Copy link
Copy Markdown
Member

@MadhavBahl MadhavBahl Jun 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, sounds perfect!

// Make a copy
const testInfo = this.state.testInfo;
if (!(index === this.state.selectedParagraph.length - 1))
Expand Down