From c2c9dfb4661ffef3d1180f7c6a4bd23bb28912dd Mon Sep 17 00:00:00 2001 From: Kamil Sienkiewicz Date: Thu, 11 May 2023 17:53:23 +0200 Subject: [PATCH 1/7] chore: implement padding for map popup --- src/ui/popup.js | 37 ++++++++++++++++++++++--------------- test/unit/ui/popup.test.js | 21 +++++++++++++++++++++ 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/src/ui/popup.js b/src/ui/popup.js index c41e0cb90f6..ed4242d380a 100644 --- a/src/ui/popup.js +++ b/src/ui/popup.js @@ -14,6 +14,7 @@ import {isLngLatBehindGlobe} from '../geo/projection/globe_util.js'; import type Map from './map.js'; import type {LngLatLike} from '../geo/lng_lat.js'; import type {PointLike} from '@mapbox/point-geometry'; +import type {PaddingOptions} from '../geo/edge_insets.js'; import type Marker from './marker.js'; const defaultOptions = { @@ -21,7 +22,8 @@ const defaultOptions = { closeOnClick: true, focusAfterOpen: true, className: '', - maxWidth: "240px" + maxWidth: "240px", + padding: {top:0, left:0, right:0, bottom:0} }; export type Offset = number | PointLike | {[_: Anchor]: PointLike}; @@ -34,7 +36,8 @@ export type PopupOptions = { anchor?: Anchor, offset?: Offset, className?: string, - maxWidth?: string + maxWidth?: string, + padding?: PaddingOptions }; const focusQuerySelector = [ @@ -117,7 +120,11 @@ export default class Popup extends Evented { constructor(options: PopupOptions) { super(); - this.options = extend(Object.create(defaultOptions), options); + let validPadding = defaultOptions.padding; + if (options) { + validPadding = extend(Object.create(defaultOptions.padding), options.padding); + } + this.options = extend(Object.create(defaultOptions), {...options, padding: validPadding}); bindAll(['_update', '_onClose', 'remove', '_onMouseEvent'], this); this._classList = new Set(options && options.className ? options.className.trim().split(/\s+/) : []); @@ -580,10 +587,10 @@ export default class Popup extends Evented { const width = container.offsetWidth; const height = container.offsetHeight; - const isTop = pos.y + bottomY < height; - const isBottom = pos.y > map.transform.height - height; - const isLeft = pos.x < width / 2; - const isRight = pos.x > map.transform.width - width / 2; + const isTop = pos.y + bottomY - this.options.padding.top < height; + const isBottom = pos.y > map.transform.height - height - this.options.padding.bottom; + const isLeft = pos.x < width / 2 + this.options.padding.left; + const isRight = pos.x > map.transform.width - width / 2 - this.options.padding.right; if (isTop) { if (isLeft) return 'top-left'; @@ -689,14 +696,14 @@ function normalizeOffset(offset: Offset = new Point(0, 0), anchor: Anchor = 'bot // input specifies a radius from which to calculate offsets at all positions const cornerOffset = Math.round(Math.sqrt(0.5 * Math.pow(offset, 2))); switch (anchor) { - case 'top': return new Point(0, offset); - case 'top-left': return new Point(cornerOffset, cornerOffset); - case 'top-right': return new Point(-cornerOffset, cornerOffset); - case 'bottom': return new Point(0, -offset); - case 'bottom-left': return new Point(cornerOffset, -cornerOffset); - case 'bottom-right': return new Point(-cornerOffset, -cornerOffset); - case 'left': return new Point(offset, 0); - case 'right': return new Point(-offset, 0); + case 'top': return new Point(0, offset); + case 'top-left': return new Point(cornerOffset, cornerOffset); + case 'top-right': return new Point(-cornerOffset, cornerOffset); + case 'bottom': return new Point(0, -offset); + case 'bottom-left': return new Point(cornerOffset, -cornerOffset); + case 'bottom-right': return new Point(-cornerOffset, -cornerOffset); + case 'left': return new Point(offset, 0); + case 'right': return new Point(-offset, 0); } return new Point(0, 0); } diff --git a/test/unit/ui/popup.test.js b/test/unit/ui/popup.test.js index d01373e1fb0..f5a65be162c 100644 --- a/test/unit/ui/popup.test.js +++ b/test/unit/ui/popup.test.js @@ -479,6 +479,27 @@ test('Popup', (t) => { const point = args[1]; const transform = args[2]; + test('Popup respects padding and changes anchor accordingly', (t) => { + const map = createMap(t); + const padding = {top: 10, bottom: 10, left: 10, right: 10}; + const popup = new Popup({padding}) + .setText('Test') + .setLngLat([0, 0]) + .addTo(map); + map._domRenderTaskQueue.run(); + + Object.defineProperty(popup.getElement(), 'offsetWidth', {value: 5}); + Object.defineProperty(popup.getElement(), 'offsetHeight', {value: 5}); + + t.stub(map, 'project').returns(point); + t.stub(map.transform, 'locationPoint3D').returns(point); + popup.setLngLat([0, 0]); + map._domRenderTaskQueue.run(); + + t.ok(popup.getElement().classList.contains(`mapboxgl-popup-anchor-${anchor}`)); + t.end(); + }); + test(`Popup automatically anchors to ${anchor}`, (t) => { const map = createMap(t); const popup = new Popup() From 38bd0ae2a28f659c35bd95ef85772e9f8f588e76 Mon Sep 17 00:00:00 2001 From: Kamil Sienkiewicz Date: Thu, 11 May 2023 18:00:05 +0200 Subject: [PATCH 2/7] chore: fix code styling --- src/ui/popup.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/ui/popup.js b/src/ui/popup.js index ed4242d380a..00f86f34343 100644 --- a/src/ui/popup.js +++ b/src/ui/popup.js @@ -587,10 +587,10 @@ export default class Popup extends Evented { const width = container.offsetWidth; const height = container.offsetHeight; - const isTop = pos.y + bottomY - this.options.padding.top < height; + const isTop = pos.y + bottomY < height + this.options.padding.top; const isBottom = pos.y > map.transform.height - height - this.options.padding.bottom; const isLeft = pos.x < width / 2 + this.options.padding.left; - const isRight = pos.x > map.transform.width - width / 2 - this.options.padding.right; + const isRight = pos.x > map.transform.width - width / 2 - this.options.padding.right; if (isTop) { if (isLeft) return 'top-left'; @@ -696,14 +696,14 @@ function normalizeOffset(offset: Offset = new Point(0, 0), anchor: Anchor = 'bot // input specifies a radius from which to calculate offsets at all positions const cornerOffset = Math.round(Math.sqrt(0.5 * Math.pow(offset, 2))); switch (anchor) { - case 'top': return new Point(0, offset); - case 'top-left': return new Point(cornerOffset, cornerOffset); - case 'top-right': return new Point(-cornerOffset, cornerOffset); - case 'bottom': return new Point(0, -offset); - case 'bottom-left': return new Point(cornerOffset, -cornerOffset); - case 'bottom-right': return new Point(-cornerOffset, -cornerOffset); - case 'left': return new Point(offset, 0); - case 'right': return new Point(-offset, 0); + case 'top': return new Point(0, offset); + case 'top-left': return new Point(cornerOffset, cornerOffset); + case 'top-right': return new Point(-cornerOffset, cornerOffset); + case 'bottom': return new Point(0, -offset); + case 'bottom-left': return new Point(cornerOffset, -cornerOffset); + case 'bottom-right': return new Point(-cornerOffset, -cornerOffset); + case 'left': return new Point(offset, 0); + case 'right': return new Point(-offset, 0); } return new Point(0, 0); } From 2b11868b9c7dbbc3c7dfc32b90808d696993d28d Mon Sep 17 00:00:00 2001 From: Kamil Sienkiewicz Date: Fri, 12 May 2023 12:58:01 +0200 Subject: [PATCH 3/7] fix: typings in popup.js --- src/ui/popup.js | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/src/ui/popup.js b/src/ui/popup.js index 00f86f34343..a2506f3ebe2 100644 --- a/src/ui/popup.js +++ b/src/ui/popup.js @@ -17,6 +17,11 @@ import type {PointLike} from '@mapbox/point-geometry'; import type {PaddingOptions} from '../geo/edge_insets.js'; import type Marker from './marker.js'; +/** + * A helper type: converts all Object type values to non-maybe types. + */ +type Required = $ObjMap(v: V) => $NonMaybeType>; + const defaultOptions = { closeButton: true, closeOnClick: true, @@ -115,17 +120,15 @@ export default class Popup extends Evented { _trackPointer: boolean; _pos: ?Point; _anchor: Anchor; + _padding: Required; _classList: Set; _marker: ?Marker; constructor(options: PopupOptions) { super(); - let validPadding = defaultOptions.padding; - if (options) { - validPadding = extend(Object.create(defaultOptions.padding), options.padding); - } - this.options = extend(Object.create(defaultOptions), {...options, padding: validPadding}); + this.options = extend(Object.create(defaultOptions), options); bindAll(['_update', '_onClose', 'remove', '_onMouseEvent'], this); + this._padding = this._getPadding(); this._classList = new Set(options && options.className ? options.className.trim().split(/\s+/) : []); } @@ -575,6 +578,19 @@ export default class Popup extends Evented { this._update(event.point); } + _getPadding(): Required { + if (!this.options.padding) { + return defaultOptions.padding; + } + + return { + top: this.options.padding.top || 0, + left: this.options.padding.left || 0, + bottom: this.options.padding.bottom || 0, + right: this.options.padding.right || 0, + }; + } + _getAnchor(bottomY: number): Anchor { if (this.options.anchor) { return this.options.anchor; } @@ -587,10 +603,10 @@ export default class Popup extends Evented { const width = container.offsetWidth; const height = container.offsetHeight; - const isTop = pos.y + bottomY < height + this.options.padding.top; - const isBottom = pos.y > map.transform.height - height - this.options.padding.bottom; - const isLeft = pos.x < width / 2 + this.options.padding.left; - const isRight = pos.x > map.transform.width - width / 2 - this.options.padding.right; + const isTop = pos.y + bottomY < height + this._padding.top; + const isBottom = pos.y > map.transform.height - height - this._padding.bottom; + const isLeft = pos.x < width / 2 + this._padding.left; + const isRight = pos.x > map.transform.width - width / 2 - this._padding.right; if (isTop) { if (isLeft) return 'top-left'; From 3487c2ede0a01e2db2505f7ea7979392e9432b16 Mon Sep 17 00:00:00 2001 From: Kamil Sienkiewicz Date: Thu, 25 May 2023 14:41:51 +0200 Subject: [PATCH 4/7] chore: use global padding instead of local padding in popup, adjust placement of popup --- src/ui/popup.js | 57 ++++++++++++-------------------------- test/unit/ui/popup.test.js | 4 +-- 2 files changed, 20 insertions(+), 41 deletions(-) diff --git a/src/ui/popup.js b/src/ui/popup.js index a2506f3ebe2..d8732714ac5 100644 --- a/src/ui/popup.js +++ b/src/ui/popup.js @@ -14,21 +14,14 @@ import {isLngLatBehindGlobe} from '../geo/projection/globe_util.js'; import type Map from './map.js'; import type {LngLatLike} from '../geo/lng_lat.js'; import type {PointLike} from '@mapbox/point-geometry'; -import type {PaddingOptions} from '../geo/edge_insets.js'; import type Marker from './marker.js'; -/** - * A helper type: converts all Object type values to non-maybe types. - */ -type Required = $ObjMap(v: V) => $NonMaybeType>; - const defaultOptions = { closeButton: true, closeOnClick: true, focusAfterOpen: true, className: '', - maxWidth: "240px", - padding: {top:0, left:0, right:0, bottom:0} + maxWidth: "240px" }; export type Offset = number | PointLike | {[_: Anchor]: PointLike}; @@ -41,8 +34,7 @@ export type PopupOptions = { anchor?: Anchor, offset?: Offset, className?: string, - maxWidth?: string, - padding?: PaddingOptions + maxWidth?: string }; const focusQuerySelector = [ @@ -120,7 +112,6 @@ export default class Popup extends Evented { _trackPointer: boolean; _pos: ?Point; _anchor: Anchor; - _padding: Required; _classList: Set; _marker: ?Marker; @@ -128,7 +119,6 @@ export default class Popup extends Evented { super(); this.options = extend(Object.create(defaultOptions), options); bindAll(['_update', '_onClose', 'remove', '_onMouseEvent'], this); - this._padding = this._getPadding(); this._classList = new Set(options && options.className ? options.className.trim().split(/\s+/) : []); } @@ -578,23 +568,11 @@ export default class Popup extends Evented { this._update(event.point); } - _getPadding(): Required { - if (!this.options.padding) { - return defaultOptions.padding; - } - - return { - top: this.options.padding.top || 0, - left: this.options.padding.left || 0, - bottom: this.options.padding.bottom || 0, - right: this.options.padding.right || 0, - }; - } - _getAnchor(bottomY: number): Anchor { if (this.options.anchor) { return this.options.anchor; } const map = this._map; + const padding = this._map.transform.padding; const container = this._container; const pos = this._pos; @@ -603,24 +581,25 @@ export default class Popup extends Evented { const width = container.offsetWidth; const height = container.offsetHeight; - const isTop = pos.y + bottomY < height + this._padding.top; - const isBottom = pos.y > map.transform.height - height - this._padding.bottom; - const isLeft = pos.x < width / 2 + this._padding.left; - const isRight = pos.x > map.transform.width - width / 2 - this._padding.right; + const isTop = pos.y + bottomY < height + padding.top; + const isLeft = pos.x < width / 2 + padding.left; + const isRight = pos.x > map.transform.width - width / 2 - padding.right; + const isTopOnEitherSide = pos.y + bottomY < height / 2 + padding.top; + const isBottomOnEitherSide = pos.y > map.transform.height - height / 2 - padding.bottom; - if (isTop) { - if (isLeft) return 'top-left'; - if (isRight) return 'top-right'; - return 'top'; + if (isLeft) { + if (isTopOnEitherSide) return 'top-left'; + if (isBottomOnEitherSide) return 'bottom-left'; + return 'left'; } - if (isBottom) { - if (isLeft) return 'bottom-left'; - if (isRight) return 'bottom-right'; + + if (isRight) { + if (isTopOnEitherSide) return 'top-right'; + if (isBottomOnEitherSide) return 'bottom-right'; + return 'right'; } - if (isLeft) return 'left'; - if (isRight) return 'right'; - return 'bottom'; + return isTop ? 'top' : 'bottom'; } _updateClassList() { diff --git a/test/unit/ui/popup.test.js b/test/unit/ui/popup.test.js index f5a65be162c..94e0d16242f 100644 --- a/test/unit/ui/popup.test.js +++ b/test/unit/ui/popup.test.js @@ -481,8 +481,8 @@ test('Popup', (t) => { test('Popup respects padding and changes anchor accordingly', (t) => { const map = createMap(t); - const padding = {top: 10, bottom: 10, left: 10, right: 10}; - const popup = new Popup({padding}) + map.setPadding({top: 10, bottom: 10, left: 10, right: 10}); + const popup = new Popup() .setText('Test') .setLngLat([0, 0]) .addTo(map); From 50db2267d1f523af83c56c927922223ef1027bc2 Mon Sep 17 00:00:00 2001 From: Kamil Sienkiewicz Date: Thu, 25 May 2023 15:57:40 +0200 Subject: [PATCH 5/7] chore: fix test-flow error --- 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 d8732714ac5..78f9cc3c1e9 100644 --- a/src/ui/popup.js +++ b/src/ui/popup.js @@ -572,12 +572,12 @@ export default class Popup extends Evented { if (this.options.anchor) { return this.options.anchor; } const map = this._map; - const padding = this._map.transform.padding; const container = this._container; const pos = this._pos; if (!map || !container || !pos) return 'bottom'; + const padding = this._map.transform.padding; const width = container.offsetWidth; const height = container.offsetHeight; From 23f71420b1b1dc560469e5fbf7e42461708cc474 Mon Sep 17 00:00:00 2001 From: Kamil Sienkiewicz Date: Thu, 25 May 2023 16:05:15 +0200 Subject: [PATCH 6/7] chore: fix test-flow --- src/ui/popup.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/ui/popup.js b/src/ui/popup.js index 78f9cc3c1e9..8d087b2ba74 100644 --- a/src/ui/popup.js +++ b/src/ui/popup.js @@ -578,14 +578,19 @@ export default class Popup extends Evented { if (!map || !container || !pos) return 'bottom'; const padding = this._map.transform.padding; + const paddingTop = (padding.top || 0); + const paddingBottom = (padding.bottom || 0); + const paddingLeft = (padding.left || 0); + const paddingRight = (padding.right || 0); + const width = container.offsetWidth; const height = container.offsetHeight; - const isTop = pos.y + bottomY < height + padding.top; - const isLeft = pos.x < width / 2 + padding.left; - const isRight = pos.x > map.transform.width - width / 2 - padding.right; - const isTopOnEitherSide = pos.y + bottomY < height / 2 + padding.top; - const isBottomOnEitherSide = pos.y > map.transform.height - height / 2 - padding.bottom; + const isTop = pos.y + bottomY < height + paddingTop; + const isLeft = pos.x < width / 2 + paddingLeft; + const isRight = pos.x > map.transform.width - width / 2 - paddingRight; + const isTopOnEitherSide = pos.y + bottomY < height / 2 + paddingTop; + const isBottomOnEitherSide = pos.y > map.transform.height - height / 2 - paddingBottom; if (isLeft) { if (isTopOnEitherSide) return 'top-left'; From 4a926807e8c4c7ea8a0998454c9875150c28e7c1 Mon Sep 17 00:00:00 2001 From: Kamil Sienkiewicz Date: Thu, 25 May 2023 16:14:06 +0200 Subject: [PATCH 7/7] chore: fix test-flow --- 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 8d087b2ba74..7006e260bb7 100644 --- a/src/ui/popup.js +++ b/src/ui/popup.js @@ -577,7 +577,7 @@ export default class Popup extends Evented { if (!map || !container || !pos) return 'bottom'; - const padding = this._map.transform.padding; + const padding = map.transform.padding; const paddingTop = (padding.top || 0); const paddingBottom = (padding.bottom || 0); const paddingLeft = (padding.left || 0);