From 03ffb738425326dcc18c5ee5c8220b5d2617bbe0 Mon Sep 17 00:00:00 2001 From: rldhont Date: Tue, 13 Feb 2024 16:14:10 +0100 Subject: [PATCH 1/5] Fix JS Permalink : Use precision 6 for extent in 4326 According to these references https://xkcd.com/2170/, https://i.stack.imgur.com/crFH4.png and https://datatracker.ietf.org/doc/html/rfc7946#section-11.2, coordinates in EPSG:4326 do not require a precision greater than 6. --- assets/src/modules/Lizmap.js | 11 ++++++++++- assets/src/modules/Permalink.js | 22 ++++++++++++++++------ tests/end2end/playwright/theme.spec.ts | 8 ++++---- 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/assets/src/modules/Lizmap.js b/assets/src/modules/Lizmap.js index eb96c82378..ae70a9ed80 100644 --- a/assets/src/modules/Lizmap.js +++ b/assets/src/modules/Lizmap.js @@ -179,7 +179,16 @@ export default class Lizmap { } /** - * @param {Array} bounds - Left, bottom, right, top + * The view extent - an array with left, bottom, right, top + * @type {Array} + */ + get extent() { + return this.map.getView().calculateExtent(); + } + + /** + * Setting the view extent + * @param {Array} bounds - Left, bottom, right, top */ set extent(bounds) { this.map.getView().fit(bounds, {nearest: true}); diff --git a/assets/src/modules/Permalink.js b/assets/src/modules/Permalink.js index 0b00435912..7273c032af 100644 --- a/assets/src/modules/Permalink.js +++ b/assets/src/modules/Permalink.js @@ -21,6 +21,7 @@ export default class Permalink { this._ignoreHashChange = false; // Store the build or received hash this._hash = ''; + this._extent4326 = [0, 0, 0, 0, 0]; // Change `checked`, `style` states based on URL fragment if (window.location.hash) { @@ -147,21 +148,29 @@ export default class Permalink { } _runPermalink(setExtent = true) { - this._hash = ''+window.location.hash; - const items = mainLizmap.state.layersAndGroupsCollection.layers.concat(mainLizmap.state.layersAndGroupsCollection.groups); - + if (this._hash === ''+window.location.hash) { + return; + } if (window.location.hash === "") { + this._hash = ''; return; } + this._hash = ''+window.location.hash; + + const items = mainLizmap.state.layersAndGroupsCollection.layers.concat(mainLizmap.state.layersAndGroupsCollection.groups); + const [extent4326, itemsInURL, stylesInURL, opacitiesInURL] = window.location.hash.substring(1).split('|').map(part => part.split(',')); - if (setExtent && extent4326.length === 4) { + if (setExtent + && extent4326.length === 4 + && this._extent4326.filter((v, i) => {return parseFloat(extent4326[i]).toPrecision(6) != v}).length != 0) { const mapExtent = transformExtent( extent4326.map(coord => parseFloat(coord)), 'EPSG:4326', lizMap.map.projection.projCode ); + this._extent4326 = extent4326.map(coord => parseFloat(coord).toPrecision(6)); mainLizmap.extent = mapExtent; } @@ -225,7 +234,7 @@ export default class Permalink { let hash = ''; // BBOX - let bbox = lizMap.map.getExtent().toArray(); + let bbox = mainLizmap.extent; if (lizMap.map.projection.projCode !== 'EPSG:4326') { bbox = transformExtent( bbox, @@ -233,7 +242,8 @@ export default class Permalink { 'EPSG:4326' ); } - hash = bbox.join(); + this._extent4326 = bbox.map(x => x.toFixed(6)); + hash = this._extent4326.join(); // Item's visibility, style and opacity // Only write layer's properties when visible diff --git a/tests/end2end/playwright/theme.spec.ts b/tests/end2end/playwright/theme.spec.ts index 914c2f3030..6b1261cd3f 100644 --- a/tests/end2end/playwright/theme.spec.ts +++ b/tests/end2end/playwright/theme.spec.ts @@ -25,11 +25,11 @@ test.describe('Theme', () => { const url = new URL(page.url()); await expect(url.hash).not.toHaveLength(1); // The decoded hash is - // #3.7308717840938743,43.54038574169922,4.017985172062126,43.679557362551954 + // #3.730872,43.540386,4.017985,43.679557 // |group1,Les%20quartiers // |,style1 // |1,1 - await expect(url.hash).toMatch(/#3.730871\d+,43.540385\d+,4.0179851\d+,43.679557\d+\|/) + await expect(url.hash).toMatch(/#3.7308\d+,43.5403\d+,4.0179\d+,43.6795\d+\|/) await expect(url.hash).toContain('|group1,Les%20quartiers|,style1|1,1') }); @@ -54,11 +54,11 @@ test.describe('Theme', () => { const url = new URL(page.url()); await expect(url.hash).not.toHaveLength(0); // The decoded hash is - // #3.7308717840938743,43.54038574169922,4.017985172062126,43.679557362551954 + // #3.730872,43.540386,4.017985,43.679557 // |Les%20quartiers|style2|1 // |style2 // |1 - await expect(url.hash).toMatch(/#3.730871\d+,43.540385\d+,4.0179851\d+,43.679557\d+\|/) + await expect(url.hash).toMatch(/#3.7308\d+,43.5403\d+,4.0179\d+,43.6795\d+\|/) await expect(url.hash).toContain('|Les%20quartiers|style2|1') }); }); From 1c9d3e87c5cb32ce858c0b06775c1c3a42a24553 Mon Sep 17 00:00:00 2001 From: rldhont Date: Tue, 13 Feb 2024 16:16:25 +0100 Subject: [PATCH 2/5] JS EventDispatcher events : Add not writable target property to event object --- assets/src/utils/EventDispatcher.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/assets/src/utils/EventDispatcher.js b/assets/src/utils/EventDispatcher.js index a78ae3aae6..60612f447d 100644 --- a/assets/src/utils/EventDispatcher.js +++ b/assets/src/utils/EventDispatcher.js @@ -138,6 +138,14 @@ export default class EventDispatcher { // interpretation of what an "id" is writable: false }); + // Add the immutable target property + Object.defineProperty(event, "target", { + value: this, + enumerable: false, + // This could go either way, depending on your + // interpretation of what an "id" is + writable: false + }); // Add it to the stack this._stackEventId.unshift(event['__eventid__']); // Limit the stack to 10 events From 66f80648b66507afa8db034f5846791a51230607 Mon Sep 17 00:00:00 2001 From: rldhont Date: Tue, 13 Feb 2024 16:18:23 +0100 Subject: [PATCH 3/5] Bugfix JS: synchronization beetwen MapState and map --- assets/src/modules/map.js | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/assets/src/modules/map.js b/assets/src/modules/map.js index 9b52364f59..7d3d4f53d7 100644 --- a/assets/src/modules/map.js +++ b/assets/src/modules/map.js @@ -533,7 +533,10 @@ export default class map extends olMap { ); }); - this.on('moveend', this.refreshOL2View); + this.on('moveend', () => { + this.refreshOL2View(); + this._dispatchMapStateChanged(); + }); // Init view this.syncNewOLwithOL2View(); @@ -584,7 +587,14 @@ export default class map extends olMap { } mainLizmap.state.rootMapGroup.addListener( - evt => this.getLayerOrGroupByName(evt.name).setVisible(evt.visibility), + evt => { + const olLayerOrGroup = this.getLayerOrGroupByName(evt.name); + if (olLayerOrGroup) { + olLayerOrGroup.setVisible(evt.visibility); + } else { + console.log('`'+evt.name+'` is not an OpenLayers layer or group!'); + } + }, ['layer.visibility.changed', 'group.visibility.changed'] ); @@ -648,6 +658,29 @@ export default class map extends olMap { if (startupFeatures) { this.setHighlightFeatures(startupFeatures, "geojson"); } + + mainLizmap.state.map.addListener( + evt => { + const view = this.getView(); + const updateCenter = ('center' in evt && view.getCenter().filter((v, i) => {return evt['center'][i] != v}).length != 0); + const updateResolution = ('resolution' in evt && evt['resolution'] !== view.getResolution()); + const updateExtent = ('extent' in evt && view.calculateExtent().filter((v, i) => {return evt['extent'][i] != v}).length != 0); + if (updateCenter && updateResolution) { + view.animate({ + center: evt['center'], + resolution: evt['resolution'], + duration: 50 + }); + } else if (updateCenter) { + view.setCenter(evt['center']); + } else if (updateResolution) { + view.setResolution(evt['resolution']); + } else if (updateExtent) { + view.fit(evt['extent'], {nearest: true}); + } + }, + ['map.state.changed'] + ); } get hasEmptyBaseLayer() { From b6e3f8ba135cda5f88e9f7e7f7602a2d4c7ad098 Mon Sep 17 00:00:00 2001 From: rldhont Date: Tue, 13 Feb 2024 17:20:45 +0100 Subject: [PATCH 4/5] JS MapState fires map.state.ready To be notified that the map is ready a new event `map.state.ready` is fired by MapState. And a new property `isReady` has been added. --- assets/src/modules/state/Map.js | 43 ++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/assets/src/modules/state/Map.js b/assets/src/modules/state/Map.js index bdd914987d..9f0274ffd4 100644 --- a/assets/src/modules/state/Map.js +++ b/assets/src/modules/state/Map.js @@ -22,6 +22,29 @@ const mapStateProperties = { pointScaleDenominator: {type: 'number'}, }; +/** + * Map state ready. + * @event MapState#map.state.ready + * @type {object} + * @property {string} type - map.state.ready + * @property {boolean} ready - true + */ + +/** + * Map state changed + * @event MapState#map.state.changed + * @type {object} + * @property {string} type - map.state.changed + * @property {string} [projection] - the map projection code if it changed + * @property {number[]} [center] - the map center if it changed + * @property {number[]} [size] - the map size if it changed + * @property {number[]} [extent] - the map extent (calculate by the map view) if it changed + * @property {number} [resolution] - the map resolution if it changed + * @property {number} [scaleDenominator] - the map scale denominator if it changed + * @property {number} [pointResolution] - the map resolution (calculate from the center) if it changed + * @property {number} [pointScaleDenominator] - the map scale denominator (calculate from the center) if it changed + */ + /** * Class representing the map state * @class @@ -35,6 +58,7 @@ export class MapState extends EventDispatcher { */ constructor(startupFeatures) { super(); + this._ready = false; // default values this._projection = 'EPSG:3857'; this._center = [0, 0]; @@ -47,7 +71,6 @@ export class MapState extends EventDispatcher { this._startupFeatures = startupFeatures; } - /** * Update the map state * @param {object} evt - the map state changed object @@ -59,6 +82,8 @@ export class MapState extends EventDispatcher { * @param {number} [evt.scaleDenominator] - the map scale denominator * @param {number} [evt.pointResolution] - the map resolution (calculate from the center) * @param {number} [evt.pointScaleDenominator] - the map scale denominator (calculate from the center) + * @fires MapState#map.state.ready + * @fires MapState#map.state.changed */ update(evt) { let updatedProperties = {}; @@ -114,6 +139,14 @@ export class MapState extends EventDispatcher { } // Dispatch event only if something have changed if (Object.getOwnPropertyNames(updatedProperties).length != 0) { + const neededProperties = ['center', 'size', 'extent', 'resolution']; + if (!this._ready && Object.getOwnPropertyNames(updatedProperties).filter(v => neededProperties.includes(v)).length == 4) { + this._ready = true; + this.dispatch({ + type: 'map.state.ready', + ready: true, + }); + } this.dispatch( Object.assign({ type: 'map.state.changed' @@ -122,6 +155,14 @@ export class MapState extends EventDispatcher { } } + /** + * Map is ready + * @type {boolean} + */ + get isReady() { + return this._ready; + } + /** * Map projection code * @type {string} From 2b43af04f0bf8fa6a5a7a6a1e54a35bb8cee177b Mon Sep 17 00:00:00 2001 From: rldhont Date: Wed, 14 Feb 2024 09:24:56 +0100 Subject: [PATCH 5/5] Tests e2e: fix permalink --- tests/end2end/playwright/permalink.spec.ts | 38 ++++++++++++++-------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/tests/end2end/playwright/permalink.spec.ts b/tests/end2end/playwright/permalink.spec.ts index 30e74f2478..f694f5883a 100644 --- a/tests/end2end/playwright/permalink.spec.ts +++ b/tests/end2end/playwright/permalink.spec.ts @@ -306,6 +306,18 @@ test.describe('Permalink', () => { // Close subdock await page.getByRole('button', { name: 'Close' }).click(); + // The check hash url before changing it + const old_url = new URL(page.url()); + await expect(old_url.hash).not.toHaveLength(0); + // The decoded hash is + // #3.0635044037670305,43.401957103265374,4.567657653648659,43.92018105321636 + // |layer_legend_single_symbol,tramway_lines,Group%20as%20layer + // |d%C3%A9faut,categorized, + // |1,1,1,1 + await expect(old_url.hash).toContain('|layer_legend_single_symbol,tramway_lines,Group%20as%20layer|') + await expect(old_url.hash).toContain('|d%C3%A9faut,categorized,|') + await expect(old_url.hash).toContain('|0.6,1,1') + // Test permalink with empty string styles const bbox = '3.0635044037670305,43.401957103265374,4.567657653648659,43.92018105321636' const layers = 'layer_legend_single_symbol,layer_legend_categorized,tramway_lines,Group as layer' @@ -317,20 +329,6 @@ test.describe('Permalink', () => { // Reload to force applying hash with empty string styles await page.reload({ waitUntil: 'networkidle' }); - // waiting for hash updated - await page.waitForTimeout(1000); - // The url has changed - const checked_url = new URL(page.url()); - await expect(checked_url.hash).not.toHaveLength(0); - // The decoded hash is - // #3.0635044037670305,43.401957103265374,4.567657653648659,43.92018105321636 - // |layer_legend_single_symbol,layer_legend_categorized,tramway_lines,Group%20as%20layer - // |d%C3%A9faut,d%C3%A9faut,a_single, - // |1,1,1,1 - await expect(checked_url.hash).toContain('|layer_legend_single_symbol,layer_legend_categorized,tramway_lines,Group%20as%20layer|') - await expect(checked_url.hash).toContain('|d%C3%A9faut,d%C3%A9faut,a_single,|') - await expect(checked_url.hash).toContain('|1,1,1,1') - // No error await expect(page.locator('p.error-msg')).toHaveCount(0); await expect(page.locator('#switcher lizmap-treeview ul li')).not.toHaveCount(0); @@ -355,6 +353,18 @@ test.describe('Permalink', () => { // Close subdock await page.getByRole('button', { name: 'Close' }).click(); + + // The url has changed + const checked_url = new URL(page.url()); + await expect(checked_url.hash).not.toHaveLength(0); + // The decoded hash is + // #3.0635044037670305,43.401957103265374,4.567657653648659,43.92018105321636 + // |layer_legend_single_symbol,layer_legend_categorized,tramway_lines,Group%20as%20layer + // |d%C3%A9faut,d%C3%A9faut,a_single, + // |1,1,1,1 + await expect(checked_url.hash).toContain('|layer_legend_single_symbol,layer_legend_categorized,tramway_lines,Group%20as%20layer|') + await expect(checked_url.hash).toContain('|d%C3%A9faut,d%C3%A9faut,a_single,|') + await expect(checked_url.hash).toContain('|1,1,1,1') }); test('Build permalink and change hash', async ({ page }) => {