Skip to content

Commit

Permalink
Rationalize Marker constructor options
Browse files Browse the repository at this point in the history
element is now optional, so it should be an option and not a required parameter. For backward compatibility, the old argument format is still supported.
  • Loading branch information
jfirebaugh committed Mar 17, 2018
1 parent 61278bc commit f3d97aa
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 14 deletions.
25 changes: 16 additions & 9 deletions src/ui/marker.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
// @flow

import DOM from '../util/dom';

import window from '../util/window';
import LngLat from '../geo/lng_lat';
import Point from '@mapbox/point-geometry';
import smartWrap from '../util/smart_wrap';
import { bindAll } from '../util/util';
import { bindAll, extend } from '../util/util';
import { type Anchor, anchorTranslate, applyAnchorClass } from './anchor';

import type Map from './map';
Expand All @@ -14,14 +14,15 @@ import type {LngLatLike} from "../geo/lng_lat";
import type {MapMouseEvent} from './events';

type Options = {
element?: HTMLElement,
offset?: PointLike,
anchor?: Anchor
};

/**
* Creates a marker component
* @param element DOM element to use as a marker. If left unspecified a default SVG will be created as the DOM element to use.
* @param options
* @param options.element DOM element to use as a marker. The default is a light blue, droplet-shaped SVG marker.
* @param options.anchor A string indicating the part of the Marker that should be positioned closest to the coordinate set via {@link Marker#setLngLat}.
* Options are `'center'`, `'top'`, `'bottom'`, `'left'`, `'right'`, `'top-left'`, `'top-right'`, `'bottom-left'`, and `'bottom-right'`.
* The default is `'center'`.
Expand All @@ -41,13 +42,19 @@ export default class Marker {
_lngLat: LngLat;
_pos: ?Point;

constructor(element: ?HTMLElement, options?: Options) {
constructor(options?: Options) {
// For backward compatibility -- the constructor used to accept the element as a
// required first argument, before it was made optional.
if (arguments[0] instanceof window.HTMLElement || arguments.length === 2) {
options = extend({element: options}, arguments[1]);
}

bindAll(['_update', '_onMapClick'], this);

this._anchor = options && options.anchor || 'center';

if (!element) {
element = DOM.create('div');
if (!options || !options.element) {
this._element = DOM.create('div');

// create default map marker SVG
const svg = DOM.createNS('http://www.w3.org/2000/svg', 'svg');
Expand Down Expand Up @@ -137,7 +144,7 @@ export default class Marker {

svg.appendChild(page1);

element.appendChild(svg);
this._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
Expand All @@ -148,12 +155,12 @@ export default class Marker {
// negative is used to move the marker up from the center so the tip is at the Marker lngLat
this._offset = Point.convert(options && options.offset || [0, -14]);
} else {
this._element = options.element;
this._offset = Point.convert(options && options.offset || [0, 0]);
}

element.classList.add('mapboxgl-marker');
this._element.classList.add('mapboxgl-marker');

this._element = element;
this._popup = null;
}

Expand Down
22 changes: 17 additions & 5 deletions test/unit/ui/marker.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,16 @@ test('Marker uses a default marker element with an appropriate offset', (t) => {
});

test('Marker uses a default marker with custom offest', (t) => {
const marker = new Marker(null, { offset: [1, 2] });
const marker = new Marker({ offset: [1, 2] });
t.ok(marker.getElement());
t.ok(marker.getOffset().equals(new Point(1, 2)));
t.end();
});

test('Marker uses the provided element', (t) => {
const el = window.document.createElement('div');
const marker = new Marker(el);
t.equal(marker.getElement(), el);
const element = window.document.createElement('div');
const marker = new Marker({element});
t.equal(marker.getElement(), element);
t.end();
});

Expand Down Expand Up @@ -128,7 +128,7 @@ test('Marker anchor defaults to center', (t) => {

test('Marker anchors as specified by the anchor option', (t) => {
const map = createMap();
const marker = new Marker(null, {anchor: 'top'})
const marker = new Marker({anchor: 'top'})
.setLngLat([0, 0])
.addTo(map);

Expand All @@ -138,3 +138,15 @@ test('Marker anchors as specified by the anchor option', (t) => {
map.remove();
t.end();
});

test('Marker accepts backward-compatible constructor parameters', (t) => {
const element = window.document.createElement('div');

const m1 = new Marker(element);
t.equal(m1.getElement(), element);

const m2 = new Marker(element, { offset: [1, 2] });
t.equal(m2.getElement(), element);
t.ok(m2.getOffset().equals(new Point(1, 2)));
t.end();
});

0 comments on commit f3d97aa

Please sign in to comment.