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

Task/#256 add statistics details #264

Merged
merged 34 commits into from
May 24, 2020
Merged

Conversation

OldMetalmind
Copy link
Member

@OldMetalmind OldMetalmind commented May 10, 2020

Closes #256

Description
Adds plot regarding Confirmed Cases, Deaths, Recovered and Hospitalized information of the current status of Covid19 in Portugal. It is possible to filter by day.

Important Note for testers
The library used for ploting was fl_flutter (https://pub.dev/packages/fl_chart) and given its limitation not every feature was able to be implemented, before opening an issue please contact me @OldMetalmind .

Not implemented

  • Logarithmic scales
  • "Ticks" for the vertical lines for he x axis days
  • HospitalizedUCI cases proportion not showing in between backgrounds in the plot
  • Horizontal Bar plots

Notes

  • Given the amount of new files and changes, it would be preferable to have two reviews before merging
  • After this merge, we need to create more tasks for not implemented features and find a better solution for the plots and its limitations.
  • There is a lot of room for improvements and code optimizations.

Changes

  • Added new screens for each of the types of plots details
  • Navigation from the updated Statistics overview
  • Loading the data from the Splash screen

Required to run
This is running to against the local running Docker container, you will need to set it up on your machine or se the production build to test.

https://github.com/dssg-pt/covid19pt-data

@OldMetalmind OldMetalmind requested a review from danidroid May 10, 2020 19:03
@OldMetalmind OldMetalmind self-assigned this May 10, 2020
@Vanethos
Copy link
Collaborator

Can you please add screenshots? :D

Copy link
Member

@miguelpruivo miguelpruivo left a comment

Choose a reason for hiding this comment

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

I have to take a whole day to review this! 😄

Copy link
Collaborator

@Vanethos Vanethos left a comment

Choose a reason for hiding this comment

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

Overall.. AMAZING work! Pro moves detected!!!

Only a few minor things, including a missing license and commented code

Copy link
Member

@miguelpruivo miguelpruivo left a comment

Choose a reason for hiding this comment

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

Overall, like @Vanethos said, it looks great and there's an incredible amount of work here. Congrats! 🎉

Other than minor things that I added to @Vanethos suggestions, it looks ok to me as well. I would only run flutter format . on the project just to make sure all files are formatted accordingly.

Copy link
Collaborator

@danidroid danidroid left a comment

Choose a reason for hiding this comment

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

Amazing work @OldMetalmind
just found some small things on UI while running on smaller devices and some imports out of place above license

@OldMetalmind OldMetalmind requested a review from danidroid May 17, 2020 16:11
@OldMetalmind OldMetalmind merged commit 88baff6 into dev May 24, 2020
@OldMetalmind OldMetalmind deleted the task/#256_add_statistics_details branch November 14, 2020 17:49
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.

Statistics Detail - New Plots
4 participants