simplify OS keys too

theopensystemslab · Aug 19, 2024 · 8aea974 · 8aea974
1 parent de021c3
commit 8aea974
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 55 deletions.
diff --git a/src/components/my-map/index.ts b/src/components/my-map/index.ts
@@ -194,15 +194,24 @@ export class MyMap extends LitElement {
   @property({ type: Boolean })
   disableVectorTiles = false;
 
+  @property({ type: String })
+  osApiKey = import.meta.env.VITE_APP_OS_API_KEY || "";
+
+  /**
+   * @deprecated - please set singular `osApiKey`
+   */
   @property({ type: String })
   osVectorTilesApiKey = import.meta.env.VITE_APP_OS_VECTOR_TILES_API_KEY || "";
 
+  /**
+   * @deprecated - please set singular `osApiKey`
+   */
   @property({ type: String })
   osFeaturesApiKey = import.meta.env.VITE_APP_OS_FEATURES_API_KEY || "";
 
   @property({ type: String })
   osCopyright =
-    `© Crown copyright and database rights ${new Date().getFullYear()} OS (0)100024857`;
+    `© Crown copyright and database rights ${new Date().getFullYear()} OS <your account number>`;
 
   @property({ type: String })
   osProxyEndpoint = "";
@@ -266,30 +275,33 @@ export class MyMap extends LitElement {
   firstUpdated() {
     const target = this.renderRoot.querySelector(`#${this.id}`) as HTMLElement;
 
+    const isUsingOS = Boolean(this.osApiKey || this.osProxyEndpoint);
+
     const basemapLayers: BaseLayer[] = [];
     let osVectorTileBasemap: VectorTileLayer | undefined,
       osRasterBasemap: TileLayer<XYZ> | undefined,
       mapboxSatelliteBasemap: VectorLayer<VectorSource> | undefined;
 
-    if (this.basemap === "OSVectorTile") {
+    if (this.basemap === "OSVectorTile" && isUsingOS) {
       osVectorTileBasemap = makeOsVectorTileBasemap(
-        this.osVectorTilesApiKey,
+        this.osApiKey,
         this.osProxyEndpoint,
         this.osCopyright,
       );
-      if (osVectorTileBasemap) basemapLayers.push(osVectorTileBasemap);
-    } else if (this.basemap === "OSRaster") {
+      basemapLayers.push(osVectorTileBasemap);
+    } else if (this.basemap === "OSRaster" && isUsingOS) {
       osRasterBasemap = makeOSRasterBasemap(
-        this.osVectorTilesApiKey,
+        this.osApiKey,
         this.osProxyEndpoint,
         this.osCopyright,
       );
-      if (osRasterBasemap) basemapLayers.push(osRasterBasemap);
-    } else if (this.basemap === "MapboxSatellite") {
-      mapboxSatelliteBasemap = makeMapboxSatelliteBasemap(
-        this.mapboxAccessToken,
-      );
-      if (mapboxSatelliteBasemap) basemapLayers.push(mapboxSatelliteBasemap);
+      basemapLayers.push(osRasterBasemap);
+    } else if (
+      this.basemap === "MapboxSatellite" &&
+      Boolean(this.mapboxAccessToken)
+    ) {
+      mapboxSatelliteBasemap = makeMapboxSatelliteBasemap();
+      basemapLayers.push(mapboxSatelliteBasemap);
     } else if (this.basemap === "OSM" || basemapLayers.length === 0) {
       // Fallback to OpenStreetMap if we've failed to make any of the above layers, or if it's set directly
       const osmBasemap = makeDefaultTileLayer();
@@ -601,13 +613,11 @@ export class MyMap extends LitElement {
     }
 
     // OS Features API & click-to-select interactions
-    const isUsingOSFeaturesAPI =
-      this.showFeaturesAtPoint &&
-      Boolean(this.osFeaturesApiKey || this.osProxyEndpoint);
-    if (isUsingOSFeaturesAPI) {
+    const isUsingOSFeatures = isUsingOS && this.showFeaturesAtPoint;
+    if (isUsingOSFeatures) {
       getFeaturesAtPoint(
         centerCoordinate,
-        this.osFeaturesApiKey,
+        this.osApiKey,
         this.osProxyEndpoint,
         false,
       );
@@ -616,7 +626,7 @@ export class MyMap extends LitElement {
         map.on("singleclick", (e) => {
           getFeaturesAtPoint(
             e.coordinate,
-            this.osFeaturesApiKey,
+            this.osApiKey,
             this.osProxyEndpoint,
             true,
           );

diff --git a/src/components/my-map/layers.test.ts b/src/components/my-map/layers.test.ts
@@ -19,11 +19,11 @@ describe("Basemap layer loading", () => {
   });
 
   it("loads OSVectorTile basemap by default and requests layers directly from OS when an API key is provided", async () => {
-    const apiKey = process.env.VITE_APP_OS_VECTOR_TILES_API_KEY;
+    const apiKey = process.env.VITE_APP_OS_API_KEY;
     await setupMap(`
       <my-map 
         id="map-vitest" 
-        osVectorTileApiKey=${apiKey}
+        osApiKey=${apiKey}
       />`);
     const vectorBasemap = window.olMap
       ?.getAllLayers()
@@ -66,12 +66,12 @@ describe("Basemap layer loading", () => {
   });
 
   it("loads OSRaster basemap when an OS API key is provided", async () => {
-    const apiKey = process.env.VITE_APP_OS_VECTOR_TILES_API_KEY;
+    const apiKey = process.env.VITE_APP_OS_API_KEY;
     await setupMap(`
       <my-map 
         id="map-vitest" 
         basemap="OSRaster"
-        osVectorTileApiKey=${apiKey}
+        osApiKey=${apiKey}
       />`);
     const rasterBasemap = window.olMap
       ?.getAllLayers()
@@ -108,7 +108,7 @@ describe("Basemap layer loading", () => {
 
   it("fallsback to an OSM basemap when an OS basemap is specified without an OS API key or proxy endpoint", async () => {
     await setupMap(`
-      <my-map id="map-vitest" basemap="OSVectorTile" osVectorTilesApiKey=${undefined} />`);
+      <my-map id="map-vitest" basemap="OSVectorTile" osApiKey="" />`);
     const osmBasemap = window.olMap
       ?.getAllLayers()
       .find((layer) => layer.get("name") === "osmBasemap");

diff --git a/src/components/my-map/layers.ts b/src/components/my-map/layers.ts
@@ -26,19 +26,12 @@ export function makeDefaultTileLayer(): TileLayer<OSM> {
   return layer;
 }
 
-export function makeMapboxSatelliteBasemap(
-  accessToken: string,
-): VectorLayer<VectorSource> | undefined {
-  const isUsingMapbox = Boolean(accessToken);
-  if (!isUsingMapbox) return;
-
-  const satelliteAttribution =
-    '<a href="https://www.mapbox.com/about/maps/" target="_blank" rel="noopener noreferrer">© Mapbox</a> <a href="http://www.openstreetmap.org/about/" target="_blank" rel="noopener noreferrer">© OpenStreetMap</a> <a href="https://labs.mapbox.com/contribute/#/-74@site/src/10" target="_blank" rel="noopener noreferrer"><strong>Improve this map</strong></a>';
-
+export function makeMapboxSatelliteBasemap(): VectorLayer<VectorSource> {
   // Layer is empty besides attribution, style is "applied" after instantiating map in index.ts
   const layer = new VectorLayer({
     source: new VectorSource({
-      attributions: satelliteAttribution,
+      attributions:
+        '<a href="https://www.mapbox.com/about/maps/" target="_blank" rel="noopener noreferrer">© Mapbox</a> <a href="http://www.openstreetmap.org/about/" target="_blank" rel="noopener noreferrer">© OpenStreetMap</a> <a href="https://labs.mapbox.com/contribute/#/-74@site/src/10" target="_blank" rel="noopener noreferrer"><strong>Improve this map</strong></a>',
     }),
   });
   layer.set("name", "mapboxSatelliteBasemap"); // @todo debug why not actually set??
@@ -49,10 +42,7 @@ export function makeOSRasterBasemap(
   apiKey: string,
   proxyEndpoint: string,
   copyright: string,
-): TileLayer<XYZ> | undefined {
-  const isUsingOS = Boolean(apiKey || proxyEndpoint);
-  if (!isUsingOS) return;
-
+): TileLayer<XYZ> {
   const tileServiceURL = getServiceURL({
     service: "xyz",
     apiKey,
@@ -74,10 +64,7 @@ export function makeOsVectorTileBasemap(
   apiKey: string,
   proxyEndpoint: string,
   copyright: string,
-): VectorTileLayer | undefined {
-  const isUsingOS = Boolean(apiKey || proxyEndpoint);
-  if (!isUsingOS) return;
-
+): VectorTileLayer {
   const vectorTileServiceUrl = getServiceURL({
     service: "vectorTile",
     apiKey,
@@ -99,7 +86,6 @@ export function makeOsVectorTileBasemap(
     proxyEndpoint,
     params: { srs: "3857" },
   });
-
   // ref https://github.com/openlayers/ol-mapbox-style#usage-example
   fetch(vectorTileStyleUrl)
     .then((response) => response.json())

diff --git a/src/components/my-map/main.test.ts b/src/components/my-map/main.test.ts
@@ -15,16 +15,13 @@ declare global {
 }
 
 test("olMap is added to the global window for tests", async () => {
-  await setupMap(`<my-map id="map-vitest" disableVectorTiles />`);
+  await setupMap(`<my-map id="map-vitest" />`);
   expect(window.olMap).toBeTruthy();
   expect(window.olMap).toBeInstanceOf(Map);
 });
 
 describe("MyMap on initial render with OSM basemap", async () => {
-  beforeEach(
-    () => setupMap('<my-map id="map-vitest" disableVectorTiles />'),
-    2500,
-  );
+  beforeEach(() => setupMap('<my-map id="map-vitest" basemap="OSM" />'), 2500);
 
   it("should render a custom element with a shadow root", () => {
     const map = document.body.querySelector("my-map");
@@ -37,22 +34,22 @@ describe("MyMap on initial render with OSM basemap", async () => {
 
 describe("Keyboard navigation of map container, controls and attribution links", () => {
   it("map container should be keyboard navigable by default", async () => {
-    await setupMap(`<my-map id="map-vitest" disableVectorTiles />`);
+    await setupMap(`<my-map id="map-vitest" />`);
     const map = getShadowRoot("my-map")?.getElementById("map-vitest");
     expect(map).toBeTruthy;
     expect(map?.getAttribute("tabindex")).toEqual("0");
   });
 
   it("should omit map container from tab order if not interactive", async () => {
-    await setupMap(`<my-map id="map-vitest" disableVectorTiles staticMode />`);
+    await setupMap(`<my-map id="map-vitest" staticMode />`);
     const map = getShadowRoot("my-map")?.getElementById("map-vitest");
     expect(map).toBeTruthy;
     expect(map?.getAttribute("tabindex")).toEqual("-1");
   });
 
   it("should keep map container in tab order if attributions are collapsed", async () => {
     await setupMap(
-      `<my-map id="map-vitest" disableVectorTiles staticMode collapseAttributions />`,
+      `<my-map id="map-vitest" staticMode collapseAttributions />`,
     );
     const map = getShadowRoot("my-map")?.getElementById("map-vitest");
     expect(map).toBeTruthy;
@@ -77,7 +74,7 @@ describe("Snap points loading behaviour", () => {
       id="map-vitest" 
       zoom=${ZOOM_OUTWITH_RANGE} 
       drawMode 
-      osVectorTileApiKey=${process.env.VITE_APP_OS_VECTOR_TILES_API_KEY}
+      osApiKey=${process.env.VITE_APP_OS_API_KEY}
     />`);
     const pointsLayer = window.olMap
       ?.getAllLayers()
@@ -93,7 +90,7 @@ describe("Snap points loading behaviour", () => {
       id="map-vitest" 
       zoom=${ZOOM_WITHIN_RANGE} 
       drawMode 
-      osVectorTileApiKey=${process.env.VITE_APP_OS_VECTOR_TILES_API_KEY}
+      osApiKey=${process.env.VITE_APP_OS_API_KEY}
     />`);
     expect(getSnapSpy).toHaveBeenCalledOnce();
   });
@@ -103,7 +100,7 @@ describe("Snap points loading behaviour", () => {
       id="map-vitest" 
       zoom=${ZOOM_OUTWITH_RANGE} 
       drawMode 
-      osVectorTileApiKey=${process.env.VITE_APP_OS_VECTOR_TILES_API_KEY}
+      osApiKey=${process.env.VITE_APP_OS_API_KEY}
     />`);
     window.olMap?.getView().setZoom(ZOOM_WITHIN_RANGE);
     window.olMap?.dispatchEvent("loadend");
@@ -116,7 +113,7 @@ describe("Snap points loading behaviour", () => {
       id="map-vitest" 
       zoom=${ZOOM_WITHIN_RANGE}
       drawMode 
-      osVectorTileApiKey=${process.env.VITE_APP_OS_VECTOR_TILES_API_KEY}
+      osApiKey=${process.env.VITE_APP_OS_API_KEY}
     />`);
     const pointsLayer = window.olMap
       ?.getAllLayers()
@@ -138,7 +135,7 @@ describe("Snap points loading behaviour", () => {
       id="map-vitest" 
       zoom=${ZOOM_WITHIN_RANGE}
       drawMode 
-      osVectorTileApiKey=${process.env.VITE_APP_OS_VECTOR_TILES_API_KEY}
+      osApiKey=${process.env.VITE_APP_OS_API_KEY}
     />`);
     expect(getSnapSpy).toHaveBeenCalledTimes(1);
     window.olMap?.dispatchEvent("moveend");