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

JS Map - Use native zoom levels #4007

Merged
merged 7 commits into from
Dec 12, 2023
Merged

Conversation

rldhont
Copy link
Collaborator

@rldhont rldhont commented Dec 6, 2023

For EPSG:3857 and others if only 2 scales are defined, the native zoom levels are used for the Map.

Some legacy codes are removed.

FUnded by 3Liz

@rldhont rldhont added map viewer javascript Pull requests that update Javascript code run end2end If the PR must run end2end tests or not backport release_3_7 labels Dec 6, 2023
@github-actions github-actions bot added this to the 3.8.0 milestone Dec 6, 2023
@Gustry
Copy link
Member

Gustry commented Dec 6, 2023

As discussed together about the print, KISS : same projection everywhere ?

Linked to PR 3liz/lizmap-plugin#526

@github-actions github-actions bot added the tests unit tests and docker configuration for tests label Dec 7, 2023
@Gustry
Copy link
Member

Gustry commented Dec 8, 2023

Trying to sum up the conversation from yesterday :

version <= 3.6 :

  • nothing changed
  • CFG configuration like existing
  •        use_native_scales: false, 
          "mapScales": [
              100,
              500,
              1000,
              2500,
              5000,
              10000,
              25000,
              50000,
              100000,
              250000,
              500000
          ],
          "minScale": 100,
          "maxScale": 500000,

image

Version >= 3.7

  • checkbox "Use native".
  • If checked :
    • users must set min and max
    • scales list is hidden
  • If unchecked :
    • min and max readonly
    • scales list editable
    • min and max readonly because determined from scales list
    • no more warning like screenshot above
    • Only if the scale list is displayed, show the information text about print ?
    •     use_native_scales: true
          "mapScales": [
              100,
              500000
          ],
          "minScale": 100,
          "maxScale": 500000,
      Still mapscales with only max items ?

@Gustry
Copy link
Member

Gustry commented Dec 11, 2023

I'm trying the PR. Looks good !

Side question, whatever the checkbox about native scales, if the map list has only two values, we have native, right ?

@Gustry
Copy link
Member

Gustry commented Dec 11, 2023

Side question, whatever the checkbox about native scales, if the map list has only two values, we have native, right ?

According to JS code, it seems not, but anyway, very rare to have only two values in this list.

https://github.com/3liz/lizmap-web-client/pull/4007/files#diff-0da40e308a26999f09137dd8e1062f96599436c1d67d4c1289054d4494539659R204

@rldhont rldhont force-pushed the use-native-zoom-levels branch from 649e2ed to 71371e0 Compare December 11, 2023 16:09
@rldhont rldhont marked this pull request as ready for review December 11, 2023 16:11
@rldhont
Copy link
Collaborator Author

rldhont commented Dec 11, 2023

@nboisteault @Gustry ready for reiews

@rldhont rldhont force-pushed the use-native-zoom-levels branch from 71371e0 to 6255665 Compare December 11, 2023 16:34
@rldhont
Copy link
Collaborator Author

rldhont commented Dec 11, 2023

Linked to 3liz/lizmap-plugin#537

@Gustry
Copy link
Member

Gustry commented Dec 12, 2023

Side question, whatever the checkbox about native scales, if the map list has only two values, we have native, right ?

Indeed, it doesn't happen anymore.

LGTM for me

Copy link
Member

@Gustry Gustry left a comment

Choose a reason for hiding this comment

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

From the test side

Copy link
Member

@nboisteault nboisteault left a comment

Choose a reason for hiding this comment

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

👍

@rldhont rldhont merged commit 427a69a into 3liz:master Dec 12, 2023
9 checks passed
@3liz-bot
Copy link
Contributor

The backport to release_3_7 failed:

The process '/usr/bin/git' failed with exit code 1
stderr
error: could not apply 6678ebfdd... JS: Use native zoom levels
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

stdout
Auto-merging assets/src/legacy/map.js
CONFLICT (content): Merge conflict in assets/src/legacy/map.js

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-release_3_7 release_3_7
# Navigate to the new working tree
cd .worktrees/backport-release_3_7
# Create a new branch
git switch --create backport-4007-to-release_3_7
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick 6678ebfdd3f81235212cdd1ede253e8ec0420395,f9008de2aa03c4c0d7375b7a606ff9f6f9bd076e,1beeb89fa3145bcd084802cec15bf9bf9bccbf77,c3a45ca933058adec2439ace7f60e7cae6e432fc,3da806b6ee0b8be59931bc01c35050d1809235cb,50d383d24cac91054deaeeeaac26b3ab538deef1,6255665daf1de9f53b2dbbd9d420af113bb25c8c
# Push it to GitHub
git push --set-upstream origin backport-4007-to-release_3_7
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-release_3_7

Then, create a pull request where the base branch is release_3_7 and the compare/head branch is backport-4007-to-release_3_7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
failed backport javascript Pull requests that update Javascript code map viewer run end2end If the PR must run end2end tests or not tests unit tests and docker configuration for tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants