From d70a3235fbfd4d79eba0e8876659a6a017eedd32 Mon Sep 17 00:00:00 2001 From: Sebastien Corbin Date: Tue, 6 Aug 2019 12:21:47 +0200 Subject: [PATCH 1/7] Configurable hash (#8596) --- src/ui/hash.js | 41 ++++++++++++-- src/ui/map.js | 6 ++- test/unit/ui/hash.test.js | 109 +++++++++++++++++++++++++++++++++++++- 3 files changed, 147 insertions(+), 9 deletions(-) diff --git a/src/ui/hash.js b/src/ui/hash.js index 938accae4fa..086b0286bcd 100644 --- a/src/ui/hash.js +++ b/src/ui/hash.js @@ -15,9 +15,12 @@ import type Map from './map'; class Hash { _map: Map; _updateHash: () => ?TimeoutID; + _hashName: ?string; - constructor() { + constructor(hashName: string | boolean) { + this._hashName = (typeof hashName === 'string' && hashName) || null; bindAll([ + '_getCurrentHash', '_onHashChange', '_updateHash' ], this); @@ -67,18 +70,46 @@ class Hash { if (mapFeedback) { // new map feedback site has some constraints that don't allow // us to use the same hash format as we do for the Map hash option. - hash += `#/${lng}/${lat}/${zoom}`; + hash += `/${lng}/${lat}/${zoom}`; } else { - hash += `#${zoom}/${lat}/${lng}`; + hash += `${zoom}/${lat}/${lng}`; } if (bearing || pitch) hash += (`/${Math.round(bearing * 10) / 10}`); if (pitch) hash += (`/${Math.round(pitch)}`); - return hash; + + if (this._hashName && !mapFeedback) { + let found = false; + const parts = window.location.hash.slice(1).split('&').map(part => { + const key = part.split('=')[0]; + if (key === this._hashName) { + found = true; + return `${key}=${hash}`; + } + return part; + }).filter(a => a); + if (!found) { + parts.push(`${this._hashName || ''}=${hash}`); + } + return `#${parts.join('&')}`; + } + + return `#${hash}`; + } + + _getCurrentHash() { + const hash = window.location.hash.replace('#', ''); + if (this._hashName) { + const keyval = hash.split('&').map( + part => part.split('=') + ).find(part => part[0] === this._hashName); + return (keyval ? keyval[1] || '' : '').split('/'); + } + return hash.split('/'); } _onHashChange() { - const loc = window.location.hash.replace('#', '').split('/'); + const loc = this._getCurrentHash(); if (loc.length >= 3) { this._map.jumpTo({ center: [+loc[2], +loc[1]], diff --git a/src/ui/map.js b/src/ui/map.js index fc0f4e176dc..5fd65adbe3f 100755 --- a/src/ui/map.js +++ b/src/ui/map.js @@ -64,7 +64,7 @@ type IControl = { /* eslint-enable no-use-before-define */ type MapOptions = { - hash?: boolean, + hash?: boolean | string, interactive?: boolean, container: HTMLElement | string, bearingSnap?: number, @@ -173,6 +173,8 @@ const defaultOptions = { * * @param {boolean} [options.hash=false] If `true`, the map's position (zoom, center latitude, center longitude, bearing, and pitch) will be synced with the hash fragment of the page's URL. * For example, `http://path/to/my/page.html#2.59/39.26/53.07/-24.1/60`. + * If a string is provided, it will used as the name for a parameter-styled hash, for example: + * `http://path/to/my/page.html#map=2.59/39.26/53.07/-24.1/60&foo=bar` * @param {boolean} [options.interactive=true] If `false`, no mouse, touch, or keyboard listeners will be attached to the map, so it will not respond to interaction. * @param {number} [options.bearingSnap=7] The threshold, measured in degrees, that determines when the map's * bearing will snap to north. For example, with a `bearingSnap` of 7, if the user rotates @@ -378,7 +380,7 @@ class Map extends Camera { bindHandlers(this, options); - this._hash = options.hash && (new Hash()).addTo(this); + this._hash = options.hash && (new Hash(options.hash)).addTo(this); // don't set position from options if set through hash if (!this._hash || !this._hash._onHashChange()) { this.jumpTo({ diff --git a/test/unit/ui/hash.test.js b/test/unit/ui/hash.test.js index d3cf7306e58..59baa4d20f2 100644 --- a/test/unit/ui/hash.test.js +++ b/test/unit/ui/hash.test.js @@ -4,8 +4,8 @@ import window from '../../../src/util/window'; import { createMap as globalCreateMap } from '../../util'; test('hash', (t) => { - function createHash() { - const hash = new Hash(); + function createHash(name) { + const hash = new Hash(name); hash._updateHash = hash._updateHashUnthrottled.bind(hash); return hash; } @@ -100,6 +100,70 @@ test('hash', (t) => { t.end(); }); + t.test('#_onHashChange named', (t) => { + const map = createMap(t); + const hash = createHash('map') + .addTo(map); + + window.location.hash = '#map=10/3.00/-1.00&foo=bar'; + + hash._onHashChange(); + + t.equal(map.getCenter().lng, -1); + t.equal(map.getCenter().lat, 3); + t.equal(map.getZoom(), 10); + t.equal(map.getBearing(), 0); + t.equal(map.getPitch(), 0); + + window.location.hash = ''; + + t.end(); + }); + + t.test('#_getCurrentHash', (t) => { + const map = createMap(t); + const hash = createHash() + .addTo(map); + + window.location.hash = '#10/3.00/-1.00'; + + const currentHash = hash._getCurrentHash(); + + t.equal(currentHash[0], '10'); + t.equal(currentHash[1], '3.00'); + t.equal(currentHash[2], '-1.00'); + + window.location.hash = ''; + + t.end(); + }); + + t.test('#_getCurrentHash named', (t) => { + const map = createMap(t); + const hash = createHash('map') + .addTo(map); + + window.location.hash = '#map=10/3.00/-1.00&foo=bar'; + + let currentHash = hash._getCurrentHash(); + + t.equal(currentHash[0], '10'); + t.equal(currentHash[1], '3.00'); + t.equal(currentHash[2], '-1.00'); + + window.location.hash = '#baz&map=10/3.00/-1.00'; + + currentHash = hash._getCurrentHash(); + + t.equal(currentHash[0], '10'); + t.equal(currentHash[1], '3.00'); + t.equal(currentHash[2], '-1.00'); + + window.location.hash = ''; + + t.end(); + }); + t.test('#_updateHash', (t) => { function getHash() { return window.location.hash.split('/'); @@ -145,6 +209,47 @@ test('hash', (t) => { t.equal(newHash[3], '135'); t.equal(newHash[4], '60'); + window.location.hash = ''; + + t.end(); + }); + + t.test('#_updateHash named', (t) => { + const map = createMap(t); + createHash('map') + .addTo(map); + + t.notok(window.location.hash); + + map.setZoom(3); + map.setCenter([1.0, 2.0]); + + t.ok(window.location.hash); + + t.equal(window.location.hash, '#map=3/2/1'); + + map.setPitch(60); + + t.equal(window.location.hash, '#map=3/2/1/0/60'); + + map.setBearing(135); + + t.equal(window.location.hash, '#map=3/2/1/135/60'); + + window.location.hash += '&foo=bar'; + + map.setZoom(7); + + t.equal(window.location.hash, '#map=7/2/1/135/60&foo=bar'); + + window.location.hash = '#baz&map=7/2/1/135/60&foo=bar'; + + map.setCenter([2.0, 1.0]); + + t.equal(window.location.hash, '#baz&map=7/1/2/135/60&foo=bar'); + + window.location.hash = ''; + t.end(); }); From 15c82138a71026e89fc95cb64530dec2becc852f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Corbin?= Date: Tue, 6 Aug 2019 15:03:22 +0200 Subject: [PATCH 2/7] Fix typo in doc --- src/ui/map.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ui/map.js b/src/ui/map.js index 5fd65adbe3f..6bb7147bd61 100755 --- a/src/ui/map.js +++ b/src/ui/map.js @@ -173,8 +173,7 @@ const defaultOptions = { * * @param {boolean} [options.hash=false] If `true`, the map's position (zoom, center latitude, center longitude, bearing, and pitch) will be synced with the hash fragment of the page's URL. * For example, `http://path/to/my/page.html#2.59/39.26/53.07/-24.1/60`. - * If a string is provided, it will used as the name for a parameter-styled hash, for example: - * `http://path/to/my/page.html#map=2.59/39.26/53.07/-24.1/60&foo=bar` + * If a string is provided, it will be used as the name for a parameter-styled hash, e.g. `http://path/to/my/page.html#map=2.59/39.26/53.07/-24.1/60&foo=bar`. * @param {boolean} [options.interactive=true] If `false`, no mouse, touch, or keyboard listeners will be attached to the map, so it will not respond to interaction. * @param {number} [options.bearingSnap=7] The threshold, measured in degrees, that determines when the map's * bearing will snap to north. For example, with a `bearingSnap` of 7, if the user rotates From c2f953fc9bcdb45611c05327a8ccf48f46c40b18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Corbin?= Date: Wed, 7 Aug 2019 11:59:01 +0200 Subject: [PATCH 3/7] Update jsDoc --- src/ui/map.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/map.js b/src/ui/map.js index 6bb7147bd61..8a09d8bbc7e 100755 --- a/src/ui/map.js +++ b/src/ui/map.js @@ -171,7 +171,7 @@ const defaultOptions = { * Tilesets hosted with Mapbox can be style-optimized if you append `?optimize=true` to the end of your style URL, like `mapbox://styles/mapbox/streets-v9?optimize=true`. * Learn more about style-optimized vector tiles in our [API documentation](https://www.mapbox.com/api-documentation/maps/#retrieve-tiles). * - * @param {boolean} [options.hash=false] If `true`, the map's position (zoom, center latitude, center longitude, bearing, and pitch) will be synced with the hash fragment of the page's URL. + * @param {(boolean\|string)} [options.hash=false] If `true`, the map's position (zoom, center latitude, center longitude, bearing, and pitch) will be synced with the hash fragment of the page's URL. * For example, `http://path/to/my/page.html#2.59/39.26/53.07/-24.1/60`. * If a string is provided, it will be used as the name for a parameter-styled hash, e.g. `http://path/to/my/page.html#map=2.59/39.26/53.07/-24.1/60&foo=bar`. * @param {boolean} [options.interactive=true] If `false`, no mouse, touch, or keyboard listeners will be attached to the map, so it will not respond to interaction. From e713cfa8e2c41961810161693561af9e686721d4 Mon Sep 17 00:00:00 2001 From: Sebastien Corbin Date: Mon, 12 Aug 2019 18:11:59 +0200 Subject: [PATCH 4/7] Reviewed per @asheemmamoowala --- src/ui/hash.js | 4 ++-- src/ui/map.js | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/ui/hash.js b/src/ui/hash.js index 086b0286bcd..c2a9c8f0b69 100644 --- a/src/ui/hash.js +++ b/src/ui/hash.js @@ -17,8 +17,8 @@ class Hash { _updateHash: () => ?TimeoutID; _hashName: ?string; - constructor(hashName: string | boolean) { - this._hashName = (typeof hashName === 'string' && hashName) || null; + constructor(hashName: ?string) { + this._hashName = hashName; bindAll([ '_getCurrentHash', '_onHashChange', diff --git a/src/ui/map.js b/src/ui/map.js index 8a09d8bbc7e..4d8576f4435 100755 --- a/src/ui/map.js +++ b/src/ui/map.js @@ -171,7 +171,7 @@ const defaultOptions = { * Tilesets hosted with Mapbox can be style-optimized if you append `?optimize=true` to the end of your style URL, like `mapbox://styles/mapbox/streets-v9?optimize=true`. * Learn more about style-optimized vector tiles in our [API documentation](https://www.mapbox.com/api-documentation/maps/#retrieve-tiles). * - * @param {(boolean\|string)} [options.hash=false] If `true`, the map's position (zoom, center latitude, center longitude, bearing, and pitch) will be synced with the hash fragment of the page's URL. + * @param {(boolean|string)} [options.hash=false] If `true`, the map's position (zoom, center latitude, center longitude, bearing, and pitch) will be synced with the hash fragment of the page's URL. * For example, `http://path/to/my/page.html#2.59/39.26/53.07/-24.1/60`. * If a string is provided, it will be used as the name for a parameter-styled hash, e.g. `http://path/to/my/page.html#map=2.59/39.26/53.07/-24.1/60&foo=bar`. * @param {boolean} [options.interactive=true] If `false`, no mouse, touch, or keyboard listeners will be attached to the map, so it will not respond to interaction. @@ -379,7 +379,8 @@ class Map extends Camera { bindHandlers(this, options); - this._hash = options.hash && (new Hash(options.hash)).addTo(this); + const hashName = (typeof options.hash === 'string' && options.hash) || undefined; + this._hash = options.hash && (new Hash(hashName)).addTo(this); // don't set position from options if set through hash if (!this._hash || !this._hash._onHashChange()) { this.jumpTo({ From 533dd54edcde35c462598f4a7439b2a02e967fe3 Mon Sep 17 00:00:00 2001 From: Sebastien Corbin Date: Wed, 28 Aug 2019 13:37:22 +0200 Subject: [PATCH 5/7] Ensure hash is well formed --- src/ui/hash.js | 2 +- test/unit/ui/hash.test.js | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/ui/hash.js b/src/ui/hash.js index c2a9c8f0b69..056d07fbf60 100644 --- a/src/ui/hash.js +++ b/src/ui/hash.js @@ -110,7 +110,7 @@ class Hash { _onHashChange() { const loc = this._getCurrentHash(); - if (loc.length >= 3) { + if (loc.length >= 3 && !loc.some(v => isNaN(+v))) { this._map.jumpTo({ center: [+loc[2], +loc[1]], zoom: +loc[0], diff --git a/test/unit/ui/hash.test.js b/test/unit/ui/hash.test.js index 59baa4d20f2..e0ad06d5ae3 100644 --- a/test/unit/ui/hash.test.js +++ b/test/unit/ui/hash.test.js @@ -67,6 +67,14 @@ test('hash', (t) => { t.equal(map.getBearing(), 30); t.equal(map.getPitch(), 60); + window.location.hash = '#4/wronlgy/formed/hash'; + + t.false(hash._onHashChange()); + + window.location.hash = '#map=10/3.00/-1.00&foo=bar'; + + t.false(hash._onHashChange()); + window.location.hash = ''; t.end(); @@ -115,6 +123,18 @@ test('hash', (t) => { t.equal(map.getBearing(), 0); t.equal(map.getPitch(), 0); + window.location.hash = '#map&foo=bar'; + + t.false(hash._onHashChange()); + + window.location.hash = '#map=4/5/baz&foo=bar'; + + t.false(hash._onHashChange()); + + window.location.hash = '#5/1.00/0.50/30/60'; + + t.false(hash._onHashChange()); + window.location.hash = ''; t.end(); From b361f3a91c409cc39f090a1bb8d1302c3956635c Mon Sep 17 00:00:00 2001 From: Sebastien Corbin Date: Wed, 4 Sep 2019 11:40:58 +0200 Subject: [PATCH 6/7] Review per @ryanhamley --- src/ui/hash.js | 3 ++- src/ui/map.js | 4 +++- test/unit/ui/hash.test.js | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/ui/hash.js b/src/ui/hash.js index 056d07fbf60..ec47c44981c 100644 --- a/src/ui/hash.js +++ b/src/ui/hash.js @@ -100,6 +100,7 @@ class Hash { _getCurrentHash() { const hash = window.location.hash.replace('#', ''); if (this._hashName) { + // Split the parameter-styled hash into parts and find the value we need const keyval = hash.split('&').map( part => part.split('=') ).find(part => part[0] === this._hashName); @@ -110,7 +111,7 @@ class Hash { _onHashChange() { const loc = this._getCurrentHash(); - if (loc.length >= 3 && !loc.some(v => isNaN(+v))) { + if (loc.length >= 3 && !loc.some(v => isNaN(v))) { this._map.jumpTo({ center: [+loc[2], +loc[1]], zoom: +loc[0], diff --git a/src/ui/map.js b/src/ui/map.js index 4d8576f4435..22266c9e6a2 100755 --- a/src/ui/map.js +++ b/src/ui/map.js @@ -173,7 +173,9 @@ const defaultOptions = { * * @param {(boolean|string)} [options.hash=false] If `true`, the map's position (zoom, center latitude, center longitude, bearing, and pitch) will be synced with the hash fragment of the page's URL. * For example, `http://path/to/my/page.html#2.59/39.26/53.07/-24.1/60`. - * If a string is provided, it will be used as the name for a parameter-styled hash, e.g. `http://path/to/my/page.html#map=2.59/39.26/53.07/-24.1/60&foo=bar`. + * An additional string may optionally be provided to indicate a parameter-styled hash, + * e.g. http://path/to/my/page.html#map=2.59/39.26/53.07/-24.1/60&foo=bar, where foo + * is a custom parameter and bar is an arbitrary hash distinct from the map hash. * @param {boolean} [options.interactive=true] If `false`, no mouse, touch, or keyboard listeners will be attached to the map, so it will not respond to interaction. * @param {number} [options.bearingSnap=7] The threshold, measured in degrees, that determines when the map's * bearing will snap to north. For example, with a `bearingSnap` of 7, if the user rotates diff --git a/test/unit/ui/hash.test.js b/test/unit/ui/hash.test.js index e0ad06d5ae3..8a7fd08f5bf 100644 --- a/test/unit/ui/hash.test.js +++ b/test/unit/ui/hash.test.js @@ -67,7 +67,7 @@ test('hash', (t) => { t.equal(map.getBearing(), 30); t.equal(map.getPitch(), 60); - window.location.hash = '#4/wronlgy/formed/hash'; + window.location.hash = '#4/wrongly/formed/hash'; t.false(hash._onHashChange()); From 5a1a7109ae1ef21071ac290ee42714d66cf02f04 Mon Sep 17 00:00:00 2001 From: Sebastien Corbin Date: Wed, 16 Oct 2019 10:13:35 +0200 Subject: [PATCH 7/7] Sanitize hash name input, fix flow and a bit of doc --- src/ui/hash.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/ui/hash.js b/src/ui/hash.js index ec47c44981c..282cb6e6617 100644 --- a/src/ui/hash.js +++ b/src/ui/hash.js @@ -18,7 +18,7 @@ class Hash { _hashName: ?string; constructor(hashName: ?string) { - this._hashName = hashName; + this._hashName = hashName && encodeURIComponent(hashName); bindAll([ '_getCurrentHash', '_onHashChange', @@ -78,18 +78,19 @@ class Hash { if (bearing || pitch) hash += (`/${Math.round(bearing * 10) / 10}`); if (pitch) hash += (`/${Math.round(pitch)}`); - if (this._hashName && !mapFeedback) { + if (this._hashName) { + const hashName = this._hashName; let found = false; const parts = window.location.hash.slice(1).split('&').map(part => { const key = part.split('=')[0]; - if (key === this._hashName) { + if (key === hashName) { found = true; return `${key}=${hash}`; } return part; }).filter(a => a); if (!found) { - parts.push(`${this._hashName || ''}=${hash}`); + parts.push(`${hashName}=${hash}`); } return `#${parts.join('&')}`; } @@ -98,6 +99,7 @@ class Hash { } _getCurrentHash() { + // Get the current hash from location, stripped from its number sign const hash = window.location.hash.replace('#', ''); if (this._hashName) { // Split the parameter-styled hash into parts and find the value we need