Feature/f 51 ipc map #35

ahmedfarouk2000 · 2024-11-17T20:00:04Z

The IPC Map with popOver on hovered colored countries

I didn't use any layers in here since its only about coloring the IPCs countries.
generateColorMap func: the logic looks complicated but its becase the IPCs data from endpoint that ends with ipc.json does not include all the data needed thus I need to get them from countryMapData endpoint that ends with adm0data.json, so I think its better to refactor those endpoints.

…legend

netlify · 2024-11-17T20:01:12Z

✅ Deploy Preview for wfp-hungermap ready!

Name	Link
🔨 Latest commit	`b740a89`
🔍 Latest deploy log	https://app.netlify.com/sites/wfp-hungermap/deploys/67406eea11e29c0008e8604e
😎 Deploy Preview	https://deploy-preview-35--wfp-hungermap.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

jschoedl

LGTM! Although I haven't worked with Mapbox yet and can't say too much about it.

I added some suggestions regarding types and variable names.

src/operations/IPC/IpcMapOperations.tsx

src/operations/map/MapOperations.ts

src/styles/globals.css

tailwind.config.js

marinovl7

Please revisit the data fetching issues with @Tschonti and communicate about the tooltip/popup with @ArmanpreetGhotra. Precise naming for the functions is needed to distinguish between ipc functions, nutrition functions etc. Overall the request and the functionality are perfect!

marinovl7 · 2024-11-18T18:46:43Z

src/app/page.tsx

+  const ipcDataPromise = globalRepo.getIpcData();
+  const [countryMapData, disputedAreas, ipcData] = await Promise.all([
+    countryMapDataPromise,
+    disputedAreasPromise,
+    ipcDataPromise,
+  ]);
+


@Tschonti we should not fetch the IPC Data here right? Any suggestions where to move this, so that we fetch the data only on sidebar click on the ipc icon

Yeah it'd be great if we didn't fetch it here, but it was done so the mapbox layers and interactivity can be set up with this data. I'm not sure if mapbox is okay with dynamically creating a new layer (though it probably is).

the IPC data endpoint is ~21 KB, which is not that small, but way smaller than adm0 for example, so I don't think it makes a big difference to prefetch this.

marinovl7 · 2024-11-18T18:47:46Z

src/components/Map/Map.tsx

@@ -24,7 +24,7 @@ export default function Map({ countries, disputedAreas }: MapProps) {
      style={{ height: '100%', width: '100%', zIndex: 1 }}
    >
      <AlertContainer />
-      {countries && <VectorTileLayer countries={countries} disputedAreas={disputedAreas} />}
+      {countries && <VectorTileLayer countries={countries} disputedAreas={disputedAreas} ipcData={ipcData} />}


maybe a new layer would be needed for the normal VectorTile layer instead? @Tschonti

I tried not to have additional layer, since having additional layer will have some sort of unnatural behaviour when zooming in or out (not sure if it's solvable or not)

I think this will depend on IPC/CH Country View if this will show details about country ipc phases thus, its better to have a seprate layer.

src/operations/map/MapOperations.ts

marinovl7 · 2024-11-18T18:50:20Z

src/operations/map/MapOperations.ts

+      if (hoveredPolygonId && (!feature || hoveredPolygonId !== feature.id)) {
+        baseMap.setFeatureState({ source: 'countries', id: hoveredPolygonId }, { hover: false });
+        hoveredPolygonId = undefined;
+        baseMap.getCanvas().style.cursor = '';
+      }


When you hover a country the country color is set to nothing, this needs to be fixed, maybe that is causing the issue

what issue exactly?

marinovl7 · 2024-11-18T18:51:10Z

src/operations/map/MapOperations.ts

+          const formattedPopNum = ipc_popnmbr.toLocaleString('en-US', {
+            useGrouping: true,
+          });
+          const title = `${countryName}`;
+          const body = `Date of analysis: ${date_of_analysis} <br> Validity period: ${analysis_period}`;
+          const conclusion = `${formattedPopNum} people in IPC/CH Phase 3 and above <br> (${ipc_percent}% of people in the analyzed areas)`;
+
+          const popupContent = this.generatePopupContent(title, body, conclusion);
+          popup.setLngLat(e.lngLat).setHTML(popupContent).addTo(baseMap);
+          break;


Can't we use our tooltip here? Maybe align with @ArmanpreetGhotra

We can for sure, but I prefer using the mapBox pop up which adaptively adjusts its position based on the mouse's location. e.g if there is no space on the right side of the screen, the popup will automatically appear on the left, and so on.

src/operations/map/MapOperations.ts

jschoedl

One small remark - the rest looks fine for me. :)

src/operations/map/MapOperations.ts

Tschonti · 2024-11-21T13:26:25Z

I merged main - the MapOperations file changed a lot, so there was a huge merge conflict and had to refactor this solution to make it work. The only thing that's different than before the merge is that the inactive countries are still greyed out on the IPC map.

ahmedfarouk2000 added 9 commits November 14, 2024 00:16

feat: init ipc map

49227ca

fix: match source code logic

57a2b86

fix: other counties color

a2f5a82

feat: added popup on country hover

7eefe57

Merge branch 'main' into feature/f-51-ipc-map

b2befab

feat: formalize popover on country hover with proper style, show ipc …

61059f1

…legend

Merge branch 'main' into feature/f-51-ipc-map

a7819ef

fix: seperate lines for tooltip hover

15acac6

fix: uncomment stuff

03f00f8

ahmedfarouk2000 requested review from Tschonti, jschoedl, marinovl7 and bohdangarchu November 17, 2024 20:00

ahmedfarouk2000 self-assigned this Nov 17, 2024

jschoedl requested changes Nov 18, 2024

View reviewed changes

marinovl7 requested changes Nov 18, 2024

View reviewed changes

ahmedfarouk2000 added 3 commits November 19, 2024 00:01

fix: improve types and variable names

4fa41e6

fix: undo the deleted .env file

caebab8

fix: highlight numbers in hovered country popup

030f456

ahmedfarouk2000 requested a review from jschoedl November 19, 2024 00:32

jschoedl reviewed Nov 19, 2024

View reviewed changes

src/operations/map/MapOperations.ts Outdated Show resolved Hide resolved

fix: convert the popover into a react component to be reusable

46fe156

ahmedfarouk2000 requested a review from marinovl7 November 19, 2024 19:57

Merge branch 'main' into feature/f-51-ipc-map

84a6dbe

Tschonti added 2 commits November 21, 2024 14:30

fix: click event

09c13ef

fix: fix country click for real

20dee69

jschoedl approved these changes Nov 22, 2024

View reviewed changes

Update zIndex in CountryHoverPopover.tsx

b740a89

marinovl7 approved these changes Nov 22, 2024

View reviewed changes

marinovl7 merged commit 207eab5 into main Nov 22, 2024
5 checks passed

marinovl7 deleted the feature/f-51-ipc-map branch November 22, 2024 11:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/f 51 ipc map #35

Feature/f 51 ipc map #35

ahmedfarouk2000 commented Nov 17, 2024

netlify bot commented Nov 17, 2024 •

edited

Loading

jschoedl left a comment

marinovl7 left a comment

marinovl7 Nov 18, 2024

Tschonti Nov 18, 2024

marinovl7 Nov 18, 2024

ahmedfarouk2000 Nov 18, 2024

ahmedfarouk2000 Nov 18, 2024

marinovl7 Nov 18, 2024

ahmedfarouk2000 Nov 18, 2024

marinovl7 Nov 18, 2024

ahmedfarouk2000 Nov 18, 2024

ahmedfarouk2000 Nov 18, 2024

ahmedfarouk2000 Nov 18, 2024

jschoedl left a comment

Tschonti commented Nov 21, 2024

Feature/f 51 ipc map #35

Feature/f 51 ipc map #35

Conversation

ahmedfarouk2000 commented Nov 17, 2024

netlify bot commented Nov 17, 2024 • edited Loading

✅ Deploy Preview for wfp-hungermap ready!

jschoedl left a comment

Choose a reason for hiding this comment

marinovl7 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jschoedl left a comment

Choose a reason for hiding this comment

Tschonti commented Nov 21, 2024

netlify bot commented Nov 17, 2024 •

edited

Loading