Skip to content

Commit

Permalink
Merge branch 'master' into feat/custom-ee-layers
Browse files Browse the repository at this point in the history
  • Loading branch information
turban committed Mar 17, 2024
2 parents 30f9bf4 + 9fcd872 commit ff46625
Show file tree
Hide file tree
Showing 12 changed files with 279 additions and 84 deletions.
30 changes: 30 additions & 0 deletions .github/workflows/dhis2-verify-app.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
name: 'dhis2: verify (app)'

on:
pull_request:
types: ['opened', 'edited', 'reopened', 'synchronize']

concurrency:
group: ${{ github.workflow}}-${{ github.ref }}

jobs:
verify:
if: "!contains(github.event.head_commit.message, '[skip ci]')"
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with:
node-version: 18.x

- name: Install
run: yarn install --frozen-lockfile

- name: Lint
run: yarn d2-style check

- name: Test
run: yarn test

- name: Build
run: yarn build
15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,18 @@
## [3.9.1](https://github.com/dhis2/maps-gl/compare/v3.9.0...v3.9.1) (2024-03-13)


### Bug Fixes

* use round style for line-join and line-cap ([#566](https://github.com/dhis2/maps-gl/issues/566)) ([476f5af](https://github.com/dhis2/maps-gl/commit/476f5af95f47ad68f50bafd0b52185515714f29b))

# [3.9.0](https://github.com/dhis2/maps-gl/compare/v3.8.6...v3.9.0) (2024-03-05)


### Features

* add class when map is rendered and push-analytics helper files and classes ([#552](https://github.com/dhis2/maps-gl/issues/552)) ([c3830ac](https://github.com/dhis2/maps-gl/commit/c3830acbbfdb699dfa3eb6bc83bda97c2c3379fc))
* add map rendered class and push-analytics config and classes ([#565](https://github.com/dhis2/maps-gl/issues/565)) ([354ba97](https://github.com/dhis2/maps-gl/commit/354ba970512e6513d3467fa0cc8fa9e3f15c0d7a)), closes [#552](https://github.com/dhis2/maps-gl/issues/552)

## [3.8.6](https://github.com/dhis2/maps-gl/compare/v3.8.5...v3.8.6) (2023-10-23)


Expand Down
5 changes: 4 additions & 1 deletion babel.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ module.exports = function (api) {
],
env: {
test: {
plugins: ['@babel/plugin-transform-runtime'],
plugins: [
'@babel/plugin-transform-runtime',
'babel-plugin-transform-import-meta',
],
},
},
}
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@dhis2/maps-gl",
"version": "3.8.6",
"version": "3.9.1",
"description": "A WebGL rendering engine for DHIS2 maps based on Mapbox GL JS.",
"publishConfig": {
"access": "public"
Expand Down Expand Up @@ -44,6 +44,7 @@
"@dhis2/cli-style": "^10.5.1",
"@types/jest": "^29.4.0",
"babel-jest": "^29.5.0",
"babel-plugin-transform-import-meta": "^2.2.1",
"concurrently": "^7.6.0",
"husky": "^8.0.3",
"identity-obj-proxy": "^3.0.0",
Expand Down
68 changes: 68 additions & 0 deletions src/Map.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ import Popup from './ui/Popup'
import Label from './ui/Label'
import './Map.css'

const renderedClass = 'dhis2-map-rendered'
const RENDER_TIMEOUT_DURATION = 500

export class MapGL extends Evented {
// Returns true if the layer type is supported
static hasLayerSupport(type) {
Expand Down Expand Up @@ -44,6 +47,7 @@ export class MapGL extends Evented {

this._mapgl = mapgl
this._glyphs = glyphs
this._renderTimeout = null

// Translate strings
if (locale) {
Expand All @@ -55,12 +59,18 @@ export class MapGL extends Evented {
})
}

mapgl.on('render', this.onRender)
mapgl.on('idle', this.onIdle)
mapgl.on('load', this.onLoad)
mapgl.on('click', this.onClick)
mapgl.on('contextmenu', this.onContextMenu)
mapgl.on('mousemove', this.onMouseMove)
mapgl.on('mouseout', this.onMouseOut)
mapgl.on('error', this.onError)
/* Data and dataloading events are an indication that
* the map is not done yet */
mapgl.on('data', this._clearRenderTimeout)
mapgl.on('dataloading', this._clearRenderTimeout)

this._layers = []
this._controls = {}
Expand Down Expand Up @@ -146,12 +156,16 @@ export class MapGL extends Evented {
remove() {
const mapgl = this._mapgl

mapgl.off('render', this.onRender)
mapgl.off('idle', this.onIdle)
mapgl.off('load', this.onLoad)
mapgl.off('click', this.onClick)
mapgl.off('contextmenu', this.onContextMenu)
mapgl.off('mousemove', this.onMouseMove)
mapgl.off('mouseout', this.onMouseOut)
mapgl.off('error', this.onError)
mapgl.off('data', this._clearRenderTimeout)
mapgl.off('dataloading', this._clearRenderTimeout)

mapgl.remove()

Expand Down Expand Up @@ -258,6 +272,21 @@ export class MapGL extends Evented {
this.getMapGL().getCanvas().style.cursor = feature ? 'pointer' : ''
}

// Remove rendered class if rendering is happening
onRender = () => {
this._removeClass(renderedClass)
this._clearRenderTimeout()
}

// Add rendered class if map is idle
onIdle = () => {
if (this.getLayers().some(layer => layer._isLoading)) {
return
}

this._setRenderTimeout()
}

// Set hover state for features
setHoverState(features) {
// Only set hover state when features are changed
Expand Down Expand Up @@ -453,6 +482,45 @@ export class MapGL extends Evented {

return { type, coordinates, position, feature }
}

_setRenderTimeout() {
// Ensure pending timeout is cleared before setting a new one
this._clearRenderTimeout()
// Make sure the map stay rendered for at least 500ms
this._renderTimeout = setTimeout(() => {
this._addClass(renderedClass)
this._renderTimeout = null
}, RENDER_TIMEOUT_DURATION)
}

_clearRenderTimeout() {
if (this._renderTimeout) {
clearTimeout(this._renderTimeout)
this._renderTimeout = null
}
}

// Add class to map container
_addClass(className) {
if (this._mapgl) {
const { classList } = this._mapgl._container

if (!classList.contains(className)) {
classList.add(className)
}
}
}

// Remove class from map container
_removeClass(className) {
if (this._mapgl) {
const { classList } = this._mapgl._container

if (classList.contains(className)) {
classList.remove(className)
}
}
}
}

export default MapGL
23 changes: 13 additions & 10 deletions src/__tests__/Map.spec.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
import Map from '../Map'

jest.mock('maplibre-gl', () => ({
Map: () => mockMapGL,
Evented: () => {},
Marker: () => {},
Popup: () => {},
AttributionControl: () => {},
NavigationControl: () => {},
FullscreenControl: () => {},
}))
jest.mock('maplibre-gl', () => {
const actualMapLibreGl = jest.requireActual('maplibre-gl')
class MockMap {
constructor() {
Object.assign(this, mockMapGL)
}
}
return {
...actualMapLibreGl,
Map: MockMap,
}
})

jest.mock('../earthengine/ee_worker_loader', () => ({
__esModule: true,
Expand All @@ -22,7 +25,7 @@ describe('DHIS2 Maps-gl Map', () => {

expect(mapgl).not.toBe(undefined)
expect(mapgl).toEqual(mockMapGL)
expect(mapgl.on).toHaveBeenCalledTimes(6)
expect(mapgl.on).toHaveBeenCalledTimes(10)
})

it('should set layer feature hover state', () => {
Expand Down
4 changes: 4 additions & 0 deletions src/layers/BingLayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,16 @@ class BingLayer extends Layer {
}

async addTo(map) {
this._isLoading = true

await this.createSource()

this.createLayer()

await super.addTo(map)

this._isLoading = false

// Make sure overlays are on top
map.orderOverlays()

Expand Down
10 changes: 8 additions & 2 deletions src/layers/EarthEngine.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class EarthEngine extends Layer {
this._map = map

if (map.styleIsLoaded()) {
this._isLoading = true
this.getWorkerInstance()
.then(async worker => {
this.worker = worker
Expand All @@ -35,6 +36,8 @@ class EarthEngine extends Layer {
super.addTo(map)
this.onLoad()

this._isLoading = false

const { preload, data } = this.options

// Get aggregations if not plugin (preload=false) and org units
Expand All @@ -46,7 +49,10 @@ class EarthEngine extends Layer {

resolve()
})
.catch(reject)
.catch(error => {
this._isLoading = false
reject(error)
})
} else {
resolve()
}
Expand All @@ -58,7 +64,7 @@ class EarthEngine extends Layer {
}

// Returns promise resolving a new worker instance
getWorkerInstance = () => {
getWorkerInstance() {
if (!this._workerPromise) {
this._workerPromise = new Promise((resolve, reject) =>
getEarthEngineWorker(this.options.getAuthToken)
Expand Down
3 changes: 3 additions & 0 deletions src/layers/ServerCluster.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ class ServerCluster extends Cluster {

if (!this.tileClusters[tileId]) {
this.tileClusters[tileId] = 'pending'
this._isLoading = true
this.options.load(this.getTileParams(tileId), this.onTileLoad)
}
}
Expand Down Expand Up @@ -196,6 +197,8 @@ class ServerCluster extends Cluster {
if (visibleTiles.includes(tileId)) {
this.updateClusters([tileId])
}

this._isLoading = false
}

getBounds = () => this.options.bounds
Expand Down
31 changes: 22 additions & 9 deletions src/layers/__tests__/EarthEngine.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,6 @@ import EarthEngine from '../EarthEngine'
const urlFormat =
'https://earthengine.googleapis.com/v1alpha/projects/earthengine-legacy/maps/.../tiles/{z}/{x}/{y}'

// Mock out EE Worker - import.meta not supported in jest
jest.mock('../../earthengine/ee_worker_loader', () => ({
__esModule: true,
default: async () => async () =>
new (class EarthEngineWorkerMock {
getTileUrl = async () => urlFormat
})(),
}))

const token = {
access_token: 'abc',
client_id: '123',
Expand Down Expand Up @@ -93,6 +84,28 @@ const options = {
}

describe('EarthEngine', () => {
beforeAll(() => {
/* Ideally the default export from 'earthengine/ee_worker_loader'
* should have been mocked instead of this, but since that function
* returns a proxy from the ComLink library, it was difficult to mock.
* If we ever want to add tests for the `getWorkerInstance` method
* itself we will have to find a way to mock that `getEarthEngineWorker`
* function. */
jest.spyOn(
EarthEngine.prototype,
'getWorkerInstance'
).mockImplementation(async () => {
class EarthEngineWorkerMock {
getTileUrl = async () => urlFormat
}
const worker = new EarthEngineWorkerMock()
return Promise.resolve(worker)
})
})

afterAll(() => {
jest.restoreAllMocks()
})
it('Should initialize', () => {
const layer = new EarthEngine()

Expand Down
8 changes: 8 additions & 0 deletions src/utils/layers.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ export const lineLayer = ({ id, color, width, opacity, source, filter }) => ({
'line-width': widthExpr(width),
'line-opacity': opacity ?? defaults.opacity,
},
layout: {
'line-join': 'round',
'line-cap': 'round',
},
filter: filter || isLine,
})

Expand Down Expand Up @@ -84,6 +88,10 @@ export const outlineLayer = ({
'line-width': widthExpr(width),
'line-opacity': opacity ?? defaults.opacity,
},
layout: {
'line-join': 'round',
'line-cap': 'round',
},
filter: filter || isPolygon,
})

Expand Down
Loading

0 comments on commit ff46625

Please sign in to comment.