search

njyjn / sg-vaccine-tracker

A data scraper that provides the latest percentage of Singapore's population which have received the COVID-19 vaccination. All data from MOH.
https://vaccine.justinng.net
MIT License
6 stars 1 forks source link

Add separator commas to counts #12

Closed njyjn closed 3 years ago

njyjn commented 3 years ago

The counts on the frontend are not being separated by commas, leading to a hard-to-read experience for users

The starting point is here https://github.com/njyjn/sg-vaccine-tracker/blob/3e3ff768033726a5b7d31409b9fa843681b2ec57/client/src/components/Counts.tsx#L104-L139

Edleyng commented 3 years ago

state.count.valueChange and state.count.valueChangeAvgPerDay are undefined types. Can't seem to change it into a number for tolocalestring to work. Any suggestions on how to go about doing this?

njyjn commented 3 years ago

@Edleyng There is potential for those properties to be undefined as they are optional historical values. Note that in the interface definition the ? is used to mark it as an optional type https://github.com/njyjn/sg-vaccine-tracker/blob/3e3ff768033726a5b7d31409b9fa843681b2ec57/client/src/types/Count.ts#L13

Therefore, in JS, the property could be either a number or undefined. You may want to use the existing formatXXX() methods which are already implementing the necessary guards

https://github.com/njyjn/sg-vaccine-tracker/blob/3e3ff768033726a5b7d31409b9fa843681b2ec57/client/src/components/Counts.tsx#L185-L190

For example, for previous, if it is applied after the guard if (!previous) JS knows that previous can no longer be undefined.

Edleyng commented 3 years ago

how do I go about testing on my local machine before I want to add and commit it to the branch?

njyjn commented 3 years ago

I am working on an easier way to launch everything including the client from a single Docker compose file but that will take a while.

I am also realizing that the manual setup for testing offline requires an AWS account since there is some request signing happening in the background (there are secret tokens hosted on AWS that can be mocked offline but need actual AWS credentials for signing) Hence the impetus for a single Docker compose file that can be shared.

In the meantime unit testing with a mocked DB will probably be the best way forward, but I don't have the bandwidth to look at that yet.

So for now please just commit and I will review on my local machine. Thanks!

njyjn commented 3 years ago

Released in #16, great work