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

Configurable basemaps from init json #5140

Merged
merged 27 commits into from Mar 22, 2021
Merged

Conversation

zoran995
Copy link
Contributor

@zoran995 zoran995 commented Jan 14, 2021

What this PR does

Fixes #907

  • Enables defining the list of baseMaps in init files. This was requested a long time ago (Allow base maps to be specified in init files #907) but never implemented. When using bingMaps, we check if there is a specified bingMapsKey and, if not, set the default one from configParameters.
  • Creates a default baseMap list to reflect the existing state
  • Implements previewBaseMapId init file parameter so dataset preview baseMap can be changed (there is no more a fixed list of available baseMaps so we can't be sure that positron baseMap will exist)

Testing

Add this to the init file and check that there are only 2 base maps.

"baseMaps": [
    {
      "item": {
        "id": "basemap-positron",
        "name": "Positron (Light)",
        "type": "open-street-map",
        "url": "https://basemaps.cartocdn.com/light_all/",
        "attribution": "© <a href='https://www.openstreetmap.org/copyright'>OpenStreetMap</a>, © <a href='https://carto.com/about-carto/'>CARTO</a>",
        "subdomains": ["a", "b", "c", "d"],
        "opacity": 1.0
      },
      "image": "/images/positron.png"        
    },
    {
      "item": {
        "id": "basemap-darkmatter",
        "name": "Dark Matter",
        "type": "open-street-map",
        "url": "https://basemaps.cartocdn.com/dark_all/",
        "attribution": "© <a href='https://www.openstreetmap.org/copyright'>OpenStreetMap</a>, © <a href='https://carto.com/about-carto/'>CARTO</a>",
        "subdomains": ["a", "b", "c", "d"],
        "opacity": 1.0
      },
      "image": "/images/dark-matter.png" 
    }
  ]
}

Pending

  • Check if the base map with a specific id is already added to avoid duplicates
  • Tests
  • Documentation - edit all init properties should be documented now
  • Create TerriaMap PR to remove references to deleted functions

Future work

  • Fix share URL so that base map set in it works correctly. @nf-s can we update the parameter for share to baseMapId instead of baseMapName - edit I couldn't get share basemap to work on ci next
  • URL parameter baseMapId - so user can explicitly set the basemap
  • Add attribution traits for all catalogItems - done in Mappable attribution #5167

Checklist

  • There are unit tests to verify my changes are correct or unit tests aren't applicable (if so, write quick reason why unit tests don't exist)
  • I've updated CHANGES.md with what I changed.

- baseMaps now can be configured from init files
- create default baseMaps list to reflect existing state from next
- clear default baseMaps when baseMaps are provided in init file
- enable configuration of dataPreview baseMap from init files
@zoran995 zoran995 marked this pull request as draft January 14, 2021 22:31
@zoran995
Copy link
Contributor Author

zoran995 commented Jan 15, 2021

Thinking right now would it might be better to move the default list to configParameters, or adding the defaultBaseMaps.json? Then the user should have an option to specify additional baseMaps in the init file or to remove some using ID.

edit - will move this to future work, after search bar

@zoran995 zoran995 marked this pull request as ready for review January 17, 2021 19:18
@zoran995
Copy link
Contributor Author

While writing documentation I also converted all initData parameters to table form and added the ones that were missing. Hope everyone is ok with that. From my point of view, this is ready for review.

@nf-s nf-s self-assigned this Jan 19, 2021
@zoran995
Copy link
Contributor Author

Docs are still missing the timeline and models property of initData, I wasn't sure if timeline is used.
For the models I didn't know where it is used, can anyone else document it?

@zoran995 zoran995 mentioned this pull request Jan 21, 2021
2 tasks
Copy link
Contributor

@nf-s nf-s left a comment

Choose a reason for hiding this comment

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

Thanks for all of your work @zoran995
Sorry it took so long to review this.

I just made some tiny changes:

  • removed references to v7 docs (catalog-items.md, catalog-groups.md...)

  • change comment in DataPreviewMap.jsx

- // Choose positron if it's available
+ // Find preview basemap using `terria.previewBaseMapId`
  • added breaking changes notice to CHANGES.md

  • change default basemap assetId to ionAssetId for IonImageryCatalogItemTraits

@nf-s nf-s merged commit fded7ef into TerriaJS:next Mar 22, 2021
@nf-s
Copy link
Contributor

nf-s commented Mar 22, 2021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants