From bcd11eca32ee00796efdb310e3eefe91b86844bf Mon Sep 17 00:00:00 2001 From: sandydoo Date: Fri, 25 May 2018 17:36:12 +0300 Subject: [PATCH 1/2] Cleanup internals * Correctly expose '_trigger' method and allow passing arbitrary arguments. * Expose `place` in autocomplete. * Remove usage of `const`. * Add assertions to base map-component. * Remove unused `getPosition` method from marker component. * Cleanup options processing. * Collect extracted event names in `_eventAttrs`. Use this array to filter out attrs when processing options. * Ensure `events` is an object, not an array of event names. --- addon/components/g-map.js | 6 ++-- addon/components/g-map/autocomplete.js | 16 ++++++--- addon/components/g-map/circle.js | 1 - addon/components/g-map/info-window.js | 11 +++++- addon/components/g-map/map-component.js | 17 +++++++--- addon/components/g-map/marker.js | 12 +------ addon/components/g-map/overlay.js | 2 +- addon/components/g-map/route.js | 4 ++- addon/components/g-map/waypoint.js | 2 ++ addon/mixins/process-options.js | 32 ++++++++++++------ addon/mixins/register-events.js | 45 ++++++++++++++----------- 11 files changed, 93 insertions(+), 55 deletions(-) diff --git a/addon/components/g-map.js b/addon/components/g-map.js index 4fa4470d..5a693230 100644 --- a/addon/components/g-map.js +++ b/addon/components/g-map.js @@ -19,7 +19,7 @@ const GMapAPI = { components: 'components', actions: { update: '_updateMap', - trigger: 'trigger', + trigger: '_trigger', } }; @@ -184,8 +184,8 @@ export default Component.extend(ProcessOptions, RegisterEvents, { * @param {String} event Event name * @return */ - _trigger(event) { - google.maps.event.trigger(get(this, 'map'), event); + _trigger(...args) { + google.maps.event.trigger(get(this, 'map'), ...args); }, _registerCanvas(canvas, isCustomCanvas = false) { diff --git a/addon/components/g-map/autocomplete.js b/addon/components/g-map/autocomplete.js index e4bf6e8a..98fcfc6c 100644 --- a/addon/components/g-map/autocomplete.js +++ b/addon/components/g-map/autocomplete.js @@ -17,6 +17,14 @@ export default MapComponent.extend({ _type: 'autocomplete', _ignoredAttrs: ['onSearch'], + init() { + this._super(...arguments); + + this.publicAPI.reopen({ + place: 'place' + }); + }, + _addComponent() { let map = get(this, 'map'); @@ -38,10 +46,10 @@ export default MapComponent.extend({ }, _onSearch() { - const place = this.mapComponent.getPlace(); - const map = get(this, 'map'); - if (place.geometry) { - tryInvoke(this, 'onSearch', [{ place, map }]); + this.place = this.mapComponent.getPlace(); + + if (this.place.geometry) { + tryInvoke(this, 'onSearch', [this.publicAPI]); } } }); diff --git a/addon/components/g-map/circle.js b/addon/components/g-map/circle.js index 51ca5659..b9f39c26 100644 --- a/addon/components/g-map/circle.js +++ b/addon/components/g-map/circle.js @@ -15,7 +15,6 @@ export default Marker.extend({ _requiredOptions: ['center', 'radius'], radius: 500, - center: reads('position'), _addComponent() { diff --git a/addon/components/g-map/info-window.js b/addon/components/g-map/info-window.js index 6bed48a0..571a0f5f 100644 --- a/addon/components/g-map/info-window.js +++ b/addon/components/g-map/info-window.js @@ -26,6 +26,7 @@ export default MapComponent.extend({ init() { this._super(...arguments); + if (!get(this, 'target')) { this._requiredOptions = this._requiredOptions.concat(['position']); } @@ -55,11 +56,14 @@ export default MapComponent.extend({ _addComponent() { this._prepareContent(); + let options = get(this, '_options'); delete options.map; + if (!get(this, 'isOpen')) { delete options.content; } + set(this, 'mapComponent', new google.maps.InfoWindow(options)); }, @@ -67,20 +71,24 @@ export default MapComponent.extend({ if (get(this, 'isOpen')) { this.open(); } + this._super(...arguments); }, _updateComponent() { let options = get(this, '_options'); + if (!get(this, 'isOpen')) { delete options.content; } + this.mapComponent.setOptions(options); }, _prepareContent() { if (!get(this, 'content')) { - const content = document.createElement('div'); + let content = document.createElement('div'); + set(this, '_targetPane', content); set(this, 'content', content); } @@ -91,6 +99,7 @@ export default MapComponent.extend({ google.maps.event.addListenerOnce(this.mapComponent, 'closeclick', () => { set(this, 'isOpen', false); }); + this.mapComponent.open(get(this, 'map'), get(this, 'target')); } }, diff --git a/addon/components/g-map/map-component.js b/addon/components/g-map/map-component.js index a1688006..ac3206f1 100644 --- a/addon/components/g-map/map-component.js +++ b/addon/components/g-map/map-component.js @@ -5,6 +5,7 @@ import PublicAPI from '../../utils/public-api'; import { get, set } from '@ember/object'; import { tryInvoke } from '@ember/utils'; import { defer, resolve } from 'rsvp'; +import { assert } from '@ember/debug'; const MapComponentAPI = { map: 'map', @@ -26,11 +27,14 @@ const MapComponentAPI = { const MapComponent = Component.extend(ProcessOptions, RegisterEvents, { tagName: '', + _type: null, _requiredOptions: ['map'], init() { this._super(...arguments); + assert('You must set a _type property on the map component.', this._type); + this.isInitialized = defer(); this.isInitialized.promise.then(() => set(this, '_isInitialized', true)); @@ -39,17 +43,20 @@ const MapComponent = Component.extend(ProcessOptions, RegisterEvents, { didInsertElement() { this._super(...arguments); + this._internalAPI._registerComponent(this._type, this.publicAPI); this._updateOrAddComponent(); }, didUpdateAttrs() { this._super(...arguments); + this._updateOrAddComponent(); }, willDestroyElement() { this._super(...arguments); + tryInvoke(this.mapComponent, 'setMap', [null]); this._internalAPI._unregisterComponent(this._type, this.publicAPI); }, @@ -60,9 +67,9 @@ const MapComponent = Component.extend(ProcessOptions, RegisterEvents, { if (this._isInitialized) { this._updateComponent(); } else { - (this._addComponent() || resolve()) - .then(() => this._didAddComponent()) - .catch(() => {}); + resolve() + .then(() => this._addComponent()) + .then(() => this._didAddComponent()); } }, @@ -73,7 +80,9 @@ const MapComponent = Component.extend(ProcessOptions, RegisterEvents, { * @method _addComponent * @return */ - _addComponent() {}, + _addComponent() { + assert('Map components must implement the _addComponent hook.'); + }, /** * Run after the map component has been initialized. This hook should be used diff --git a/addon/components/g-map/marker.js b/addon/components/g-map/marker.js index 5a8fd812..3e260b0b 100644 --- a/addon/components/g-map/marker.js +++ b/addon/components/g-map/marker.js @@ -15,22 +15,12 @@ export default MapComponent.extend({ layout, tagName: '', - _requiredOptions: ['position'], - _type: 'marker', + _requiredOptions: ['position'], position, _addComponent() { set(this, 'mapComponent', new google.maps.Marker(get(this, '_options'))); - }, - - /** - * @method getPosition - * @public - * @return {[google.maps.LatLng]} - */ - getPosition() { - return this.mapComponent && this.mapComponent.getPosition(); } }); diff --git a/addon/components/g-map/overlay.js b/addon/components/g-map/overlay.js index 77fb0c08..54daee6d 100644 --- a/addon/components/g-map/overlay.js +++ b/addon/components/g-map/overlay.js @@ -37,7 +37,7 @@ export default MapComponent.extend({ }), _addComponent() { - const Overlay = new google.maps.OverlayView(); + let Overlay = new google.maps.OverlayView(); return new Promise((resolve) => { Overlay.onAdd = () => this.add(); diff --git a/addon/components/g-map/route.js b/addon/components/g-map/route.js index bba5980d..15613097 100644 --- a/addon/components/g-map/route.js +++ b/addon/components/g-map/route.js @@ -19,10 +19,12 @@ export default MapComponent.extend({ _requiredOptions: ['directions'], _addComponent() { - const options = get(this, '_options'); + let options = get(this, '_options'); + if (!options.directions) { return reject(); } + set(this, 'mapComponent', new google.maps.DirectionsRenderer(options)); } }); diff --git a/addon/components/g-map/waypoint.js b/addon/components/g-map/waypoint.js index 25e5328e..d5d1d4df 100644 --- a/addon/components/g-map/waypoint.js +++ b/addon/components/g-map/waypoint.js @@ -19,11 +19,13 @@ export default Component.extend(ProcessOptions, { didReceiveAttrs() { this._super(...arguments); + this._registerWaypoint(get(this, '_options')); }, willDestroyElement() { this._super(...arguments); + this._unregisterWaypoint(get(this, '_options')); } }); diff --git a/addon/mixins/process-options.js b/addon/mixins/process-options.js index e7a45f99..ac9d73db 100644 --- a/addon/mixins/process-options.js +++ b/addon/mixins/process-options.js @@ -16,6 +16,8 @@ function removeObservers(obj, keys, callback) { * @extends Ember.Mixin */ export default Mixin.create({ + concatenatedProperties: ['_requiredOptions', '_watchedOptions', '_ignoredAttrs'], + /** * Specify which attributes on the component should be ignored and never * considered as a Google Maps option or event. @@ -24,9 +26,7 @@ export default Mixin.create({ * @private * @type {String[]} */ - _ignoredAttrs: ['map', '_internalAPI', 'lat', 'lng'], - - concatenatedProperties: ['_requiredOptions', '_watchedOptions', '_ignoredAttrs'], + _ignoredAttrs: ['map', '_internalAPI', 'lat', 'lng', 'events'], /** * Required options that are always included in the options object passed to @@ -57,29 +57,41 @@ export default Mixin.create({ * @return {Object} */ options: computed('attrs', 'events', function() { - let attrs = Object.keys(this.attrs).filter((k) => { - return [...get(this, '_ignoredAttrs'), ...(get(this, 'events') || [])].indexOf(k) === -1; + let { _ignoredAttrs, _eventAttrs } = getProperties(this, '_ignoredAttrs', '_eventAttrs'); + let ignoredAttrs = [..._ignoredAttrs, ..._eventAttrs]; + + let attrs = Object.keys(this.attrs).filter((attr) => { + return ignoredAttrs.indexOf(attr) === -1; }); + return getProperties(this, attrs); }), _options: computed('map', 'options', function() { let options = get(this, 'options'); - let required = getProperties(this, get(this, '_requiredOptions')); + let _requiredOptions = get(this, '_requiredOptions'); + let required = getProperties(this, _requiredOptions); + return assign(required, options); }), init() { this._super(...arguments); - let watched = get(this, '_watchedOptions'); - if (watched.length > 0) { - addObservers(this, watched, this._updateComponent); + if (!this._eventAttrs) { + this._eventAttrs = []; + } + + let _watchedOptions = get(this, '_watchedOptions'); + if (_watchedOptions.length > 0) { + addObservers(this, _watchedOptions, this._updateComponent); } }, willDestroyElement() { - removeObservers(this, get(this, '_watchedOptions'), this._updateComponent); + let _watchedOptions = get(this, '_watchedOptions'); + removeObservers(this, _watchedOptions, this._updateComponent); + this._super(...arguments); } }); diff --git a/addon/mixins/register-events.js b/addon/mixins/register-events.js index 72c735b0..e93c1869 100644 --- a/addon/mixins/register-events.js +++ b/addon/mixins/register-events.js @@ -1,5 +1,5 @@ import Mixin from '@ember/object/mixin'; -import { computed, get } from '@ember/object'; +import { computed, get, getProperties } from '@ember/object'; import { reads } from '@ember/object/computed'; import { decamelize } from '@ember/string'; @@ -29,6 +29,11 @@ export default Mixin.create({ */ _eventTarget: reads('mapComponent'), + _eventAttrs: computed('attrs', function() { + let attrNames = Object.keys(this.attrs); + return attrNames.filter((attr) => this._filterEventsByName(attr)); + }), + /** * Filter the array of passed attributes for attributes that begin with `on`. * @@ -36,9 +41,8 @@ export default Mixin.create({ * @type {Array} * @public */ - events: computed(function() { - const attrs = Object.keys(this.attrs); - return attrs.filter((attr) => this._filterEventsByName(attr)); + events: computed('_eventAttrs', function() { + return getProperties(this, get(this, '_eventAttrs')); }), /** @@ -55,10 +59,12 @@ export default Mixin.create({ }, willDestroyElement() { - const eventTarget = get(this, '_eventTarget'); + let eventTarget = get(this, '_eventTarget'); + if (eventTarget && typeof google !== 'undefined') { google.maps.event.clearInstanceListeners(eventTarget); } + this._super(...arguments); }, @@ -70,24 +76,25 @@ export default Mixin.create({ * @return */ registerEvents() { - const eventsToRegister = get(this, 'events'); - eventsToRegister.forEach((action) => { - const eventName = decamelize(action).slice(3); - const eventTarget = get(this, '_eventTarget'); + let events = get(this, 'events'); + + Object.keys(events).forEach((eventName) => { + let action = events[eventName]; + let eventTarget = get(this, '_eventTarget'); + eventName = decamelize(eventName).slice(3); google.maps.event.addDomListener(eventTarget, eventName, (googleEvent) => { let event = window.event; + let params = { + event, + googleEvent, + eventName, + target: eventTarget, + publicAPI: this.publicAPI, + map: get(this, 'map') + }; - get(this, action)( - { - event, - googleEvent, - eventName, - target: eventTarget, - publicAPI: this.publicAPI, - map: get(this, 'map') - } - ); + action(params); }); }); } From 894c9d2c22c30fb30e7aa88e80f9ad6bf76587ee Mon Sep 17 00:00:00 2001 From: sandydoo Date: Fri, 25 May 2018 18:08:47 +0300 Subject: [PATCH 2/2] Improve event registration internals Allow registering events using both an `events` hash and by passing attributes. --- addon/mixins/register-events.js | 32 +++++++++++++---- tests/integration/components/g-map-test.js | 42 +++++++++++++++++++--- 2 files changed, 63 insertions(+), 11 deletions(-) diff --git a/addon/mixins/register-events.js b/addon/mixins/register-events.js index e93c1869..e53210f3 100644 --- a/addon/mixins/register-events.js +++ b/addon/mixins/register-events.js @@ -2,6 +2,7 @@ import Mixin from '@ember/object/mixin'; import { computed, get, getProperties } from '@ember/object'; import { reads } from '@ember/object/computed'; import { decamelize } from '@ember/string'; +import { assign } from '@ember/polyfills'; /** * Register Google Maps events on any map component. @@ -19,6 +20,12 @@ import { decamelize } from '@ember/string'; * @extends Ember.Mixin */ export default Mixin.create({ + /** + * @property events + * @type {Object} + * @public + */ + /** * The target DOM node or Google Maps object to which to attach event * listeners. @@ -29,20 +36,31 @@ export default Mixin.create({ */ _eventTarget: reads('mapComponent'), + /** + * Filter the array of passed attributes for attributes that begin with `on`. + * + * @property _eventAttrs + * @private + * @return {Array} An array of extracted event names. + */ _eventAttrs: computed('attrs', function() { let attrNames = Object.keys(this.attrs); return attrNames.filter((attr) => this._filterEventsByName(attr)); }), /** - * Filter the array of passed attributes for attributes that begin with `on`. + * A combination of events passed via the `events` hash and extracted from the + * component's attributes. Events registered via an attribute take precedence. * - * @property events - * @type {Array} - * @public + * @property _events + * @private + * @return {Object} */ - events: computed('_eventAttrs', function() { - return getProperties(this, get(this, '_eventAttrs')); + _events: computed('events', '_eventAttrs', function() { + let events = get(this, 'events'); + let extractedEvents = getProperties(this, get(this, '_eventAttrs')); + + return assign({}, events, extractedEvents); }), /** @@ -76,7 +94,7 @@ export default Mixin.create({ * @return */ registerEvents() { - let events = get(this, 'events'); + let events = get(this, '_events'); Object.keys(events).forEach((eventName) => { let action = events[eventName]; diff --git a/tests/integration/components/g-map-test.js b/tests/integration/components/g-map-test.js index e9512fcc..c32639d5 100644 --- a/tests/integration/components/g-map-test.js +++ b/tests/integration/components/g-map-test.js @@ -1,6 +1,6 @@ import { module, test } from 'qunit'; import { setupRenderingTest } from 'ember-qunit'; -import { setupMapTest } from 'ember-google-maps/test-support'; +import { setupMapTest, trigger } from 'ember-google-maps/test-support'; import { setupLocations } from 'dummy/tests/helpers/locations'; import { render } from '@ember/test-helpers'; import hbs from 'htmlbars-inline-precompile'; @@ -20,7 +20,7 @@ module('Integration | Component | g map', function(hooks) { assert.ok(map, 'map initialized'); }); - test('it passes options to the map', async function(assert) { + test('it passes attributes as options to the map', async function(assert) { await render(hbs` {{g-map lat=lat lng=lng zoom=12 zoomControl=false}} `); @@ -30,6 +30,16 @@ module('Integration | Component | g map', function(hooks) { assert.notOk(map.zoomControl, 'zoom control disabled'); }); + test('it accepts an options hash', async function(assert) { + await render(hbs` + {{g-map lat=lat lng=lng options=(hash zoom=12 zoomControl=false)}} + `); + + let { map } = this.gMapAPI; + + assert.notOk(map.zoomControl, 'zoom control disabled'); + }); + test('it updates the map when attributes are changed', async function(assert) { this.zoom = 12; @@ -46,11 +56,11 @@ module('Integration | Component | g map', function(hooks) { assert.equal(map.zoom, this.zoom); }); - test('it binds events to the map', async function(assert) { + test('it extracts events from attributes and binds them to the map', async function(assert) { assert.expect(1); this.onZoomChanged = ({ eventName }) => { - assert.equal(eventName, 'zoom_changed', 'zoom changed'); + assert.equal(eventName, 'zoom_changed', 'zoom changed event'); }; await render(hbs` @@ -61,4 +71,28 @@ module('Integration | Component | g map', function(hooks) { map.setZoom(10); }); + + test('it accepts both an events hash and individual attribute events', async function(assert) { + assert.expect(2); + + this.onClick = ({ eventName }) => { + assert.equal(eventName, 'click', 'click attribute event'); + }; + + this.onZoomChanged = ({ eventName }) => { + assert.equal(eventName, 'zoom_changed', 'zoom changed event from events hash'); + }; + + await render(hbs` + {{g-map lat=lat lng=lng zoom=12 + onClick=(action onClick) + events=(hash onZoomChanged=(action onZoomChanged))}} + `); + + let { map } = this.gMapAPI; + + trigger(map, 'click'); + + map.setZoom(10); + }); });