-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Partial Bugfix #1013 : Missing state management #1022
Conversation
- Got the number working but can't figure how to update the state with div class. - Printing the numbers in console.
Still don't know how to display the calculated data dynamically when function formatNumber is being called & returned '-' to replace it with my generated total numbers.
This pull request is being automatically deployed with ZEIT Now (learn more). 🔍 Inspect: https://zeit.co/covid19india/covid19india-react/edym0zovo |
src/components/mapexplorer.js
Outdated
totalTemp += Number(element); | ||
}); | ||
|
||
setTotal(totalTemp); |
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.
remove the extra variable 'totalTemp'. You could use, js reduce function to setTotal. This way you would not be required to disable eslint and code will be more concise and readable.
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.
This was my second attempt at React programming.
I knew I was missing state management of some sort.
Thanks for input. I’ll check & update the codebase.
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.
@faisal3389 Please look into this, I have updated the code with JS reduce function.
src/components/mapexplorer.js
Outdated
if (getMonth.length === 1) { | ||
getMonth = '0' + getMonth; | ||
} |
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.
This can be done in a single step using the padStart() in JS, https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/padStart
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.
Thanks, updated the code.
@SensehacK Hey, thanks for the PR! I'm glad that you were able to get as far as you could, and that's great! I also feel this is a good start to learning react, I won't be able to merge this though, however, feel free to open up new PRs. If you have any questions in the process, we can maybe help you with the react part too! Cheers :) |
@jeremyphilemon Thanks for the kind words, So far I have found React to be very quick to prototype stuff even with very less processing power. Also I have raised a new PR #1390 . Please let me know your feedback. |
Description of PR
This is a PR of displaying total tests checked and displaying it on the home page.
Sadly I was only able to get it working in my console window, don't know how to use state management in React.
Still this is my first time actually trying React and contributing to the project, Please suggest me how can I improve my code.
@KrishnaPravin has already implemented the PR bug fix. I'm just here to contribute what little I can to this website.
Type of PR
Relevant Issues
Fixes #1013
Checklist
Screenshots