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 globe/mercator button #963

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Draft

Add globe/mercator button #963

wants to merge 12 commits into from

Conversation

wipfli
Copy link

@wipfli wipfli commented Jan 4, 2025

Launch Checklist

Adds a button for choosing the projection globe or mercator. Appears under the light settings. Live demo is here: https://wipfli.github.io/maputnik/?layer=1455082206%7E0&modal=settings#1.02/0/0

image

  • Briefly describe the changes in this PR.
  • Link to related issues.
  • Include before/after visuals or gifs if this PR includes visual changes.
  • Write tests for all new functionality.
  • Add an entry to CHANGELOG.md under the ## main section.

.github/workflows/deploy.yml Outdated Show resolved Hide resolved
@HarelM
Copy link
Collaborator

HarelM commented Jan 4, 2025

I would recommend adding a test...

@wipfli
Copy link
Author

wipfli commented Jan 4, 2025

Do you mind helping me with the test? I don't know cypress but happy to learn about it if you can help me a bit

@HarelM
Copy link
Collaborator

HarelM commented Jan 4, 2025

sure, this repo has a good practice when it comes to end-to-end tests.
There's the driver which is the low level usage of cypress, usually, you won't need to touch it:
https://github.com/maplibre/maputnik/blob/main/cypress/e2e/maputnik-driver.ts
There's the helper which obscures how the UI looks and facilitate function to use in the tests so that the tests should look like a flow given-when-then style, but it's very slim in this case:
https://github.com/maplibre/maputnik/blob/main/cypress/e2e/maputnik-cypress-helper.ts
And there's the relevant test, since you added your functionality in a modal I would start by looking at a similar test here:
https://github.com/maplibre/maputnik/blob/main/cypress/e2e/modals.cy.ts

Let me know if the above is enough or not.
Thanks!

@HarelM
Copy link
Collaborator

HarelM commented Jan 21, 2025

The maplibre packages' versions were updated to use latest ones, can you update the branch? It should reduce changes as part of this PR as well.

@birkskyum
Copy link
Member

Need rebase, since main is on GL JS 5

@birkskyum birkskyum marked this pull request as draft January 21, 2025 08:57
@codecov-commenter
Copy link

codecov-commenter commented Jan 21, 2025

Codecov Report

Attention: Patch coverage is 42.10526% with 11 lines in your changes missing coverage. Please review.

Project coverage is 62.53%. Comparing base (af01346) to head (7cc137b).
Report is 45 commits behind head on main.

Files with missing lines Patch % Lines
src/components/ModalSettings.tsx 42.10% 11 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #963      +/-   ##
==========================================
+ Coverage   59.84%   62.53%   +2.68%     
==========================================
  Files         104      104              
  Lines        3011     5786    +2775     
  Branches      680     1720    +1040     
==========================================
+ Hits         1802     3618    +1816     
- Misses       1209     2167     +958     
- Partials        0        1       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

4 participants