Skip to content

refactor(google-maps): Removes reference to GoogleMap from MapMarker #17222

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 1, 2019
Merged

refactor(google-maps): Removes reference to GoogleMap from MapMarker #17222

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5276be1
refactor(google-maps): Removes reference to GoogleMap from MapMarker
mbehrlich Sep 28, 2019
3efdf78
refactor(google-maps): Removes reference to GoogleMap from MapMarker
mbehrlich Oct 1, 2019
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
14 changes: 0 additions & 14 deletions src/google-maps/google-map/google-map.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import {async, TestBed} from '@angular/core/testing';
import {By} from '@angular/platform-browser';

import {GoogleMapsModule} from '../google-maps-module';
import {MapMarker} from '../map-marker/map-marker';
import {
createMapConstructorSpy,
createMapSpy,
Expand Down Expand Up @@ -240,18 +239,6 @@ describe('GoogleMap', () => {
expect(mapSpy.addListener).not.toHaveBeenCalledWith('tilt_changed', jasmine.any(Function));
expect(mapSpy.addListener).not.toHaveBeenCalledWith('zoom_changed', jasmine.any(Function));
});

it('calls setMap on child marker components', () => {
mapSpy = createMapSpy(DEFAULT_OPTIONS);
createMapConstructorSpy(mapSpy).and.callThrough();

const fixture = TestBed.createComponent(TestApp);
const markerComponent = fixture.debugElement.query(By.directive(MapMarker)).componentInstance;
spyOn(markerComponent, '_setMap').and.callThrough();
fixture.detectChanges();

expect(markerComponent._setMap).toHaveBeenCalledWith(mapSpy);
});
});

@Component({
Expand All @@ -264,7 +251,6 @@ describe('GoogleMap', () => {
(mapClick)="handleClick($event)"
(centerChanged)="handleCenterChanged()"
(mapRightclick)="handleRightclick($event)">
<map-marker></map-marker>
</google-map>`,
})
class TestApp {
Expand Down
26 changes: 1 addition & 25 deletions src/google-maps/google-map/google-map.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,20 @@
*/

import {
AfterContentInit,
ChangeDetectionStrategy,
Component,
ContentChildren,
ElementRef,
EventEmitter,
Input,
OnChanges,
OnDestroy,
OnInit,
Output,
QueryList,
ViewEncapsulation,
} from '@angular/core';
import {BehaviorSubject, combineLatest, Observable, Subject} from 'rxjs';
import {map, shareReplay, take, takeUntil} from 'rxjs/operators';

import {MapMarker} from '../map-marker/map-marker';

interface GoogleMapsWindow extends Window {
google?: typeof google;
}
Expand Down Expand Up @@ -62,7 +57,7 @@ export const DEFAULT_WIDTH = '500px';
template: '<div class="map-container"></div><ng-content></ng-content>',
encapsulation: ViewEncapsulation.None,
})
export class GoogleMap implements OnChanges, OnInit, AfterContentInit, OnDestroy {
export class GoogleMap implements OnChanges, OnInit, OnDestroy {
@Input() height = DEFAULT_HEIGHT;

@Input() width = DEFAULT_WIDTH;
Expand Down Expand Up @@ -188,8 +183,6 @@ export class GoogleMap implements OnChanges, OnInit, AfterContentInit, OnDestroy
*/
@Output() zoomChanged = new EventEmitter<void>();

@ContentChildren(MapMarker) _markers: QueryList<MapMarker>;

private _mapEl: HTMLElement;
_googleMap!: UpdatedGoogleMap;

Expand Down Expand Up @@ -234,13 +227,6 @@ export class GoogleMap implements OnChanges, OnInit, AfterContentInit, OnDestroy
this._watchForOptionsChanges(combinedOptionsChanges);
}

ngAfterContentInit() {
for (const marker of this._markers.toArray()) {
marker._setMap(this._googleMap);
}
this._watchForMarkerChanges();
}

ngOnDestroy() {
this._destroy.next();
this._destroy.complete();
Expand Down Expand Up @@ -472,14 +458,4 @@ export class GoogleMap implements OnChanges, OnInit, AfterContentInit, OnDestroy
}));
}
}

private _watchForMarkerChanges() {
combineLatest(this._googleMapChanges, this._markers.changes)
.pipe(takeUntil(this._destroy))
.subscribe(([googleMap, markers]) => {
for (let marker of markers) {
marker._setMap(googleMap);
}
});
}
}
72 changes: 22 additions & 50 deletions src/google-maps/map-marker/map-marker.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ import {Component} from '@angular/core';
import {async, TestBed} from '@angular/core/testing';
import {By} from '@angular/platform-browser';

import {DEFAULT_OPTIONS, UpdatedGoogleMap} from '../google-map/google-map';
import {
createMapConstructorSpy,
createMapSpy,
createMarkerConstructorSpy,
createMarkerSpy,
TestingWindow
Expand All @@ -12,6 +15,8 @@ import {GoogleMapsModule} from '../google-maps-module';
import {DEFAULT_MARKER_OPTIONS, MapMarker} from './map-marker';

describe('MapMarker', () => {
let mapSpy: jasmine.SpyObj<UpdatedGoogleMap>;

beforeEach(async(() => {
TestBed.configureTestingModule({
imports: [GoogleMapsModule],
Expand All @@ -21,6 +26,9 @@ describe('MapMarker', () => {

beforeEach(() => {
TestBed.compileComponents();

mapSpy = createMapSpy(DEFAULT_OPTIONS);
createMapConstructorSpy(mapSpy).and.callThrough();
});

afterEach(() => {
Expand All @@ -33,28 +41,24 @@ describe('MapMarker', () => {
const markerConstructorSpy = createMarkerConstructorSpy(markerSpy).and.callThrough();

const fixture = TestBed.createComponent(TestApp);
const fakeMap = {} as unknown as google.maps.Map;
const markerComponent = fixture.debugElement.query(By.directive(MapMarker)).componentInstance;
markerComponent._setMap(fakeMap);
fixture.detectChanges();

expect(markerConstructorSpy).toHaveBeenCalledWith({
...DEFAULT_MARKER_OPTIONS,
title: undefined,
label: undefined,
clickable: undefined,
map: fakeMap
map: mapSpy,
});
});

it('sets marker inputs', () => {
const fakeMap = {} as unknown as google.maps.Map;
const options: google.maps.MarkerOptions = {
position: {lat: 3, lng: 5},
title: 'marker title',
label: 'marker label',
clickable: false,
map: fakeMap,
map: mapSpy,
};
const markerSpy = createMarkerSpy(options);
const markerConstructorSpy = createMarkerConstructorSpy(markerSpy).and.callThrough();
Expand All @@ -64,15 +68,12 @@ describe('MapMarker', () => {
fixture.componentInstance.title = options.title;
fixture.componentInstance.label = options.label;
fixture.componentInstance.clickable = options.clickable;
const markerComponent = fixture.debugElement.query(By.directive(MapMarker)).componentInstance;
markerComponent._setMap(fakeMap);
fixture.detectChanges();

expect(markerConstructorSpy).toHaveBeenCalledWith(options);
});

it('sets marker options, ignoring map', () => {
const fakeMap = {} as unknown as google.maps.Map;
const options: google.maps.MarkerOptions = {
position: {lat: 3, lng: 5},
title: 'marker title',
Expand All @@ -85,15 +86,12 @@ describe('MapMarker', () => {

const fixture = TestBed.createComponent(TestApp);
fixture.componentInstance.options = options;
const markerComponent = fixture.debugElement.query(By.directive(MapMarker)).componentInstance;
markerComponent._setMap(fakeMap);
fixture.detectChanges();

expect(markerConstructorSpy).toHaveBeenCalledWith({...options, map: fakeMap});
expect(markerConstructorSpy).toHaveBeenCalledWith({...options, map: mapSpy});
});

it('gives precedence to specific inputs over options', () => {
const fakeMap = {} as unknown as google.maps.Map;
const options: google.maps.MarkerOptions = {
position: {lat: 3, lng: 5},
title: 'marker title',
Expand All @@ -107,7 +105,7 @@ describe('MapMarker', () => {
label: 'updated label',
clickable: true,
icon: 'icon name',
map: fakeMap,
map: mapSpy,
};
const markerSpy = createMarkerSpy(options);
const markerConstructorSpy = createMarkerConstructorSpy(markerSpy).and.callThrough();
Expand All @@ -118,43 +116,17 @@ describe('MapMarker', () => {
fixture.componentInstance.label = expectedOptions.label;
fixture.componentInstance.clickable = expectedOptions.clickable;
fixture.componentInstance.options = options;
const markerComponent = fixture.debugElement.query(By.directive(MapMarker)).componentInstance;
markerComponent._setMap(fakeMap);
fixture.detectChanges();

expect(markerConstructorSpy).toHaveBeenCalledWith(expectedOptions);
});

it('sets the map on the marker only once', () => {
const markerSpy = createMarkerSpy(DEFAULT_MARKER_OPTIONS);
const markerConstructorSpy = createMarkerConstructorSpy(markerSpy).and.callThrough();

const fixture = TestBed.createComponent(TestApp);
const fakeMap = {} as unknown as google.maps.Map;
const fakeMap2 = {testValue: 'test'} as unknown as google.maps.Map;
const markerComponent = fixture.debugElement.query(By.directive(MapMarker)).componentInstance;
markerComponent._setMap(fakeMap);
markerComponent._setMap(fakeMap2);
fixture.detectChanges();

expect(markerConstructorSpy).toHaveBeenCalledWith({
...DEFAULT_MARKER_OPTIONS,
title: undefined,
label: undefined,
clickable: undefined,
map: fakeMap
});
expect(markerSpy.setOptions).not.toHaveBeenCalled();
});

it('exposes methods that provide information about the marker', () => {
const markerSpy = createMarkerSpy(DEFAULT_MARKER_OPTIONS);
createMarkerConstructorSpy(markerSpy).and.callThrough();

const fixture = TestBed.createComponent(TestApp);
const fakeMap = {} as unknown as google.maps.Map;
const markerComponent = fixture.debugElement.query(By.directive(MapMarker)).componentInstance;
markerComponent._setMap(fakeMap);
fixture.detectChanges();

markerSpy.getAnimation.and.returnValue(null);
Expand Down Expand Up @@ -199,9 +171,6 @@ describe('MapMarker', () => {
createMarkerConstructorSpy(markerSpy).and.callThrough();

const fixture = TestBed.createComponent(TestApp);
const fakeMap = {} as unknown as google.maps.Map;
const markerComponent = fixture.debugElement.query(By.directive(MapMarker)).componentInstance;
markerComponent._setMap(fakeMap);
fixture.detectChanges();

expect(markerSpy.addListener)
Expand Down Expand Up @@ -234,13 +203,16 @@ describe('MapMarker', () => {

@Component({
selector: 'test-app',
template: `<map-marker [title]="title"
[position]="position"
[label]="label"
[clickable]="clickable"
[options]="options"
(mapClick)="handleClick()"
(positionChanged)="handlePositionChanged()"></map-marker>`,
template: `<google-map>
<map-marker [title]="title"
[position]="position"
[label]="label"
[clickable]="clickable"
[options]="options"
(mapClick)="handleClick()"
(positionChanged)="handlePositionChanged()">
</map-marker>
</google-map>`,
})
class TestApp {
title?: string;
Expand Down
44 changes: 17 additions & 27 deletions src/google-maps/map-marker/map-marker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@ import {
Output,
ViewEncapsulation
} from '@angular/core';
import {BehaviorSubject, combineLatest, Observable, ReplaySubject, Subject} from 'rxjs';
import {BehaviorSubject, combineLatest, Observable, Subject} from 'rxjs';
import {map, takeUntil} from 'rxjs/operators';

import {GoogleMap} from '../google-map/google-map';

/**
* Default options for the Google Maps marker component. Displays a marker
* at the Googleplex.
Expand Down Expand Up @@ -198,30 +200,26 @@ export class MapMarker implements OnInit, OnDestroy {
new BehaviorSubject<string|google.maps.MarkerLabel|undefined>(undefined);
private readonly _clickable = new BehaviorSubject<boolean|undefined>(undefined);

private readonly _map = new ReplaySubject<google.maps.Map>(1);

private readonly _destroy = new Subject<void>();

private readonly _listeners: google.maps.MapsEventListener[] = [];

private _hasMap = false;

_marker?: google.maps.Marker;

constructor(private readonly googleMap: GoogleMap) {}

ngOnInit() {
const combinedOptionsChanges = this._combineOptions();

combineLatest(this._map, combinedOptionsChanges)
.pipe(takeUntil(this._destroy))
.subscribe(([googleMap, options]) => {
if (this._marker) {
this._marker.setOptions(options);
} else {
this._marker = new google.maps.Marker(options);
this._marker.setMap(googleMap);
this._initializeEventHandlers();
}
});
combinedOptionsChanges.pipe(takeUntil(this._destroy)).subscribe(options => {
if (this._marker) {
this._marker.setOptions(options);
} else {
this._marker = new google.maps.Marker(options);
this._marker.setMap(this.googleMap._googleMap);
this._initializeEventHandlers();
}
});
}

ngOnDestroy() {
Expand All @@ -235,13 +233,6 @@ export class MapMarker implements OnInit, OnDestroy {
}
}

_setMap(googleMap: google.maps.Map) {
if (!this._hasMap) {
this._map.next(googleMap);
this._hasMap = true;
}
}

/**
* See
* developers.google.com/maps/documentation/javascript/reference/marker#Marker.getAnimation
Expand Down Expand Up @@ -339,16 +330,15 @@ export class MapMarker implements OnInit, OnDestroy {
}

private _combineOptions(): Observable<google.maps.MarkerOptions> {
return combineLatest(
this._options, this._title, this._position, this._label, this._clickable, this._map)
.pipe(map(([options, title, position, label, clickable, googleMap]) => {
return combineLatest(this._options, this._title, this._position, this._label, this._clickable)
.pipe(map(([options, title, position, label, clickable]) => {
const combinedOptions: google.maps.MarkerOptions = {
...options,
title: title || options.title,
position: position || options.position,
label: label || options.label,
clickable: clickable !== undefined ? clickable : options.clickable,
map: googleMap || null,
map: this.googleMap._googleMap || null,
};
return combinedOptions;
}));
Expand Down
14 changes: 9 additions & 5 deletions src/google-maps/testing/fake-google-map-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,15 @@ export function createMarkerConstructorSpy(markerSpy: jasmine.SpyObj<google.maps
return markerSpy;
});
const testingWindow: TestingWindow = window;
testingWindow.google = {
maps: {
'Marker': markerConstructorSpy,
},
};
if (testingWindow.google && testingWindow.google.maps) {
testingWindow.google.maps['Marker'] = markerConstructorSpy;
} else {
testingWindow.google = {
maps: {
'Marker': markerConstructorSpy,
},
};
}
return markerConstructorSpy;
}

Expand Down