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

AA storm feature / revisit landfall popup display #1410

Conversation

Max-Z80
Copy link
Collaborator

@Max-Z80 Max-Z80 commented Jan 14, 2025

Description

This PR is based on #1407
But this PR also uses code merged from #1408 => this PR shoud be merged after #1408 is merged

  • display a new popup containing 'landfall', permanently visible, on the wind point where the estimated landfall time is expected
  • display popup containing landfall details when clicking on this wind point only. (ie this popup is not displayed any more when clicking on other wind points
  • move the popup containing landfall details to the right (instead on having this popup being centered in the wind point)
  • fix behaviour of the landfall popup when clicking on it several times
  • redefine how the landfall marker is displayed: visible in the format LF-{date} as long as the landfall did not occur. Not visible otherwise.

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
  • Add / update necessary tests?
  • Add / update outdated documentation?

Screenshot/video of feature:

image

@Max-Z80 Max-Z80 force-pushed the aa-storm-feature/revisit-landfall-popup-display branch from 9199a46 to 1635a2c Compare January 14, 2025 17:26
Copy link

github-actions bot commented Jan 14, 2025

Build succeeded and deployed at https://prism-1410.surge.sh
(hash 32845f0 deployed at 2025-01-17T13:40:06)

Base automatically changed from improve-style-of-windpoint-tooltip to feature/add-new-AA-storm January 14, 2025 21:03
@ericboucher ericboucher changed the base branch from feature/add-new-AA-storm to feature/permanently-show-last-analysed-date-tooltip January 14, 2025 21:04
@ericboucher
Copy link
Collaborator

@Max-Z80 let's move the tooltip to the right a bit as Marc did in the Figma designs to avoid hiding parts of the country
I am not a fan of the "landfall" text but don't really have a better idea haha. Maybe Marc could have ideas?

Base automatically changed from feature/permanently-show-last-analysed-date-tooltip to feature/add-new-AA-storm January 15, 2025 14:39
@@ -227,7 +229,7 @@ const AnticipatoryActionStormLayer = React.memo(
e.preventDefault();
dispatch(hidePopup()); // hides the black tooltip containing the district names
const feature = e.features?.[0];
setSelectedFeature(feature as Feature<Point>);
setSelectedFeature(feature as unknown as AAStormTimeSeriesFeature);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this avoidable?

Copy link
Collaborator

Choose a reason for hiding this comment

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

(using "unknown")

Copy link
Collaborator Author

@Max-Z80 Max-Z80 Jan 16, 2025

Choose a reason for hiding this comment

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

yes, I have pushed a simple version without unknown.

@ericboucher ericboucher merged commit 8721d26 into feature/add-new-AA-storm Jan 17, 2025
6 checks passed
@ericboucher ericboucher deleted the aa-storm-feature/revisit-landfall-popup-display branch January 17, 2025 16:23
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.

2 participants