From f1761b212ec545b83ab07d1777a4c6baf1850975 Mon Sep 17 00:00:00 2001 From: Andrew Harvey Date: Thu, 18 Jan 2018 12:45:52 +1100 Subject: [PATCH 1/2] default marker offset --- src/ui/marker.js | 16 ++++++++++++++-- test/unit/ui/marker.test.js | 16 ++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/ui/marker.js b/src/ui/marker.js index 831dc02a31a..809c4b6ebc7 100644 --- a/src/ui/marker.js +++ b/src/ui/marker.js @@ -31,8 +31,6 @@ class Marker { _pos: ?Point; constructor(element: ?HTMLElement, options?: {offset: PointLike}) { - this._offset = Point.convert(options && options.offset || [0, 0]); - bindAll(['_update', '_onMapClick'], this); if (!element) { @@ -127,8 +125,22 @@ class Marker { svg.appendChild(page1); element.appendChild(svg); + + // if no element and no offset option given apply an offset for the default marker + const defaultMarkerOffset = [0, -14]; + if (!(options && options.offset)) { + if (!options) { + options = { + offset: defaultMarkerOffset + }; + } else { + options.offset = defaultMarkerOffset; + } + } } + this._offset = Point.convert(options && options.offset || [0, 0]); + element.classList.add('mapboxgl-marker'); this._element = element; diff --git a/test/unit/ui/marker.test.js b/test/unit/ui/marker.test.js index 6018bbb8fe5..d8b553c2d78 100644 --- a/test/unit/ui/marker.test.js +++ b/test/unit/ui/marker.test.js @@ -6,6 +6,7 @@ const Map = require('../../../src/ui/map'); const Marker = require('../../../src/ui/marker'); const Popup = require('../../../src/ui/popup'); const LngLat = require('../../../src/geo/lng_lat'); +const Point = require('@mapbox/point-geometry'); function createMap() { const container = window.document.createElement('div'); @@ -25,6 +26,21 @@ test('Marker', (t) => { t.test('default marker', (t) => { const marker = new Marker(); t.ok(marker.getElement(), 'default marker is created'); + t.ok(marker.getOffset().equals(new Point(0, -14)), 'default marker with no offset uses default marker offset'); + t.end(); + }); + + t.test('default marker with some options', (t) => { + const marker = new Marker(null, { foo: 'bar' }); + t.ok(marker.getElement(), 'default marker is created'); + t.ok(marker.getOffset().equals(new Point(0, -14)), 'default marker with no offset uses default marker offset'); + t.end(); + }); + + t.test('default marker with custom offest', (t) => { + const marker = new Marker(null, { offset: [1, 2] }); + t.ok(marker.getElement(), 'default marker is created'); + t.ok(marker.getOffset().equals(new Point(1, 2)), 'default marker with supplied offset'); t.end(); }); From e49ca66abb8c7767aac1d2a058ed8542417fb631 Mon Sep 17 00:00:00 2001 From: Andrew Harvey Date: Wed, 24 Jan 2018 12:50:24 +1100 Subject: [PATCH 2/2] include details on where the default marker offset number came from --- src/ui/marker.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/ui/marker.js b/src/ui/marker.js index 809c4b6ebc7..5ff9e804453 100644 --- a/src/ui/marker.js +++ b/src/ui/marker.js @@ -127,6 +127,12 @@ class Marker { element.appendChild(svg); // if no element and no offset option given apply an offset for the default marker + // the -14 as the y value of the default marker offset was determined as follows + // + // the marker tip is at the center of the shadow ellipse from the default svg + // the y value of the center of the shadow ellipse relative to the svg top left is "shadow transform translate-y (29.0) + ellipse cy (5.80029008)" + // offset to the svg center "height (41 / 2)" gives (29.0 + 5.80029008) - (41 / 2) and rounded for an integer pixel offset gives 14 + // negative is used to move the marker up from the center so the tip is at the Marker lngLat const defaultMarkerOffset = [0, -14]; if (!(options && options.offset)) { if (!options) {