Skip to content

Commit

Permalink
refactor to conform to setter/getter conventions
Browse files Browse the repository at this point in the history
  • Loading branch information
Molly Lloyd committed Aug 24, 2016
1 parent fe2f6da commit 71b40aa
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 87 deletions.
83 changes: 31 additions & 52 deletions js/ui/marker.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ Marker.prototype = {
* @param {Map} map
* @returns {Marker} `this`
*/
addTo: function(map) {
addTo: function (map) {
this.remove();
this._map = map;
map.getCanvasContainer().appendChild(this._el);
Expand All @@ -54,7 +54,7 @@ Marker.prototype = {
* marker.remove();
* @returns {Marker} `this`
*/
remove: function() {
remove: function () {
if (this._map) {
this._map.off('move', this._update);
this._map = null;
Expand All @@ -69,7 +69,7 @@ Marker.prototype = {
* Get the marker's geographical location
* @returns {LngLat}
*/
getLngLat: function() {
getLngLat: function () {
return this._lngLat;
},

Expand All @@ -78,43 +78,35 @@ Marker.prototype = {
* @param {LngLat} lnglat
* @returns {Marker} `this`
*/
setLngLat: function(lnglat) {
setLngLat: function (lnglat) {
this._lngLat = LngLat.convert(lnglat);
if (this._popup) this._popup.setLngLat(this._lngLat);
this._update();
return this;
},

getElement: function() {
getElement: function () {
return this._el;
},

/**
* Binds a Popup to the Marker
* @param {HTMLElement|String|Popup} content the DOM content to appear in the popup, or
* an instance of the Popup class
* @param {Object} [options] options for the Popup class
* @param {boolean} [options.closeButton=true] If `true`, a close button will appear in the
* top right corner of the popup.
* @param {boolean} [options.closeOnClick=true] If `true`, the popup will closed when the
* map is clicked.
* @param {string} options.anchor - A string indicating the popup's location relative to
* the coordinate set via [Popup#setLngLat](#Popup#setLngLat).
* Options are `'top'`, `'bottom'`, `'left'`, `'right'`, `'top-left'`,
* `'top-right'`, `'bottom-left'`, and `'bottom-right'`.
* @param {Popup=} popup an instance of the `Popup` class. If undefined or null, any popup
* set on this `Marker` instance is unset
* @returns {Marker} `this`
*/

bindPopup: function(content, options){
if (content instanceof Popup) {
this._popup = content;
setPopup: function (popup) {
if (popup == null) {
this._closePopup();
delete this._popupHandlersAdded;
delete this._popup;
} else if (popup instanceof Popup) {
this._popup = popup;
} else {
if (!this._popup || options) {
this._popup = new Popup(options);
}
content instanceof HTMLElement ? this._popup.setDOMContent(content) : this._popup.setHTML(content);
util.warnOnce('Marker.setPopup only accepts an instance of the Popup class as an argument. If no argument is provided, the popup is unset from this Marker instance');
}

if (this._popup && this._lngLat) this._popup.setLngLat(this._lngLat);

if (!this._popupHandlersAdded) {
Expand All @@ -124,46 +116,32 @@ Marker.prototype = {
return this;
},

/**
* Returns the Popup instance that is bound to the Marker
* @returns {Popup} popup
*/
getPopup: function () {
return this._popup;
},

/**
* Opens or closes the bound popup, depending on the current state
* @returns {Marker} `this`
*/
togglePopup: function(){
togglePopup: function () {
if (this._popup) {
if (this._popup._map){
if (this._popup._map) {
this._closePopup();
} else {
this._openPopup();
}
}
},

/**
* Returns the Popup instance that is bound to the Marker
* @returns {Popup} popup
*/
getPopup: function(){
if (this._popup) {
return this._popup;
}
},

/**
* Unbinds a Popup from the Marker instance
* Returns the Popup instance that was bound to the Marker
* @returns {Popup} popup
*/
unbindPopup: function(){
this._closePopup();
var popup = this._popup;
delete this._popupHandlersAdded;
delete this._popup;
return popup;
},

_openPopup: function(e) {
_openPopup: function (e) {
// prevent event from bubbling up to the map canvas
e.stopPropagation();

if (!this._popup || !this._map) return;

if (!this._popup._map) {
Expand All @@ -173,17 +151,18 @@ Marker.prototype = {
return this;
},

_closePopup: function(){
_closePopup: function () {
if (this._popup) {
this._popup.remove();
}

return this;
},

_update: function() {
_update: function () {
if (!this._map) return;
var pos = this._map.project(this._lngLat)._add(this._offset);
DOM.setTransform(this._el, 'translate(' + pos.x + 'px,' + pos.y + 'px)');
}
};

4 changes: 1 addition & 3 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
"highlight.js": "9.3.0",
"istanbul": "^0.4.2",
"jsdom": "^9.4.2",
"jsdom-global": "^2.1.0",
"json-loader": "^0.5.4",
"lodash": "^4.13.1",
"mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#3e36b193a0c442a3fd863119f101afa6db97b32d",
Expand Down Expand Up @@ -94,15 +95,12 @@
"build-token": "browserify debug/access-token.js --debug --transform envify > debug/access-token-generated.js",
"watch-bench": "node bench/download-data.js && watchify bench/index.js --plugin [minifyify --no-map] --transform [babelify --presets react] --transform unassertify --transform envify --outfile bench/index-generated.js --verbose",
"start-server": "st --no-cache --localhost --port 9966 --index index.html .",

"start": "run-p build-token watch-dev watch-bench start-server",
"start-debug": "run-p build-token watch-dev start-server",
"start-bench": "run-p build-token watch-bench start-server",

"build-docs": "documentation build --github --format html --config documentation.yml --theme ./docs/_theme --output docs/api/",
"build": "npm run build-docs # invoked by publisher when publishing docs on the mb-pages branch",
"start-docs": "npm run build-min && npm run build-docs && jekyll serve --watch",

"lint": "eslint --ignore-path .gitignore js test bench docs/_posts/examples/*.html",
"open-changed-examples": "git diff --name-only mb-pages HEAD -- docs/_posts/examples/*.html | awk '{print \"http://127.0.0.1:4000/mapbox-gl-js/example/\" substr($0,33,length($0)-37)}' | xargs open",
"test-suite": "node test/render.test.js && node test/query.test.js",
Expand Down
50 changes: 18 additions & 32 deletions test/js/ui/marker.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,14 @@ var extend = require('../../../js/util/util').extend;
var Map = require('../../../js/ui/map');
var Marker = require('../../../js/ui/marker');
var Popup = require('../../../js/ui/popup');
var jsdom = require('jsdom-global');

function prepareDOM(html) {
html = html || '<!doctype html><html><body><div id="map"></div></body></html>';
if (typeof document !== 'undefined') {
return;
}
global.document = jsdom.jsdom(html);
global.window = global.document.defaultView;
global.navigator = {
userAgent: 'JSDOM'
};
global.HTMLElement = global.window.HTMLElement;

var cleanup = jsdom();
var mapdiv = document.createElement('div');
mapdiv.id = "map";
document.body.appendChild(mapdiv);
return cleanup;
}

function createMap(options, callback) {
Expand All @@ -45,59 +40,50 @@ function createMap(options, callback) {

test('Marker', function (t) {
t.test('constructor', function (t) {
prepareDOM();
var cleanup = prepareDOM();
var el = document.createElement('div');
var marker = new Marker(el);
t.ok(marker.getElement(), 'marker element is created');
cleanup();
t.end();
});

t.test('marker is added to map', function (t) {
prepareDOM();
var cleanup = prepareDOM();
var map = createMap();
var el = document.createElement('div');

map.on('load', function () {
var marker = new Marker(el).setLngLat([-77.01866, 38.888]);
t.ok(marker.addTo(map) instanceof Marker, 'marker.addTo(map) returns Marker instance');
t.ok(marker._map, 'marker instance is bound to map instance');
cleanup();
t.end();
});


});

t.test('popups can be bound to marker instance', function (t) {
prepareDOM();
var cleanup = prepareDOM();
var map = createMap();
var el = document.createElement('div');
var popup = new Popup();
var marker = new Marker(el).setLngLat([-77.01866, 38.888]).addTo(map);
marker.bindPopup(popup);
marker.setPopup(popup);
t.ok(marker.getPopup() instanceof Popup, 'popup created with Popup instance');
marker.unbindPopup();

marker.bindPopup('<h1>pop</h1>');
t.ok(marker.getPopup() instanceof Popup, 'popup created with HTML string');
marker.unbindPopup();

el.text = 'pop pop';
marker.bindPopup(el);
t.ok(marker.getPopup() instanceof Popup, 'popup created with HTMLElement');

cleanup();
t.end();
});

t.test('popups can be unbound from a marker instance', function (t) {
prepareDOM();
var cleanup = prepareDOM();
var map = createMap();
var el = document.createElement('div');
var marker = new Marker(el).setLngLat([-77.01866, 38.888]).addTo(map);
marker.bindPopup('<h1>pop</h1>');
marker.setPopup(new Popup());
t.ok(marker.getPopup() instanceof Popup);
t.deepEqual(marker.getPopup(), marker.unbindPopup(), 'Marker.unbindPopup returns previously bound popup');
t.ok(!marker.getPopup(), 'Marker.unbindPopup successfully removes Popup instance from Marker instance');
t.ok(marker.setPopup() instanceof Marker, 'passing no argument to Marker.setPopup() is valid');
t.ok(!marker.getPopup(), 'Calling setPopup with no argument successfully removes Popup instance from Marker instance');
t.end();
cleanup();
});

t.end();
Expand Down

0 comments on commit 71b40aa

Please sign in to comment.