-
Notifications
You must be signed in to change notification settings - Fork 34
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
Use backend report status directly for "readiness" and "activation" #1430
base: feature/add-new-AA-storm
Are you sure you want to change the base?
Conversation
Build succeeded and deployed at https://prism-1430.surge.sh |
const category = AACategoryKeyToCategoryMap[categoryKey]; | ||
|
||
// If the storm status is not active, all watched districts should be marked as inactive | ||
if (!isActivated) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this check necessary? won't the sets be empty anyway further down?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this check is necessary because if isActivated is false, we need all watched districts to be explicitly added to naDistricts. Otherwise, they wouldn't be included anywhere.
Also we can be in ready status and have watched districts in the affected districts list. In this case the status already take in account the 'landfall leaditme > 72 hours' and we dont have to put those district in the activated list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a doubt about whether we should display districts in the NA>XXX boxes in the leftPanel when the report status is monitoring
or ready
because on figma, there is no district displayed when the status is ready / monitoring
Since, the spec have changed a bit in the meantime (and the figma template did not follow on changes), it could well be that the current implementation is as expected (ie display NAxxx boxes when status is monitoring/ready)
… into use-storm-status
Ok, is this ready for review by @wadhwamatic ? |
]; | ||
} | ||
return [activeResult, naResult]; | ||
// Determine if watched districts are active based on storm status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to add a test on parseAndTransformAA()
Description
This PR is a change to use the report status to display readiness in the left panel and activated districts.
What this means is that if the status is ready, even though some districts are listed as activated, we don't show them as active since the landfall lead time is above 72 hours.
In the future we might display activated regions in that case, when they will be available from the backend.
How to test the feature:
Checklist - did you ...
Test your changes with
REACT_APP_COUNTRY=rbd yarn start
REACT_APP_COUNTRY=cambodia yarn start
REACT_APP_COUNTRY=mozambique yarn start
Screenshot/video of feature: