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

Add dropdown to select map style #1209

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Add dropdown to select map style #1209

wants to merge 19 commits into from

Conversation

echaidemenos
Copy link
Collaborator

@echaidemenos echaidemenos commented Apr 29, 2024

Description

This PR adds a drop-down on the navbar to select the map style.

Implement @wadhwamatic 's suggestion: #1199 (comment)

TODO: remove the "street" styles, they are just for test the feature is working. Find working satellite styles.

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

@echaidemenos echaidemenos requested review from ericboucher and wadhwamatic and removed request for wadhwamatic April 29, 2024 13:25
Copy link

github-actions bot commented Apr 30, 2024

Build succeeded and deployed at https://prism-1209.surge.sh
(hash 7e93fe1 deployed at 2024-08-22T06:09:38)

@echaidemenos echaidemenos changed the base branch from master to 1199-print-labels April 30, 2024 12:27
Base automatically changed from 1199-print-labels to master April 30, 2024 20:52
@ericboucher
Copy link
Collaborator

ericboucher commented Jul 17, 2024

@wadhwamatic I updated the branch of this PR to catchup with main.

We need to update the firstSymbolId logic to account for the fact different layers have different symbol layer ids
Are there other changes you would want to make? Or should we close this PR for now?

@wadhwamatic
Copy link
Member

@ericboucher - I'm not sure what the firstSymbolId logic does. Can you explain?

I think that the order might need to be configurable. In this example, if we change the base to satellite, and try to show layers, they do not appear presumably because the satellite layer is on top. This includes the boundary layer which is hidden

I also see that toggling back and forth between styles breaks rendering of admin level data.

@ericboucher
Copy link
Collaborator

@wadhwamatic yep that's because of the first symbol logic that loops through the different layers in style to find the first symbol and position our admin boundaries and other layers below it. It's currently not updating when you switch style which then creates errors when loading the other layers

@wadhwamatic
Copy link
Member

@ericboucher - ok understood. Are you suggesting we create a separate PR to address firstSymbolId? I'm ok with that approach. Let's just make sure it's captured in an issue so we don't lose track of this.

@ericboucher
Copy link
Collaborator

ericboucher commented Aug 1, 2024

@wadhwamatic this is ready for review. We do a full refresh of the map upon map style selection.

However, we currently have the extra map styles in shared which means this functionality is active for all countries. This doesn't feel like what we would want righ?
Also, I'm not a fan of the world icon which is often used for languages and recommend using the layers icon instead but that's up to you.

Screenshot 2024-08-01 at 2 46 20 PM

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.

3 participants