Skip to content

Conversation

@Divyansh0811
Copy link

No description provided.

"version": "1.13.1",
"resolved": "https://registry.npmjs.org/underscore/-/underscore-1.13.1.tgz",
"integrity": "sha512-hzSoAVtJF+3ZtiFX0VgfFPHEDRm7Y/QPjGyNo4TVdnDTdft3tr8hEkD25a1jC+TjTuE7tkHGKkhwCgs9dgBB2g=="
},
Copy link
Member

Choose a reason for hiding this comment

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

You can delete this file, no need of updating this

package.json Outdated
"@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
Member

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
Author

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
Member

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

Comment on lines 165 to 173
if (index === this.state.selectedParagraph.length - 1) {
this.setState({
characters,
words,
timeRemaining: 0,
});
return;
}

Copy link
Member

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
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.

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
Member

Choose a reason for hiding this comment

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

Yes, sounds perfect!

Copy link
Member

@MadhavBahl MadhavBahl left a comment

Choose a reason for hiding this comment

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

Three changes

  • Delete package-lock.json file - it can be autogenerated once again
  • Revert package.json
  • wpm logic

package.json Outdated
Comment on lines 2 to 42
"name": "flashtype",
"version": "0.1.0",
"private": true,
"homepage": "http://theleanprogrammer.com/flashtype/",
"dependencies": {
"@testing-library/jest-dom": "^5.11.4",
"@testing-library/react": "^11.1.0",
"@testing-library/user-event": "^12.1.10",
"gh-pages": "^3.1.0",
"react": "^17.0.1",
"react-dom": "^17.0.1",
"react-scripts": "4.0.2",
"typewriter-effect": "^2.17.0",
"web-vitals": "^1.0.1"
},
"scripts": {
"start": "react-scripts start",
"build": "react-scripts build",
"test": "react-scripts test",
"eject": "react-scripts eject",
"predeploy": "npm run build",
"deploy": "gh-pages -d build"
},
"eslintConfig": {
"extends": [
"react-app",
"react-app/jest"
]
},
"browserslist": {
"production": [
">0.2%",
"not dead",
"not op_mini all"
],
"development": [
"last 1 chrome version",
"last 1 firefox version",
"last 1 safari version"
]
}
Copy link
Member

Choose a reason for hiding this comment

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

Let's not change the tab width to 4 spaces, I prefer keeping it 2 spaces in json files

Can you revert this change?

Copy link
Author

Choose a reason for hiding this comment

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

yes, sure.

Comment on lines 165 to 173
if (index === this.state.selectedParagraph.length - 1) {
this.setState({
characters,
words,
timeRemaining: 0,
});
return;
}

Copy link
Member

Choose a reason for hiding this comment

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

Yes, sounds perfect!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants