Add May Places, Divisions and Buildings themes. [#25, #41] #56

bdon · 2024-05-29T15:00:21Z

change interactiveLayers state to visibleThemes since a single theme has more than one MapLibre layer (one for each type)
hook up places, divisions, buildings themes to ThemeSelector
remove non-Overture basemap
Add components to encapsulate ThemeSource and ThemeTypeLayer objects - use conventions around layer IDs and URLs
change color palette to x-ray - one color per theme
add divisions_division label layer from Divisions theme that's always visible to assist navigation
add glyphs using MapLibre demo repo on GitHub Pages for label layer
use non-compact static attribution linking to OpenStreetMap and Overture Maps
add theme and type to inspector properties

Will add comments inline.

Major thing missing is a data-driven way to determine the types (layers) to build out of a theme. This could change from release from release; for now this is hardcoded to May's.

…rtureMaps#41] * change interactiveLayers state to visibleThemes since a single theme has more than one MapLibre layer (one for each type) * hook up places, divisions, buildings themes to ThemeSelector * remove non-Overture basemap * Add components to encapsulate ThemeSource and ThemeTypeLayer objects - use conventions around layer IDs and URLs * change color palette to x-ray - one color per theme * add divisions_division label layer from Divisions theme that's always visible to assist navigation * add glyphs using MapLibre demo repo on GitHub Pages for label layer * use non-compact static attribution linking to OpenStreetMap and Overture Maps * add theme and type to inspector properties

bdon · 2024-05-29T15:01:19Z

site/src/ThemeSelector.jsx

@@ -3,22 +3,21 @@ import { useState, useEffect } from "react";
 import LayerIcon from "./icons/icon-layers.svg?react";
 import "./ThemeSelector.css"; 

-function ThemeSelector({ interactiveLayers }) {
+function ThemeSelector({ visibleThemes }) {

  const [places, setPlaces] = useState(true);


Would prefer to have a data-driven way to generate list of layers/types out of themes - could change between releases

I 100% agree. Seems like the release notes at https://labs.overturemaps.org/data/releases.json are a good starting point, but need to be augmented to include this information. Avoiding hardcoding this information will also make it possible to toggle between different versions of the data more easily.

Not sure how we can both have dynamic layers and consistent styling though. The ThemeTypeLayer includes both the layers/types and styling. I don't think the latter should live in any release manifest.

Could we include the entire schema repo as a submodule or use the YAMLs as part of the build or dynamically?

The YAML can give us 1) all themes, 2) all types for each theme, and 3) all geometry types in each theme.
We would have to assign colors, etc in some external way

Yes, I love the idea of using the schema directly. I had a similar thought for how we could augment the inspector panel with documentation tooltips: #40.

Since Overture releases are versioned, it may make more sense to include the schema as a sidecar for each release. On initialization, we could download the schema, initialize all the layers using the geometry types, and build indices for documentation, etc. This approach would decouple the io-site release with Overture data releases, and would enable side-by-side comparison of different versions.

bdon · 2024-05-29T15:01:44Z

site/src/Map.jsx

          <GeolocateControl />
+          <AttributionControl customAttribution='<a href="https://openstreetmap.org/copyright" target="_blank">© OpenStreetMap contributors</a>, <a href="https://overturemaps.org" target="_blank">Overture Maps Foundation</a>' />


Unfortunately customAttribution cannot be reactive.

Let's add an AI to discuss attribution during the next TF meeting. I think we'll need guidance on the exact text for each layer.

Open questions I have are:

Any other ODBL licensed data we need to include explicitly in the attribution?

What does attribution look like for Places?

Is there a better link for Overture attribution? Seems like a more robust release manifest which includes all the sources for each theme would be more appropriate. Might not exist now, but worth starting the discussion.

We should model appropriate attribution behavior before go-live, agreed!

bdon · 2024-05-29T15:02:52Z

site/src/Map.jsx

+            }}
+          />
+
+          {visibleThemes.includes("divisions") ? (


This was changed from using layout.visibility because that would override any of our defined layout properties, and using JSX lets react-map-gl perform the style diff. Any reason we would prefer changing visibility instead of removing the layer?

The only consideration I can think of is performance. Perhaps the overhead of adding/remove the layers is greater than keeping them loaded but invisible?

Once we get the land cover layers loaded in we can re-examine whether any performance optimizations are required from a rendering perspective.

The base/land_cover layers are in now the PR respecting the cartography.max_zoom / min_zoom attributes.

Removing/adding the layer does make HTTP requests but those will be cached by all browsers but Firefox. The same is true of modifying visibility at the Style level.

bdon · 2024-05-29T15:03:21Z

site/src/Map.jsx

+          <ThemeSource name="places" url={PMTILES_URL} />
+          <ThemeSource name="divisions" url={PMTILES_URL} />
+
+          <Layer


This layer shows the division type of the divisions theme, and always appears on top so the map is navigable.

site/src/Map.jsx

charliemcgrady

A couple of non-blocking style comments:

The labels for boundaries are awesome. Not sure if there's an easy way to order them based on population, etc. to ensure more "important" labels are more prominent.
The x-ray theme might not work well for color blind folks. We may want a more divergent color palette so layers are more distinguishable from each other.
The map worked well for me, but
I'm super curious to see how land cover perform. Definitely worth doing performance testing on lower end mobile devices and laptops to see which layers should be on by default.

site/src/Map.jsx

charliemcgrady · 2024-05-30T04:18:51Z

site/src/Map.jsx

+  return (
+    <>
+      {point ? (
+        <Layer


Are there any major benefits to defining the layers in React like this vs. a JSON file?

My concern which styling the map in imperative code is it removes the ability to style the map using Maputnik. @Bonkles mentioned we may have some cartographer resourcing, so it would be good to understand if this will inhibit their workflow.

We might be able to pull out JSON to put into Maputnik, but any cartography changes that happen in Maputnik must be ported back into the code manually, so I feel having them work inside the code, JSX or JS etc, is more sustainable.

site/src/Map.jsx

charliemcgrady · 2024-05-30T04:33:35Z

site/src/Map.jsx

+      setMapEntity({
+        theme: feature.source,
+        type: feature.sourceLayer,
+        ...feature.properties,
+      });


Now that we have more layers, we can't assume there will only be one active map entity at a time.

We discussed two options:

A Maputnik style popup where you require two clicks to active the inspector panel: (a) click the map and display popup with every feature selected, (b) click the feature you want to inspect.

A separate radio button control for which layer should display in the inspector panel on click.

After some thought, I think #1 is a good starting point as it negates the need for an additional control and enables quick inspection of all features at a given lat/lng. Any thoughts/opinions on this?

No need to add as part of PR - I'm planning on adding this functionality once this PR lands.

is necessary even with 2) because building_part and division, even if viewed in isolation, will always have multiple overlapping entities at the same position.

charliemcgrady · 2024-05-30T04:39:00Z

site/src/Map.jsx

          <GeolocateControl />
+          <AttributionControl customAttribution='<a href="https://openstreetmap.org/copyright" target="_blank">© OpenStreetMap contributors</a>, <a href="https://overturemaps.org" target="_blank">Overture Maps Foundation</a>' />


Let's add an AI to discuss attribution during the next TF meeting. I think we'll need guidance on the exact text for each layer.

Open questions I have are:

Any other ODBL licensed data we need to include explicitly in the attribution?

What does attribution look like for Places?

Is there a better link for Overture attribution? Seems like a more robust release manifest which includes all the sources for each theme would be more appropriate. Might not exist now, but worth starting the discussion.

charliemcgrady · 2024-05-30T04:47:58Z

site/src/Map.jsx

+ThemeTypeLayer.propTypes = {
+  theme: PropTypes.string.isRequired,
+  type: PropTypes.string.isRequired,
+  color: PropTypes.string.isRequired,
+  point: PropTypes.bool,
+  line: PropTypes.bool,
+  polygon: PropTypes.bool,
+  extrusion: PropTypes.bool,
+};


This interface looks good for now, but may become restrictive overtime. For example, we may want to dynamically color places based on type (or display some icons based on presets like the iD editor).

charliemcgrady · 2024-05-30T04:50:24Z

site/src/Map.jsx

+            }}
+          />
+
+          {visibleThemes.includes("divisions") ? (


The only consideration I can think of is performance. Perhaps the overhead of adding/remove the layers is greater than keeping them loaded but invisible?

Once we get the land cover layers loaded in we can re-examine whether any performance optimizations are required from a rendering perspective.

charliemcgrady · 2024-05-30T04:53:18Z

site/src/Map.jsx

+  url: PropTypes.string.isRequired,
+};
+
+const ThemeTypeLayer = ({


My preference would be to refactor the layers into separate components per theme if we stick with the JSX definitions.

…X for toggling visibility.

bdon · 2024-05-30T13:14:17Z

Updated with transportation layer as well. There are a couple problems with the PMTiles on each theme (holes, missing tags, etc) but those are decoupled from the frontend.

Bonkles

As we've said, nothing blocking that I see, but we can continue to evolve the styling and UX around this!

charliemcgrady

Agreed with @Bonkles - this is a great start. I really like the direction of more declarative layers based in React.

bdon commented May 29, 2024

View reviewed changes

site/src/Map.jsx Show resolved Hide resolved

Add base theme; use 3d extrusions; filter layers by geometry type.

bd4962b

charliemcgrady reviewed May 30, 2024

View reviewed changes

add transportation theme; use visibility property style instead of JS…

97d1d19

…X for toggling visibility.

Bonkles self-requested a review May 30, 2024 15:14

Bonkles approved these changes May 30, 2024

View reviewed changes

charliemcgrady approved these changes Jun 3, 2024

View reviewed changes

Bonkles merged commit 962f95f into OvertureMaps:main Jun 4, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add May Places, Divisions and Buildings themes. [#25, #41] #56

Add May Places, Divisions and Buildings themes. [#25, #41] #56

bdon commented May 29, 2024

bdon May 29, 2024

charliemcgrady May 30, 2024

bdon May 30, 2024

charliemcgrady Jun 3, 2024

bdon May 29, 2024

charliemcgrady May 30, 2024

Bonkles May 30, 2024

bdon May 29, 2024

charliemcgrady May 30, 2024

bdon May 30, 2024

bdon May 29, 2024

charliemcgrady left a comment •

edited

Loading

charliemcgrady May 30, 2024

bdon May 30, 2024

charliemcgrady May 30, 2024

bdon May 30, 2024

charliemcgrady May 30, 2024

charliemcgrady May 30, 2024

charliemcgrady May 30, 2024

charliemcgrady May 30, 2024

bdon commented May 30, 2024

Bonkles left a comment

charliemcgrady left a comment

		<GeolocateControl />
		<AttributionControl customAttribution='<a href="https://openstreetmap.org/copyright" target="_blank">© OpenStreetMap contributors</a>, <a href="https://overturemaps.org" target="_blank">Overture Maps Foundation</a>' />

Add May Places, Divisions and Buildings themes. [#25, #41] #56

Add May Places, Divisions and Buildings themes. [#25, #41] #56

Conversation

bdon commented May 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charliemcgrady left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bdon commented May 30, 2024

Bonkles left a comment

Choose a reason for hiding this comment

charliemcgrady left a comment

Choose a reason for hiding this comment

charliemcgrady left a comment •

edited

Loading