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

#8334 Persist viewer type in the map configuration #8990

Merged
merged 10 commits into from
Mar 8, 2023

Conversation

allyoucanmap
Copy link
Contributor

Description

This PR introduces the concept of visualization mode inside mapstore with following changes:

  • store the visualizationMode information inside the saved map configuration to persist the mode every time a map is opened
  • remove path that include mapType eg /viewer/openlayers/1. This change has retrocompatible support by recognizing the visualization mode based on mapType provided in the url and replacing the url with the new format /viewer/1
  • add configuration in localConfig to define which library should be used based on device. default config is:
{
    "mapType": {
        "2D": {
            "desktop": "openlayers",
            "mobile": "leaflet"
        },
        "3D": {
            "desktop": "cesium",
            "mobile": "cesium"
        }
    }
}
  • maps saved with 3D mode included in geostories will be displayed in 3D mode with some limitations:
    • scroll events does not pass through the map
    • default zoom controls needs to be re-implemented
    • identify popup is not available
  • maps saved with 3D mode included in dashboard will be displayed in 3D mode with some limitations:
    • identify popup is not available

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x", remove the others)

  • Minor changes to existing features

Issue

What is the current behavior?

#8334

What is the new behavior?

Breaking change

Does this PR introduce a breaking change? (check one with "x", remove the other)

  • Yes

Other useful information

@allyoucanmap allyoucanmap added enhancement 3D All issues related to the 3D rendering in CesiumJs C027-COMUNE_FI-2023-SUPPORT labels Mar 1, 2023
@allyoucanmap allyoucanmap added this to the 2023.01.01 milestone Mar 1, 2023
@allyoucanmap allyoucanmap self-assigned this Mar 1, 2023
@allyoucanmap allyoucanmap linked an issue Mar 1, 2023 that may be closed by this pull request
2 tasks
@allyoucanmap allyoucanmap modified the milestones: 2023.01.01, 2023.02.00 Mar 1, 2023
@allyoucanmap allyoucanmap marked this pull request as ready for review March 1, 2023 17:22
@tdipisa tdipisa requested a review from offtherailz March 1, 2023 17:25
Copy link
Member

@offtherailz offtherailz left a comment

Choose a reason for hiding this comment

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

Generally looks good. A few suggestion for clean up in the inline comments.

notes I asked serveral "use constants" in tests. It is a good practice, anyway I don't want to be to much boring

web/client/epics/__tests__/maptype-test.js Outdated Show resolved Hide resolved
web/client/components/geostory/common/map/Controls.jsx Outdated Show resolved Hide resolved
web/client/components/map/enhancers/autoMapType.js Outdated Show resolved Hide resolved
web/client/plugins/containers/ToolsContainer.jsx Outdated Show resolved Hide resolved
@@ -28,7 +29,7 @@ class Manager extends React.Component {

static defaultProps = {
items: [],
mapType: "openlayers",
mapType: MapLibraries.OPENLAYERS,
Copy link
Member

Choose a reason for hiding this comment

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

This can be removed, looks a copy and paste error

@@ -80,7 +81,7 @@ class ManagerMenu extends React.Component {
itemSelected: () => {},
title: <MenuItem header>Manager</MenuItem>,
controls: [],
mapType: "leaflet",
mapType: MapLibraries.LEAFLET,
Copy link
Member

Choose a reason for hiding this comment

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

same of the other ToolContainer. it is used in it only. Moreover is set to leaflet and not mapped to the state, so I think it can be removed.

web/client/product/appConfig.js Show resolved Hide resolved
web/client/reducers/__tests__/mapInfo-test.js Outdated Show resolved Hide resolved
@allyoucanmap
Copy link
Contributor Author

@offtherailz as discussed I'll move visualizationMode inside the map config

@allyoucanmap allyoucanmap requested a review from offtherailz March 6, 2023 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3D All issues related to the 3D rendering in CesiumJs C027-COMUNE_FI-2023-SUPPORT enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Persist viewer type in the map configuration
2 participants