Skip to content

Commit

Permalink
fix: handle multiple layers in TerraDrawOpenLayersAdapter
Browse files Browse the repository at this point in the history
  • Loading branch information
JamesLMilner committed Oct 26, 2024
1 parent b40f970 commit 80d2aa8
Show file tree
Hide file tree
Showing 4 changed files with 144 additions and 17 deletions.
14 changes: 13 additions & 1 deletion development/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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: {
Expand Down
2 changes: 1 addition & 1 deletion src/adapters/common/base.adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ export abstract class TerraDrawBaseAdapter implements TerraDrawAdapter {
return this._coordinatePrecision;
}

private getAdapterListeners() {
protected getAdapterListeners() {
return [
new AdapterListener<BasePointerListener>({
name: "pointerdown",
Expand Down
23 changes: 18 additions & 5 deletions src/adapters/openlayers.adapter.spec.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
/**
* @jest-environment jsdom
*/
import { InjectableOL, TerraDrawOpenLayersAdapter } from "./openlayers.adapter";
import Map from "ol/Map";
import { Pixel } from "ol/pixel";
Expand Down Expand Up @@ -35,21 +38,33 @@ 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(() => ({
setAttribute: jest.fn(),
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;
};
Expand Down Expand Up @@ -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: {
Expand All @@ -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();
});
});

Expand Down Expand Up @@ -843,7 +857,6 @@ describe("TerraDrawOpenLayersAdapter", () => {
});

it("performs clear on unregister", () => {
adapter.register(MockCallbacks());
adapter.unregister();

expect(clear).toHaveBeenCalledTimes(1);
Expand Down
122 changes: 112 additions & 10 deletions src/adapters/openlayers.adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export class TerraDrawOpenLayersAdapter extends TerraDrawBaseAdapter {
config: {
map: Map;
lib: InjectableOL;
zIndex?: number;
} & BaseAdapterConfig,
) {
super(config);
Expand Down Expand Up @@ -64,6 +65,7 @@ export class TerraDrawOpenLayersAdapter extends TerraDrawBaseAdapter {
const vectorLayer = new this._lib.VectorLayer({
source: vectorSource as unknown as VectorSource<never>,
style: (feature) => this.getStyles(feature, this.stylingFunction()),
zIndex: config.zIndex ?? 100000,
});

this._map.addLayer(vectorLayer);
Expand Down Expand Up @@ -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.
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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();
Expand All @@ -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();
}
}

0 comments on commit 80d2aa8

Please sign in to comment.