From 80d2aa85878c2fbdb921e87f7ad2e20df5904d5d Mon Sep 17 00:00:00 2001 From: James Milner Date: Sat, 26 Oct 2024 14:24:21 +0100 Subject: [PATCH] fix: handle multiple layers in TerraDrawOpenLayersAdapter --- development/src/index.ts | 14 ++- src/adapters/common/base.adapter.ts | 2 +- src/adapters/openlayers.adapter.spec.ts | 23 ++++- src/adapters/openlayers.adapter.ts | 122 ++++++++++++++++++++++-- 4 files changed, 144 insertions(+), 17 deletions(-) diff --git a/development/src/index.ts b/development/src/index.ts index 6ec1fd69..21fa4927 100644 --- a/development/src/index.ts +++ b/development/src/index.ts @@ -380,6 +380,17 @@ const example = { new TileLayer({ source: new OSM(), }), + // If you want to experiment with multiple layers uncomment this + // new VectorLayer({ + // background: '#1a2b39', + // source: new VectorSource({ + // url: 'https://openlayers.org/data/vector/ecoregions.json', + // format: new GeoJSON(), + // }), + // style: { + // 'fill-color': ['string', ['get', 'COLOR'], '#eee'], + // }, + // }) ], target: this.generateId(Libraries.OpenLayers), view: new View({ @@ -389,7 +400,8 @@ const example = { controls: [], }); - map.once("postrender", () => { + // All layers must be rendered before we can start drawing + map.once("rendercomplete", () => { const draw = new TerraDraw({ adapter: new TerraDrawOpenLayersAdapter({ lib: { diff --git a/src/adapters/common/base.adapter.ts b/src/adapters/common/base.adapter.ts index a92ca69e..3b40c8fc 100644 --- a/src/adapters/common/base.adapter.ts +++ b/src/adapters/common/base.adapter.ts @@ -136,7 +136,7 @@ export abstract class TerraDrawBaseAdapter implements TerraDrawAdapter { return this._coordinatePrecision; } - private getAdapterListeners() { + protected getAdapterListeners() { return [ new AdapterListener({ name: "pointerdown", diff --git a/src/adapters/openlayers.adapter.spec.ts b/src/adapters/openlayers.adapter.spec.ts index 6614557a..4e56b38a 100644 --- a/src/adapters/openlayers.adapter.spec.ts +++ b/src/adapters/openlayers.adapter.spec.ts @@ -1,3 +1,6 @@ +/** + * @jest-environment jsdom + */ import { InjectableOL, TerraDrawOpenLayersAdapter } from "./openlayers.adapter"; import Map from "ol/Map"; import { Pixel } from "ol/pixel"; @@ -35,14 +38,20 @@ const createMockOLMap = (multipleCanvases?: boolean) => { }, addEventListener: jest.fn(), removeEventListener: jest.fn(), + compareDocumentPosition: jest + .fn() + .mockImplementationOnce(() => 2) + .mockImplementationOnce(() => 4), }, ]; if (multipleCanvases) { canvases.push({ ...canvases[0] }); + canvases.push({ ...canvases[0] }); } return { + once: jest.fn((_, callback) => callback()), getCoordinateFromPixel: jest.fn(() => [0, 0] as Pixel), getPixelFromCoordinate: jest.fn(() => [0, 0] as Coordinate), getViewport: jest.fn(() => ({ @@ -50,6 +59,12 @@ const createMockOLMap = (multipleCanvases?: boolean) => { querySelectorAll: jest.fn(() => canvases), })), addLayer: jest.fn(), + getLayerGroup: jest.fn(() => ({ + getLayers: jest.fn(() => ({ + on: jest.fn((_, cb) => cb()), + un: jest.fn((_, cb) => cb()), + })), + })), getInteractions: jest.fn(() => [new DragPan(), new DoubleClickZoom()]), } as unknown as Map; }; @@ -273,7 +288,7 @@ describe("TerraDrawOpenLayersAdapter", () => { expect(result.getBoundingClientRect).toBeDefined(); }); - it("throws error if there are multiple canvases", () => { + it("returns the map element correctly when there are multiple canvases", () => { const adapter = new TerraDrawOpenLayersAdapter({ map: createMockOLMap(true) as Map, lib: { @@ -284,9 +299,8 @@ describe("TerraDrawOpenLayersAdapter", () => { } as unknown as InjectableOL, }); - expect(() => adapter.getMapEventElement()).toThrow( - "Terra Draw currently only supports 1 canvas with OpenLayers", - ); + const result = adapter.getMapEventElement(); + expect(result.getBoundingClientRect).toBeDefined(); }); }); @@ -843,7 +857,6 @@ describe("TerraDrawOpenLayersAdapter", () => { }); it("performs clear on unregister", () => { - adapter.register(MockCallbacks()); adapter.unregister(); expect(clear).toHaveBeenCalledTimes(1); diff --git a/src/adapters/openlayers.adapter.ts b/src/adapters/openlayers.adapter.ts index 4512d6c7..1e570688 100644 --- a/src/adapters/openlayers.adapter.ts +++ b/src/adapters/openlayers.adapter.ts @@ -37,6 +37,7 @@ export class TerraDrawOpenLayersAdapter extends TerraDrawBaseAdapter { config: { map: Map; lib: InjectableOL; + zIndex?: number; } & BaseAdapterConfig, ) { super(config); @@ -64,6 +65,7 @@ export class TerraDrawOpenLayersAdapter extends TerraDrawBaseAdapter { const vectorLayer = new this._lib.VectorLayer({ source: vectorSource as unknown as VectorSource, style: (feature) => this.getStyles(feature, this.stylingFunction()), + zIndex: config.zIndex ?? 100000, }); this._map.addLayer(vectorLayer); @@ -181,6 +183,31 @@ export class TerraDrawOpenLayersAdapter extends TerraDrawBaseAdapter { this._vectorSource.removeFeature(deleted); } + /** + * Sorts an array of DOM elements based on their order in the document, from earliest to latest. + * @param elements - An array of `HTMLElement` objects to be sorted. + * @returns A new array of `HTMLElement` objects sorted by their document order. + */ + private sortElementsByDOMOrder(elements: HTMLElement[]) { + // Sort the elements based on their DOM position + return elements.sort((a, b) => { + const position = a.compareDocumentPosition(b); + + // If a comes before b in the DOM + if (position & Node.DOCUMENT_POSITION_FOLLOWING) { + return -1; + } + + // If a comes after b in the DOM + if (position & Node.DOCUMENT_POSITION_PRECEDING) { + return 1; + } + + // If they are the same element + return 0; + }); + } + /** * Returns the longitude and latitude coordinates from a given PointerEvent on the map. * @param event The PointerEvent or MouseEvent containing the screen coordinates of the pointer. @@ -201,15 +228,16 @@ export class TerraDrawOpenLayersAdapter extends TerraDrawBaseAdapter { * @returns The HTMLElement representing the map container. */ public getMapEventElement() { - const canvases = this._container.querySelectorAll("canvas"); - - if (canvases.length > 1) { - throw Error( - "Terra Draw currently only supports 1 canvas with OpenLayers", - ); - } - - return canvases[0]; + // Each VectorLayer has a canvas element that is used to render the features, it orders + // these in the order they are added to the map. The last canvas is the one that is on top + // so we need to add the event listeners to this canvas so that the events are captured. + const canvases = Array.from(this._container.querySelectorAll("canvas")); + const sortedCanvases = this.sortElementsByDOMOrder( + canvases, + ) as HTMLCanvasElement[]; + const topCanvas = sortedCanvases[sortedCanvases.length - 1]; + + return topCanvas; } /** @@ -311,8 +339,71 @@ export class TerraDrawOpenLayersAdapter extends TerraDrawBaseAdapter { } } + private registeredLayerHandlers: + | undefined + | { addLayers: () => void; removeLayers: () => void }; + public register(callbacks: TerraDrawCallbacks) { super.register(callbacks); + + // We need to handle the complex case in OpenLayers where adding and removing layers + // can change the canvas ordering preventing the event listeners from working correctly + if (!this.registeredLayerHandlers) { + const layerGroup = this._map.getLayerGroup(); + const layers = layerGroup.getLayers(); + + const removeListeners = () => { + this._listeners.forEach((listener) => { + listener.unregister(); + }); + }; + + const addListeners = () => { + this._listeners = this.getAdapterListeners(); + + this._listeners.forEach((listener) => { + listener.register(); + }); + }; + + this.registeredLayerHandlers = { + addLayers: () => { + removeListeners(); + this._map.once("rendercomplete", () => { + if (this._currentModeCallbacks) { + addListeners(); + } + }); + }, + removeLayers: () => { + removeListeners(); + this._map.once("rendercomplete", () => { + if (this._currentModeCallbacks) { + addListeners(); + } + }); + }, + }; + + layers.on("add", () => { + removeListeners(); + this._map.once("rendercomplete", () => { + if (this._currentModeCallbacks) { + addListeners(); + } + }); + }); + + layers.on("remove", () => { + removeListeners(); + this._map.once("rendercomplete", () => { + if (this._currentModeCallbacks) { + addListeners(); + } + }); + }); + } + this._currentModeCallbacks && this._currentModeCallbacks.onReady && this._currentModeCallbacks.onReady(); @@ -323,7 +414,18 @@ export class TerraDrawOpenLayersAdapter extends TerraDrawBaseAdapter { } public unregister(): void { - // TODO: It seems this shouldn't be necessary as extends BaseAdapter which as this method + if (this.registeredLayerHandlers) { + const layerGroup = this._map.getLayerGroup(); + if (layerGroup) { + const layers = layerGroup.getLayers(); + + if (layers) { + layers.un("add", this.registeredLayerHandlers.addLayers); + layers.un("remove", this.registeredLayerHandlers.removeLayers); + } + } + } + return super.unregister(); } }