Skip to content
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.

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

case count UI #160

Closed
wants to merge 3 commits into from
Closed

case count UI #160

wants to merge 3 commits into from

Conversation

Mati-Roy
Copy link
Contributor

issue: #144

@hnykda hnykda self-requested a review March 18, 2020 13:20
Copy link
Contributor

@hnykda hnykda left a comment

Choose a reason for hiding this comment

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

Nice. I think we can merge so it doesn't desynchronize with master.

Those reformatting stuff are making it harder to review.

@hnykda hnykda self-requested a review March 18, 2020 13:21
@hnykda
Copy link
Contributor

hnykda commented Mar 18, 2020

Wait - I actually don't see any code which would do what you printscreened at the issue

Copy link
Contributor

@hnykda hnykda left a comment

Choose a reason for hiding this comment

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

I don't see the code which should do the change. Was it commited?

Is it necessary to include lines.js if there are no changes? Isn't that an autoformatting and then git commit -a combination consequence?

@lagerros
Copy link
Contributor

@hnykda I ran this locally and this implement those changes, see these lines:

image

@lagerros lagerros self-requested a review March 18, 2020 17:59
Copy link
Contributor

@lagerros lagerros left a comment

Choose a reason for hiding this comment

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

Styling request:

image

  • Have the "Confirmed cases" and the actual numbers be in separate columns, both of which are left aligned
  • Have the actual numbers be in that shade of orange
  • Switch around numbers and titles
  • Put the "90% confidence interval, based on a combination of statistical modelling and human forecasting" in smaller font below
  • Add a separating line before containment measures

Copy link
Contributor

@hnykda hnykda left a comment

Choose a reason for hiding this comment

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

Apologies, my bad...

OK from my POV, but there are Jacob's additions.

@hnykda
Copy link
Contributor

hnykda commented Mar 19, 2020

Hey Mati. Thanks a lot, it's still helpful as we are using it in #104, but just not in a sidebar as Jan wanted to remove that one for now. So I am closing this one and @wolverdude is going to do something similar but in the current view

@hnykda hnykda closed this Mar 19, 2020
Covid automation moved this from In progress to Done Mar 19, 2020
@hnykda hnykda deleted the case-count branch March 19, 2020 17:21
@hnykda hnykda added invalid This doesn't seem right and removed invalid This doesn't seem right labels Mar 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Covid
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants