feat: add testing coldspot page #223
Conversation
s-bessey
changed the title
|
Visit the preview URL for this PR (updated for commit a1adc6a): https://signal-ri--pr223-testing-coldspot-ourk1o27.web.app (expires Thu, 01 Feb 2024 17:42:15 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 00e607c4a7f37e76a51e675795a92e0f2f741992 |
| <template> | ||
| <DashboardCard width="full"> | ||
| <template #subtitle> | ||
| TThis tool shows us places where fewer people were tested for COVID-19 |
There was a problem hiding this comment.
Noticed this typo but I assume that we're not really taking a look at this this go around though
| TThis tool shows us places where fewer people were tested for COVID-19 | |
| This tool shows us places where fewer people were tested for COVID-19 |
There was a problem hiding this comment.
This is looking good! A few things that I noticed when I was looking at the site and not the code:
The "How do we reach people" Card
The spacing on this card is a little odd... Because the title goes across the width of the card, the table feels like it should be vertically centered when it isn't. This might be out of scope for this PR, but I just noticed it.
The Gap Chart Hover Highlight
This also might be out of scope for this PR but the highlighting is weird... Admittedly, this might be my fault but it feels odd that the whole bar isn't highlighting?
| // { | ||
| // type: "text", | ||
| // from: { data: "bars" }, | ||
| // encode: { | ||
| // enter: { | ||
| // x: { field: "x2", offset: -5 }, | ||
| // y: { field: "y", offset: { field: "height", mult: 0.5 } }, | ||
| // fill: { value: "#FFFFFF" }, | ||
| // align: { value: "right" }, | ||
| // baseline: { value: "middle" }, | ||
| // // text: { | ||
| // // signal: "format(datum.datum.rate, '.0d')", | ||
| // // }, | ||
| // }, | ||
| // }, | ||
| // }, |
There was a problem hiding this comment.
Do you need to keep this?
There was a problem hiding this comment.
Was just waiting for feedback on it, but I think not
src/utils/utils.ts
Outdated
| export const formatUsString = new Intl.NumberFormat("en-US", { | ||
| minimumFractionDigits: 0, | ||
| maximumFractionDigits: 0, | ||
| }); |
There was a problem hiding this comment.
This is a little bit of a nitpick, but I wonder if this would be better written as a function? In all the other files you're calling formatUsString.format() which is fine, but it could be cleaner if you wrote it like this I think:
| export const formatUsString = new Intl.NumberFormat("en-US", { | |
| minimumFractionDigits: 0, | |
| maximumFractionDigits: 0, | |
| }); | |
| export const USStringFormat = new Intl.NumberFormat("en-US", { | |
| minimumFractionDigits: 0, | |
| maximumFractionDigits: 0, | |
| }); | |
| export function formatUsString(usString) { | |
| return USStringFormat.format(usString); | |
| } |
This way, you have the format defined by Intl and a small wrapper to format rates that you can use without needing to remember that you need to call .format(). How does that feel?
|
Data expected this April 2024 @s-bessey |
No description provided.