Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rationalize Marker constructor options #6356

Merged
merged 1 commit into from
Mar 17, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();
});