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

[DRAFT]: Divided AA into drought and storm layers #1370

Merged

Conversation

Doniaab
Copy link
Collaborator

@Doniaab Doniaab commented Nov 22, 2024

Description

  • [Create another type of anticipatory action : Anticipatory Action Storm]
  • [ Update code to handle multiple anticipatory actions]
  • [Update nav items to handle childrens, add menu selection item on click]
  • [Separates AAStorm panel and AADrought panel]

How to test the feature:

  • [Click on the anticipatory action menu to switch between AAStorm and AADrought layers]
  • [Panels should be differents]

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:

return;
}

// Change AA layer in url
Copy link
Collaborator Author

@Doniaab Doniaab Nov 23, 2024

Choose a reason for hiding this comment

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

TODO: Changes in this file are wip.
should rework AA layers switch in url, currently not working correctly

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

switch between AA in url is now working but date query no longer appear in url after switch

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

resolved

@@ -3,7 +3,7 @@ import { Provider } from 'react-redux';
import { BrowserRouter } from 'react-router-dom';
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the views (District, Forecast, Hometable, Timeline...) will be different for AA storm so they should probably live under AA Drought? What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess we can move it later, I just want to be careful when copying components like this not to carry over extra complexity and avoidable dependence

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, I have not yet touched on the distinction between the view elements specific to each AA. Also i wanted to test if everything was loaded correctly when i switch AA.

I'm going to see what can be put in common for the two AA types, perhaps extracting common parts.
But I understand that the current view elements are specific to AA drought and must be separated from AA storm

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have moved the AA drought specific components to AADroughtPanel folder

Copy link

github-actions bot commented Nov 24, 2024

Build succeeded and deployed at https://prism-1370.surge.sh
(hash 4c7b9f9 deployed at 2024-12-02T15:25:55)

@ericboucher ericboucher marked this pull request as draft November 24, 2024 21:06
@Max-Z80 Max-Z80 force-pushed the feat/handle-multiple-AA-layers-and-panels branch from e3e89d9 to 26dc723 Compare November 27, 2024 13:53
@@ -37,7 +38,7 @@ export function keepLayer(layer: LayerType, newLayer: LayerType) {
return false;
}

if (newLayer.type === layer.type && layer.type === 'anticipatory_action') {
if (newLayer.type === layer.type && isAnticipatoryActionLayer(layer.type)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this will only be needed for aa_drought

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, this is only relevant for the aa_drought because it handles the fact that aa_drought can display the layer of each 'window' simultanously on the map. There is no concept of window in the aa_storm layer, as far as I know. I will fix that.

@ericboucher ericboucher marked this pull request as ready for review December 1, 2024 22:20
@Max-Z80 Max-Z80 merged commit 2a64115 into feature/add-new-AA-storm Dec 2, 2024
6 checks passed
@Max-Z80 Max-Z80 deleted the feat/handle-multiple-AA-layers-and-panels branch December 2, 2024 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants