neherlab / covid19_scenarios

Models of COVID-19 outbreak trajectories and hospital demand
https://covid19-scenarios.org
MIT License
1.36k stars 354 forks source link

add progress indicator to the result section #706

Closed Chong-anson closed 4 years ago

Chong-anson commented 4 years ago

Related issues and PRs

Description

Impacted Areas in the application

Testing

vercel[bot] commented 4 years ago

This pull request is being automatically deployed with Vercel (learn more). To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/covid19-scenarios/covid19-scenarios/3flh3tfrr ✅ Preview: https://covid19-scenarios-git-fork-chong-anson-progress-indicator.covid19-scenarios.now.sh

codecov[bot] commented 4 years ago

Codecov Report

Merging #706 into master will decrease coverage by 0.05%. The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #706      +/-   ##
==========================================
- Coverage   24.58%   24.53%   -0.06%     
==========================================
  Files         134      135       +1     
  Lines        2896     2902       +6     
  Branches      386      388       +2     
==========================================
  Hits          712      712              
- Misses       2184     2190       +6     
Impacted Files Coverage Δ
src/components/Main/Results/PlotSpinner.tsx 0.00% <0.00%> (ø)
src/components/Main/Results/ResultsCard.tsx 0.00% <0.00%> (ø)
...ponents/Main/Scenario/ConfirmedCasesDataPicker.tsx 0.00% <0.00%> (ø)
ivan-aksamentov commented 4 years ago

@Chong-anson Thanks, it looks promising, however I insist on this important feature (from the original issue):

  • should not obscure the plot completely, such that the moment when changes arrive can be seen

The whole idea of the app is to allow user to explore different parameters and to see how they affect the results (for example, when relaxing the quarantine measures, user should be able to see that the death plot goes up, etc.) In this PR, when loading indicator is shown, the plot disappears and the entire results section changes its layout. This makes it difficult to track the changes visually when changing parameters.

Instead of conditional rendering (spinner vs plot) we could implement a semi-transparent overlay on top of the plot, such that the data and the transition from one result to another is visible at all times.

Note, that rendering the whole plot from scratch is very costly (takes from 500 to 1000ms on average machine), so we want to ensure that only the changed items are re-rendered (for example, axes, grid and legend stays the same). This means that we need to avoid unmounting the plot component, changing it's size and changing its props unnecessarily.

Also, currently, the size is changing when switching between the spinner and the plot. Implementing overlay should hopefully fix that. Additional bonus for ensuring that there is no size jump on first render (you know, when the results section "unrolls" down on app load).

This is a creative issue, so feel free to experiment, implement your own idea of how it may look like and refactor the code structure when needed.

Chong-anson commented 4 years ago

Thanks for your detailed comment! I will work on that!

Chong-anson commented 4 years ago

Hi Ivan, I edited the components in ResultCard and added some CSS. Sorry for the delay, my internet was down. I tried to work on the "unrolling issue" in the first render, but have been stuck. As the render was handled by React-Resize-Detector, I cannot interfere with the sizing

codeclimate[bot] commented 4 years ago

Code Climate has analyzed commit ac025d01 and detected 0 issues on this pull request.

View more on Code Climate.

ivan-aksamentov commented 4 years ago

This looks good thanks! Resolves #688