From 4bd9639afbf511d3910fb282c51fefb9c6b9e897 Mon Sep 17 00:00:00 2001 From: Aidan Date: Wed, 21 Jul 2021 12:44:49 -0600 Subject: [PATCH 01/17] addClassName method can be called before the popup is on map --- src/ui/popup.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ui/popup.js b/src/ui/popup.js index 3ba3f94c2b6..a4a50b6b960 100644 --- a/src/ui/popup.js +++ b/src/ui/popup.js @@ -443,7 +443,9 @@ export default class Popup extends Evented { addClassName(className: string) { if (this._container) { this._container.classList.add(className); - } + } else if (this.options.className && this.options.className !== ``) { + this.options.className += ` ${className}`; + } else { this.options.className = className; } } /** From 4b98354537a0e589651d16e8fd419397171c099d Mon Sep 17 00:00:00 2001 From: Aidan Date: Wed, 21 Jul 2021 13:25:25 -0600 Subject: [PATCH 02/17] Added tests --- test/unit/ui/popup.test.js | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/test/unit/ui/popup.test.js b/test/unit/ui/popup.test.js index be48373c1a2..9dd22a31d97 100644 --- a/test/unit/ui/popup.test.js +++ b/test/unit/ui/popup.test.js @@ -610,6 +610,42 @@ test('Popup adds classes from className option, methods for class manipulations t.end(); }); +test('Popup.addClassName adds classes when called before adding popup to map', (t) => { + const map = createMap(t); + const popup = new Popup(); + popup.addClassName('some'); + popup.addClassName('classes'); + + popup.setText('Test') + .setLngLat([0, 0]) + .addTo(map); + + const popupContainer = popup.getElement(); + t.ok(popupContainer.classList.contains('some')); + t.ok(popupContainer.classList.contains('classes')); + t.end(); +}); +test('Popup className option and addClassName both add classes', (t) => { + const map = createMap(t); + const popup = new Popup({className: 'some classes'}); + popup.addClassName('even'); + popup.addClassName('more'); + + popup.setText('Test') + .setLngLat([0, 0]) + .addTo(map); + + popup.addClassName('one-more'); + + const popupContainer = popup.getElement(); + t.ok(popupContainer.classList.contains('some')); + t.ok(popupContainer.classList.contains('classes')); + t.ok(popupContainer.classList.contains('even')); + t.ok(popupContainer.classList.contains('more')); + t.ok(popupContainer.classList.contains('one-more')); + t.end(); +}); + test('Cursor-tracked popup disappears on mouseout', (t) => { const map = createMap(t); From 1e2f063f1ec66cfe101515d50c451ac1e66db7ef Mon Sep 17 00:00:00 2001 From: Aidan Date: Wed, 21 Jul 2021 14:29:45 -0600 Subject: [PATCH 03/17] Updating incorrect examples --- src/ui/popup.js | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/ui/popup.js b/src/ui/popup.js index a4a50b6b960..ed3d9f93f1a 100644 --- a/src/ui/popup.js +++ b/src/ui/popup.js @@ -454,8 +454,11 @@ export default class Popup extends Evented { * @param {string} className Non-empty string with CSS class name to remove from popup container * * @example - * let popup = new mapboxgl.Popup() - * popup.removeClassName('some-class') + * const popup = new mapboxgl.Popup({className: 'some classes'}) + .setText('Popup') + .setLngLat([0, 0]) + .addTo(map); + * popup.removeClassName('some') */ removeClassName(className: string) { if (this._container) { @@ -484,7 +487,12 @@ export default class Popup extends Evented { * * @example * let popup = new mapboxgl.Popup() - * popup.toggleClassName('toggleClass') + * const popup = new mapboxgl.Popup({className: 'highlighted'}) + .setText('Popup') + .setLngLat([0, 0]) + .addTo(map); + * popup.toggleClassName('highlighted') + * popup.toggleClassName('highlighted') */ toggleClassName(className: string) { if (this._container) { From c5235982ece87779e7ed57f877169279ec719b01 Mon Sep 17 00:00:00 2001 From: Aidan Date: Wed, 21 Jul 2021 14:36:35 -0600 Subject: [PATCH 04/17] Added console.warn error messages for incorrect function use --- src/ui/popup.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ui/popup.js b/src/ui/popup.js index ed3d9f93f1a..fa323c47f52 100644 --- a/src/ui/popup.js +++ b/src/ui/popup.js @@ -463,6 +463,8 @@ export default class Popup extends Evented { removeClassName(className: string) { if (this._container) { this._container.classList.remove(className); + } else { + console.warn(`removeClassName() has no effect before the popup is added to a map.`); } } @@ -497,6 +499,8 @@ export default class Popup extends Evented { toggleClassName(className: string) { if (this._container) { return this._container.classList.toggle(className); + } else { + console.warn(`toggleClassName() has no effect before the popup is added to a map.`); } } From 051be6b8e68bd8834b225a7b695801f457c56db7 Mon Sep 17 00:00:00 2001 From: Arindam Bose Date: Tue, 20 Jul 2021 16:15:05 -0700 Subject: [PATCH 05/17] Pin chrome to version 91 (#10887) * Pin to chrome version 91 * Pin chrome version for test-browser --- .circleci/config.yml | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 789aefb7e45..2a3d5c0b323 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -209,7 +209,8 @@ jobs: steps: - attach_workspace: at: ~/ - - browser-tools/install-chrome + - browser-tools/install-chrome: + chrome-version: 91.0.4472.164 - run: yarn run test-render - store_test_results: path: test/integration/render-tests @@ -221,7 +222,8 @@ jobs: steps: - attach_workspace: at: ~/ - - browser-tools/install-chrome + - browser-tools/install-chrome: + chrome-version: 91.0.4472.164 - run: yarn run test-render-prod - store_test_results: path: test/integration/render-tests @@ -233,7 +235,8 @@ jobs: steps: - attach_workspace: at: ~/ - - browser-tools/install-chrome + - browser-tools/install-chrome: + chrome-version: 91.0.4472.164 - run: yarn run test-query - store_test_results: path: test/integration/query-tests @@ -260,7 +263,8 @@ jobs: steps: - attach_workspace: at: ~/ - - browser-tools/install-chrome + - browser-tools/install-chrome: + chrome-version: 91.0.4472.164 - run: name: Collect performance stats command: node bench/gl-stats.js @@ -274,7 +278,8 @@ jobs: steps: - attach_workspace: at: ~/ - - browser-tools/install-browser-tools + - browser-tools/install-browser-tools: + chrome-version: 91.0.4472.164 - run: yarn run build-dev - run: yarn run build-token - run: From 646002a17ebb26f8d6980dfb8703adea67a04edb Mon Sep 17 00:00:00 2001 From: Aidan H Date: Wed, 21 Jul 2021 15:19:54 -0600 Subject: [PATCH 06/17] Update src/ui/popup.js Co-authored-by: Ricky Reusser --- src/ui/popup.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/popup.js b/src/ui/popup.js index fa323c47f52..cb91427710a 100644 --- a/src/ui/popup.js +++ b/src/ui/popup.js @@ -443,7 +443,7 @@ export default class Popup extends Evented { addClassName(className: string) { if (this._container) { this._container.classList.add(className); - } else if (this.options.className && this.options.className !== ``) { + } else if (this.options.className) { this.options.className += ` ${className}`; } else { this.options.className = className; } } From bca4cfba0b5924891c26996f72297944e2493872 Mon Sep 17 00:00:00 2001 From: Aidan Date: Wed, 21 Jul 2021 15:21:43 -0600 Subject: [PATCH 07/17] Fixed asterisks for jsdoc --- src/ui/popup.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ui/popup.js b/src/ui/popup.js index cb91427710a..7ea6674163b 100644 --- a/src/ui/popup.js +++ b/src/ui/popup.js @@ -455,9 +455,9 @@ export default class Popup extends Evented { * * @example * const popup = new mapboxgl.Popup({className: 'some classes'}) - .setText('Popup') - .setLngLat([0, 0]) - .addTo(map); + * .setText('Popup') + * .setLngLat([0, 0]) + * .addTo(map); * popup.removeClassName('some') */ removeClassName(className: string) { @@ -490,9 +490,9 @@ export default class Popup extends Evented { * @example * let popup = new mapboxgl.Popup() * const popup = new mapboxgl.Popup({className: 'highlighted'}) - .setText('Popup') - .setLngLat([0, 0]) - .addTo(map); + * .setText('Popup') + * .setLngLat([0, 0]) + * .addTo(map); * popup.toggleClassName('highlighted') * popup.toggleClassName('highlighted') */ From aba074ba7b1baeaa6c9239fda8304816e9c2d6ea Mon Sep 17 00:00:00 2001 From: Aidan Date: Wed, 21 Jul 2021 15:23:57 -0600 Subject: [PATCH 08/17] Removing else with early return --- src/ui/popup.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ui/popup.js b/src/ui/popup.js index 7ea6674163b..d1d31bfeb52 100644 --- a/src/ui/popup.js +++ b/src/ui/popup.js @@ -499,9 +499,8 @@ export default class Popup extends Evented { toggleClassName(className: string) { if (this._container) { return this._container.classList.toggle(className); - } else { - console.warn(`toggleClassName() has no effect before the popup is added to a map.`); } + console.warn(`toggleClassName() has no effect before the popup is added to a map.`); } _createCloseButton() { From 774d41e9195e6303b0199370016be9e97ced3aa8 Mon Sep 17 00:00:00 2001 From: Heather Stenson Date: Wed, 21 Jul 2021 16:37:42 -0700 Subject: [PATCH 09/17] Updates to `flyTo` docs (#10890) * updates to flyto * no trailing spaces --- src/ui/camera.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ui/camera.js b/src/ui/camera.js index f468bccb58d..e142d6d1e32 100644 --- a/src/ui/camera.js +++ b/src/ui/camera.js @@ -1200,11 +1200,11 @@ class Camera extends Evented { /** * Changes any combination of center, zoom, bearing, and pitch, animating the transition along a curve that * evokes flight. The animation seamlessly incorporates zooming and panning to help - * the user maintain her bearings even after traversing a great distance. + * the user maintain their bearings even after traversing a great distance. * - * Note: The animation will be skipped, and this will behave equivalently to `jumpTo` - * if the user has the `reduced motion` accessibility feature enabled in their operating system, - * unless 'options' includes `essential: true`. + * If a user has the `reduced motion` accessibility feature enabled in their + * operating system, the animation will be skipped and this will behave + * equivalently to `jumpTo`, unless 'options' includes `essential: true`. * * @memberof Map# * @param {Object} options Options describing the destination and animation of the transition. @@ -1216,9 +1216,9 @@ class Camera extends Evented { * value selected by participants in the user study discussed in * [van Wijk (2003)](https://www.win.tue.nl/~vanwijk/zoompan.pdf). A value of * `Math.pow(6, 0.25)` would be equivalent to the root mean squared average velocity. A - * value of 1 would produce a circular motion. + * value of 1 would produce a circular motion. If `options.minZoom` is specified, this option will be ignored. * @param {number} [options.minZoom] The zero-based zoom level at the peak of the flight path. If - * `options.curve` is specified, this option is ignored. + * this option is specified, `options.curve` will be ignored. * @param {number} [options.speed=1.2] The average speed of the animation defined in relation to * `options.curve`. A speed of 1.2 means that the map appears to move along the flight path * by 1.2 times `options.curve` screenfuls every second. A _screenful_ is the map's visible span. From f67bb879ef432d4877b4bccc66c5f0a675ca41f7 Mon Sep 17 00:00:00 2001 From: Aidan Date: Thu, 22 Jul 2021 15:22:44 -0600 Subject: [PATCH 10/17] Added _classList Set to Popup --- src/ui/popup.js | 35 ++++++++++++++++++----------------- test/unit/ui/popup.test.js | 36 ++++++++++++++++++++++++++++++++++-- 2 files changed, 52 insertions(+), 19 deletions(-) diff --git a/src/ui/popup.js b/src/ui/popup.js index d1d31bfeb52..d89c55aaf77 100644 --- a/src/ui/popup.js +++ b/src/ui/popup.js @@ -107,11 +107,13 @@ export default class Popup extends Evented { _lngLat: LngLat; _trackPointer: boolean; _pos: ?Point; + _classList: Set; constructor(options: PopupOptions) { super(); this.options = extend(Object.create(defaultOptions), options); bindAll(['_update', '_onClose', 'remove', '_onMouseMove', '_onMouseUp', '_onDrag'], this); + this._classList = new Set(options && options.className ? options.className.split(` `) : []); } /** @@ -435,17 +437,18 @@ export default class Popup extends Evented { * Adds a CSS class to the popup container element. * * @param {string} className Non-empty string with CSS class name to add to popup container + * @returns {Popup} `this` * * @example * let popup = new mapboxgl.Popup() * popup.addClassName('some-class') */ addClassName(className: string) { + this._classList.add(className); if (this._container) { this._container.classList.add(className); - } else if (this.options.className) { - this.options.className += ` ${className}`; - } else { this.options.className = className; } + } + return this; } /** @@ -453,19 +456,17 @@ export default class Popup extends Evented { * * @param {string} className Non-empty string with CSS class name to remove from popup container * + * @returns {Popup} `this` * @example * const popup = new mapboxgl.Popup({className: 'some classes'}) - * .setText('Popup') - * .setLngLat([0, 0]) - * .addTo(map); * popup.removeClassName('some') */ removeClassName(className: string) { + this._classList.delete(className); if (this._container) { this._container.classList.remove(className); - } else { - console.warn(`removeClassName() has no effect before the popup is added to a map.`); } + return this; } /** @@ -489,18 +490,18 @@ export default class Popup extends Evented { * * @example * let popup = new mapboxgl.Popup() - * const popup = new mapboxgl.Popup({className: 'highlighted'}) - * .setText('Popup') - * .setLngLat([0, 0]) - * .addTo(map); - * popup.toggleClassName('highlighted') * popup.toggleClassName('highlighted') */ toggleClassName(className: string) { if (this._container) { - return this._container.classList.toggle(className); + this._container.classList.toggle(className); + } + if (this._classList.has(className)) { + this._classList.delete(className); + return false; } - console.warn(`toggleClassName() has no effect before the popup is added to a map.`); + this._classList.add(className); + return true; } _createCloseButton() { @@ -534,8 +535,8 @@ export default class Popup extends Evented { this._container = DOM.create('div', 'mapboxgl-popup', this._map.getContainer()); this._tip = DOM.create('div', 'mapboxgl-popup-tip', this._container); this._container.appendChild(this._content); - if (this.options.className) { - this.options.className.split(' ').forEach(name => + if (this._classList) { + this._classList.forEach(name => this._container.classList.add(name)); } diff --git a/test/unit/ui/popup.test.js b/test/unit/ui/popup.test.js index 9dd22a31d97..8cb5c7708f6 100644 --- a/test/unit/ui/popup.test.js +++ b/test/unit/ui/popup.test.js @@ -628,8 +628,8 @@ test('Popup.addClassName adds classes when called before adding popup to map', ( test('Popup className option and addClassName both add classes', (t) => { const map = createMap(t); const popup = new Popup({className: 'some classes'}); - popup.addClassName('even'); - popup.addClassName('more'); + popup.addClassName('even') + .addClassName('more'); popup.setText('Test') .setLngLat([0, 0]) @@ -646,6 +646,38 @@ test('Popup className option and addClassName both add classes', (t) => { t.end(); }); +test('Methods for class manipulations works properly when popup is not on map', (t) => { + const map = createMap(t); + const popup = new Popup() + .setText('Test') + .setLngLat([0, 0]) + .addClassName('some') + .addClassName('classes'); + + let popupContainer = popup.addTo(map).getElement(); + t.ok(popupContainer.classList.contains('some')); + t.ok(popupContainer.classList.contains('classes')); + + popup.remove(); + popup.removeClassName('some'); + popupContainer = popup.addTo(map).getElement(); + + t.ok(!popupContainer.classList.contains('some')); + + popup.remove(); + popup.toggleClassName('toggle'); + popupContainer = popup.addTo(map).getElement(); + + t.ok(popupContainer.classList.contains('toggle')); + + popup.remove(); + popup.toggleClassName('toggle'); + popupContainer = popup.addTo(map).getElement(); + + t.ok(!popupContainer.classList.contains('toggle')); + t.end(); +}); + test('Cursor-tracked popup disappears on mouseout', (t) => { const map = createMap(t); From da5268b22b15ca077ffdac8fe2912b0a18abbed1 Mon Sep 17 00:00:00 2001 From: Aidan Date: Thu, 22 Jul 2021 16:52:18 -0600 Subject: [PATCH 11/17] Trim and split on any whitespace as Ricky suggested --- src/ui/popup.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ui/popup.js b/src/ui/popup.js index d89c55aaf77..1b57bf0c5bb 100644 --- a/src/ui/popup.js +++ b/src/ui/popup.js @@ -113,7 +113,8 @@ export default class Popup extends Evented { super(); this.options = extend(Object.create(defaultOptions), options); bindAll(['_update', '_onClose', 'remove', '_onMouseMove', '_onMouseUp', '_onDrag'], this); - this._classList = new Set(options && options.className ? options.className.split(` `) : []); + this._classList = new Set(options && options.className ? + options.className.trim().split(/\s+/) : []); } /** From acfa8890dc7a16e4155d7879087887eba266aadd Mon Sep 17 00:00:00 2001 From: Aidan Date: Thu, 22 Jul 2021 17:21:53 -0600 Subject: [PATCH 12/17] Update documentation for @returns --- src/ui/popup.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ui/popup.js b/src/ui/popup.js index 1b57bf0c5bb..4e615216a81 100644 --- a/src/ui/popup.js +++ b/src/ui/popup.js @@ -438,7 +438,7 @@ export default class Popup extends Evented { * Adds a CSS class to the popup container element. * * @param {string} className Non-empty string with CSS class name to add to popup container - * @returns {Popup} `this` + * @returns {Popup} Returns itself to allow for method chaining. * * @example * let popup = new mapboxgl.Popup() @@ -457,7 +457,7 @@ export default class Popup extends Evented { * * @param {string} className Non-empty string with CSS class name to remove from popup container * - * @returns {Popup} `this` + * @returns {Popup} Returns itself to allow for method chaining. * @example * const popup = new mapboxgl.Popup({className: 'some classes'}) * popup.removeClassName('some') From 5d7b61da8e1dddcf4b552b2ed57f7d2c4873d01d Mon Sep 17 00:00:00 2001 From: Aidan Date: Fri, 23 Jul 2021 17:57:14 -0600 Subject: [PATCH 13/17] Refactored so that changes to the container are always performed by _updateClassList --- src/ui/popup.js | 133 +++++++++++++++++++------------------ test/unit/ui/popup.test.js | 10 +-- 2 files changed, 69 insertions(+), 74 deletions(-) diff --git a/src/ui/popup.js b/src/ui/popup.js index 4e615216a81..1483e766277 100644 --- a/src/ui/popup.js +++ b/src/ui/popup.js @@ -8,7 +8,7 @@ import LngLat from '../geo/lng_lat.js'; import Point from '@mapbox/point-geometry'; import window from '../util/window.js'; import smartWrap from '../util/smart_wrap.js'; -import {type Anchor, anchorTranslate, applyAnchorClass} from './anchor.js'; +import {type Anchor, anchorTranslate} from './anchor.js'; import type Map from './map.js'; import type {LngLatLike} from '../geo/lng_lat.js'; @@ -107,6 +107,7 @@ export default class Popup extends Evented { _lngLat: LngLat; _trackPointer: boolean; _pos: ?Point; + _anchor: Anchor; _classList: Set; constructor(options: PopupOptions) { @@ -151,9 +152,6 @@ export default class Popup extends Evented { if (this._trackPointer) { this._map.on('mousemove', this._onMouseMove); this._map.on('mouseup', this._onMouseUp); - if (this._container) { - this._container.classList.add('mapboxgl-popup-track-pointer'); - } this._map._canvasContainer.classList.add('mapboxgl-track-pointer'); } else { this._map.on('move', this._update); @@ -273,9 +271,6 @@ export default class Popup extends Evented { if (this._map) { this._map.on('move', this._update); this._map.off('mousemove', this._onMouseMove); - if (this._container) { - this._container.classList.remove('mapboxgl-popup-track-pointer'); - } this._map._canvasContainer.classList.remove('mapboxgl-track-pointer'); } @@ -300,9 +295,6 @@ export default class Popup extends Evented { this._map.off('move', this._update); this._map.on('mousemove', this._onMouseMove); this._map.on('drag', this._onDrag); - if (this._container) { - this._container.classList.add('mapboxgl-popup-track-pointer'); - } this._map._canvasContainer.classList.add('mapboxgl-track-pointer'); } @@ -447,7 +439,7 @@ export default class Popup extends Evented { addClassName(className: string) { this._classList.add(className); if (this._container) { - this._container.classList.add(className); + this._updateClassList(); } return this; } @@ -465,7 +457,7 @@ export default class Popup extends Evented { removeClassName(className: string) { this._classList.delete(className); if (this._container) { - this._container.classList.remove(className); + this._updateClassList(); } return this; } @@ -494,15 +486,18 @@ export default class Popup extends Evented { * popup.toggleClassName('highlighted') */ toggleClassName(className: string) { - if (this._container) { - this._container.classList.toggle(className); - } + let finalState: boolean; if (this._classList.has(className)) { this._classList.delete(className); - return false; + finalState = false; + } else { + this._classList.add(className); + finalState = true; } - this._classList.add(className); - return true; + if (this._container) { + this._updateClassList(); + } + return finalState; } _createCloseButton() { @@ -527,6 +522,47 @@ export default class Popup extends Evented { this._update(event.point); } + _getAnchor(offset: any) { + if (this.options.anchor) { return this.options.anchor; } + + const pos: any = this._pos; + const width = this._container.offsetWidth; + const height = this._container.offsetHeight; + + let anchorComponents; + if (pos.y + offset.bottom.y < height) { + anchorComponents = ['top']; + } else if (pos.y > this._map.transform.height - height) { + anchorComponents = ['bottom']; + } else { + anchorComponents = []; + } + + if (pos.x < width / 2) { + anchorComponents.push('left'); + } else if (pos.x > this._map.transform.width - width / 2) { + anchorComponents.push('right'); + } + + if (anchorComponents.length === 0) { + return 'bottom'; + } + return ((anchorComponents.join('-'): any): Anchor); + + } + + _updateClassList() { + const classes = new Set(this._classList); + classes.add('mapboxgl-popup'); + if (this._anchor) { + classes.add(`mapboxgl-popup-anchor-${this._anchor}`); + } + if (this._trackPointer) { + classes.add('mapboxgl-popup-track-pointer'); + } + this._container.className = [...classes].join(' '); + } + _update(cursor: ?PointLike) { const hasPosition = this._lngLat || this._trackPointer; @@ -536,14 +572,6 @@ export default class Popup extends Evented { this._container = DOM.create('div', 'mapboxgl-popup', this._map.getContainer()); this._tip = DOM.create('div', 'mapboxgl-popup-tip', this._container); this._container.appendChild(this._content); - if (this._classList) { - this._classList.forEach(name => - this._container.classList.add(name)); - } - - if (this._trackPointer) { - this._container.classList.add('mapboxgl-popup-track-pointer'); - } } if (this.options.maxWidth && this._container.style.maxWidth !== this.options.maxWidth) { @@ -554,46 +582,22 @@ export default class Popup extends Evented { this._lngLat = smartWrap(this._lngLat, this._pos, this._map.transform); } - if (this._trackPointer && !cursor) return; + if (!this._trackPointer || cursor) { + const pos = this._pos = this._trackPointer && cursor ? cursor : this._map.project(this._lngLat); - const pos = this._pos = this._trackPointer && cursor ? cursor : this._map.project(this._lngLat); + const offset = normalizeOffset(this.options.offset); + const anchor = this._anchor = this._getAnchor(offset); + this._updateClassList(); - let anchor: ?Anchor = this.options.anchor; - const offset = normalizeOffset(this.options.offset); - - if (!anchor) { - const width = this._container.offsetWidth; - const height = this._container.offsetHeight; - let anchorComponents; - - if (pos.y + offset.bottom.y < height) { - anchorComponents = ['top']; - } else if (pos.y > this._map.transform.height - height) { - anchorComponents = ['bottom']; - } else { - anchorComponents = []; - } - - if (pos.x < width / 2) { - anchorComponents.push('left'); - } else if (pos.x > this._map.transform.width - width / 2) { - anchorComponents.push('right'); - } - - if (anchorComponents.length === 0) { - anchor = 'bottom'; - } else { - anchor = (anchorComponents.join('-'): any); - } + const offsetedPos = pos.add(offset[anchor]).round(); + this._map._requestDomTask(() => { + if (this._container && anchor) { + DOM.setTransform(this._container, `${anchorTranslate[anchor]} translate(${offsetedPos.x}px,${offsetedPos.y}px)`); + } + }); } - const offsetedPos = pos.add(offset[anchor]).round(); - this._map._requestDomTask(() => { - if (this._container && anchor) { - DOM.setTransform(this._container, `${anchorTranslate[anchor]} translate(${offsetedPos.x}px,${offsetedPos.y}px)`); - applyAnchorClass(this._container, anchor, 'popup'); - } - }); + this._updateClassList(); } _focusFirstElement() { @@ -615,10 +619,9 @@ export default class Popup extends Evented { } function normalizeOffset(offset: ?Offset) { - if (!offset) { - return normalizeOffset(new Point(0, 0)); + if (!offset) offset = (new Point(0, 0)); - } else if (typeof offset === 'number') { + if (typeof offset === 'number') { // input specifies a radius from which to calculate offsets at all positions const cornerOffset = Math.round(Math.sqrt(0.5 * Math.pow(offset, 2))); return { diff --git a/test/unit/ui/popup.test.js b/test/unit/ui/popup.test.js index 8cb5c7708f6..0685b3e2e11 100644 --- a/test/unit/ui/popup.test.js +++ b/test/unit/ui/popup.test.js @@ -599,18 +599,10 @@ test('Popup adds classes from className option, methods for class manipulations popup.toggleClassName('toggle'); t.ok(!popupContainer.classList.contains('toggle')); - t.throws(() => popup.addClassName('should throw exception'), window.DOMException); - t.throws(() => popup.removeClassName('should throw exception'), window.DOMException); - t.throws(() => popup.toggleClassName('should throw exception'), window.DOMException); - - t.throws(() => popup.addClassName(''), window.DOMException); - t.throws(() => popup.removeClassName(''), window.DOMException); - t.throws(() => popup.toggleClassName(''), window.DOMException); - t.end(); }); -test('Popup.addClassName adds classes when called before adding popup to map', (t) => { +test('Popup#addClassName adds classes when called before adding popup to map (#9677)', (t) => { const map = createMap(t); const popup = new Popup(); popup.addClassName('some'); From b9e20fac76b37b4678c573cb20fdc1084e4a7c53 Mon Sep 17 00:00:00 2001 From: Aidan Date: Fri, 23 Jul 2021 18:08:51 -0600 Subject: [PATCH 14/17] Inlining function from anchor.js to marker.js --- src/ui/anchor.js | 8 -------- src/ui/marker.js | 8 ++++++-- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/ui/anchor.js b/src/ui/anchor.js index 78b2bc2b99f..7a089a41b8b 100644 --- a/src/ui/anchor.js +++ b/src/ui/anchor.js @@ -22,11 +22,3 @@ export const anchorTranslate: {[_: Anchor]: string} = { 'left': 'translate(0,-50%)', 'right': 'translate(-100%,-50%)' }; - -export function applyAnchorClass(element: HTMLElement, anchor: Anchor, prefix: string) { - const classList = element.classList; - for (const key in anchorTranslate) { - classList.remove(`mapboxgl-${prefix}-anchor-${key}`); - } - classList.add(`mapboxgl-${prefix}-anchor-${anchor}`); -} diff --git a/src/ui/marker.js b/src/ui/marker.js index 59b9f6111ce..3dcde0e3a90 100644 --- a/src/ui/marker.js +++ b/src/ui/marker.js @@ -6,7 +6,7 @@ import LngLat from '../geo/lng_lat.js'; import Point from '@mapbox/point-geometry'; import smartWrap from '../util/smart_wrap.js'; import {bindAll, extend} from '../util/util.js'; -import {type Anchor, anchorTranslate, applyAnchorClass} from './anchor.js'; +import {type Anchor, anchorTranslate} from './anchor.js'; import {Event, Evented} from '../util/evented.js'; import type Map from './map.js'; import type Popup from './popup.js'; @@ -231,7 +231,11 @@ export default class Marker extends Evented { // prevent focusing on click e.preventDefault(); }); - applyAnchorClass(this._element, this._anchor, 'marker'); + const classList = this._element.classList; + for (const key in anchorTranslate) { + classList.remove(`mapboxgl-marker-anchor-${key}`); + } + classList.add(`mapboxgl-marker-anchor-${this._anchor}`); this._popup = null; } From fd70ead6e3b87d23af3b51133169f80016894fdb Mon Sep 17 00:00:00 2001 From: Aidan Date: Sun, 25 Jul 2021 14:13:00 -0600 Subject: [PATCH 15/17] Typo fix --- test/unit/ui/popup.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/ui/popup.test.js b/test/unit/ui/popup.test.js index 0685b3e2e11..2073e14d3b4 100644 --- a/test/unit/ui/popup.test.js +++ b/test/unit/ui/popup.test.js @@ -576,7 +576,7 @@ test('Popup#remove is idempotent (#2395)', (t) => { t.end(); }); -test('Popup adds classes from className option, methods for class manipulations works properly', (t) => { +test('Popup adds classes from className option, methods for class manipulation work properly', (t) => { const map = createMap(t); const popup = new Popup({className: 'some classes'}) .setText('Test') @@ -638,7 +638,7 @@ test('Popup className option and addClassName both add classes', (t) => { t.end(); }); -test('Methods for class manipulations works properly when popup is not on map', (t) => { +test('Methods for class manipulation work properly when popup is not on map', (t) => { const map = createMap(t); const popup = new Popup() .setText('Test') From aa89bc26c291e37d530643e963b8539bc0ac7102 Mon Sep 17 00:00:00 2001 From: Aidan Date: Mon, 2 Aug 2021 15:10:57 -0600 Subject: [PATCH 16/17] Small changes from suggestions --- src/ui/popup.js | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/ui/popup.js b/src/ui/popup.js index 1483e766277..11154812f3c 100644 --- a/src/ui/popup.js +++ b/src/ui/popup.js @@ -487,8 +487,7 @@ export default class Popup extends Evented { */ toggleClassName(className: string) { let finalState: boolean; - if (this._classList.has(className)) { - this._classList.delete(className); + if (this._classList.delete(className)) { finalState = false; } else { this._classList.add(className); @@ -528,8 +527,8 @@ export default class Popup extends Evented { const pos: any = this._pos; const width = this._container.offsetWidth; const height = this._container.offsetHeight; - let anchorComponents; + if (pos.y + offset.bottom.y < height) { anchorComponents = ['top']; } else if (pos.y > this._map.transform.height - height) { @@ -552,15 +551,15 @@ export default class Popup extends Evented { } _updateClassList() { - const classes = new Set(this._classList); - classes.add('mapboxgl-popup'); + const classes = [...this._classList]; + classes.push('mapboxgl-popup'); if (this._anchor) { - classes.add(`mapboxgl-popup-anchor-${this._anchor}`); + classes.push(`mapboxgl-popup-anchor-${this._anchor}`); } if (this._trackPointer) { - classes.add('mapboxgl-popup-track-pointer'); + classes.push('mapboxgl-popup-track-pointer'); } - this._container.className = [...classes].join(' '); + this._container.className = classes.join(' '); } _update(cursor: ?PointLike) { @@ -587,7 +586,6 @@ export default class Popup extends Evented { const offset = normalizeOffset(this.options.offset); const anchor = this._anchor = this._getAnchor(offset); - this._updateClassList(); const offsetedPos = pos.add(offset[anchor]).round(); this._map._requestDomTask(() => { From 745581bf6263fa6bf5e40128c340a40edae0bcf4 Mon Sep 17 00:00:00 2001 From: Aidan Date: Mon, 2 Aug 2021 15:31:32 -0600 Subject: [PATCH 17/17] Aadding ;s to examples --- src/ui/popup.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ui/popup.js b/src/ui/popup.js index 325119965f3..04f96e007a2 100644 --- a/src/ui/popup.js +++ b/src/ui/popup.js @@ -467,8 +467,8 @@ export default class Popup extends Evented { * * @returns {Popup} Returns itself to allow for method chaining. * @example - * const popup = new mapboxgl.Popup({className: 'some classes'}) - * popup.removeClassName('some') + * const popup = new mapboxgl.Popup({className: 'some classes'}); + * popup.removeClassName('some'); */ removeClassName(className: string) { this._classList.delete(className); @@ -512,8 +512,8 @@ export default class Popup extends Evented { * @returns {boolean} If the class was removed return `false`. If the class was added, then return `true`. * * @example - * const popup = new mapboxgl.Popup() - * popup.toggleClassName('highlighted') + * const popup = new mapboxgl.Popup(); + * popup.toggleClassName('highlighted'); */ toggleClassName(className: string) { let finalState: boolean;