Skip to content

Commit

Permalink
fix: restructured and improved addListener typings (#23)
Browse files Browse the repository at this point in the history
  • Loading branch information
@usefulthink
usefulthink authored Jan 19, 2023
1 parent 604b99e commit 0e02b40
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 87 deletions.
21 changes: 10 additions & 11 deletions src/computed-marker-attributes.ts
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
import {toLatLng} from './position-formats';
import {
AttributeKey,
attributeKeys,
Attributes,
Position
} from './marker-attributes';
import {AttributeKey, attributeKeys} from './marker-attributes';

import type {Marker} from './marker';
import type {StaticAttributes} from './marker-attributes';
Expand All @@ -16,7 +11,7 @@ import type {StaticAttributes} from './marker-attributes';
export class ComputedMarkerAttributes<T = unknown>
implements Partial<StaticAttributes>
{
private marker_: Marker<T>;
private readonly marker_: Marker<T>;
private callbackDepth_: number = 0;

// Attributes are declaration-only, the implementataion uses dynamic
Expand All @@ -42,6 +37,12 @@ export class ComputedMarkerAttributes<T = unknown>
this.marker_ = marker;
}

/**
* Resolves the specified attribute into a static value, calling a dynamic
* attribute function.
*
* @param key
*/
private getComputedAttributeValue<TKey extends AttributeKey>(
key: TKey
): StaticAttributes[TKey] | undefined {
Expand Down Expand Up @@ -78,11 +79,9 @@ export class ComputedMarkerAttributes<T = unknown>
}
});

// position is treated separately, so it is skipped here
for (const key of attributeKeys) {
if (key === 'position') {
continue;
}
// position is treated separately, so it is skipped here
if (key === 'position') continue;

Object.defineProperty(this.prototype, key, {
get(this: ComputedMarkerAttributes) {
Expand Down
165 changes: 89 additions & 76 deletions src/marker.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {
darken,
brighten,
darken,
luminance,
parseCssColorValue,
rgbaToString
Expand All @@ -11,8 +11,8 @@ import {ComputedMarkerAttributes} from './computed-marker-attributes';
import {attributeKeys} from './marker-attributes';

import type {IconProvider} from './icons';
import type {Attributes} from './marker-attributes';
import type {MapState} from './map-state-observer';
import type {Attributes, StaticAttributes} from './marker-attributes';

/**
* The Marker class. The optional type-parameter T can be used to specify a type
Expand Down Expand Up @@ -60,7 +60,7 @@ export class Marker<TUserData = unknown> {

private map_: google.maps.Map | null = null;
private mapObserver_: MapStateObserver | null = null;
private mapEventListener_: google.maps.MapsEventListener | null = null;
private mapEventListeners_: google.maps.MapsEventListener[] = [];

private data_: TUserData | null = null;
private markerState_: MarkerState = {visible: false, hovered: false};
Expand Down Expand Up @@ -101,8 +101,6 @@ export class Marker<TUserData = unknown> {
this.markerView_ = new google.maps.marker.AdvancedMarkerView();
this.markerView_.content = this.pinView_.element;

this.bindMarkerEvents();

if (data) this.data_ = data;

// set all remaining parameters as attributes
Expand All @@ -120,13 +118,19 @@ export class Marker<TUserData = unknown> {
* event system, while any dom-events will be added to the marker-element
* itself.
*
* - FIXME: normalize event-handler-parameters
* - FIXME: extend the typings to be explicit about the callback-parameters
*
* @param eventName 'click', 'dragstart', 'dragend', 'drag' or any DOM
* event-name.
* @param handler
*/
addListener<K extends keyof GoogleMapsAMVEventMap>(
eventName: K,
handler: (ev: GoogleMapsAMVEventMap[K]) => void
): google.maps.MapsEventListener;
addListener<K extends keyof HTMLElementEventMap>(
eventName: K,
handler: (ev: HTMLElementEventMap[K]) => void
): google.maps.MapsEventListener;

addListener(
eventName: string,
handler: ((ev: google.maps.MapMouseEvent) => void) | ((ev: Event) => void)
Expand Down Expand Up @@ -155,8 +159,8 @@ export class Marker<TUserData = unknown> {
}

/**
* The map property is a proxy for this.map_, setting the map will also start
* listening for map-state events and update the marker.
* Stores the map-instance. The map will be passed on to the
* AdvancedMarkerView in `performUpdate()`.
*/
get map(): google.maps.Map | null {
return this.map_ || null;
Expand All @@ -167,24 +171,16 @@ export class Marker<TUserData = unknown> {
return;
}

if (this.mapEventListener_) {
this.mapEventListener_.remove();
this.mapEventListener_ = null;
this.mapObserver_ = null;
}

this.unbindEvents_();
this.mapObserver_ = null;
this.map_ = map;

if (map) {
this.mapObserver_ = MapStateObserver.getInstance(map);
this.mapEventListener_ = this.mapObserver_.addListener(() =>
this.update()
);

this.update();
} else {
this.markerView_.map = null;
this.bindEvents_();
}

this.update();
}

/**
Expand Down Expand Up @@ -234,12 +230,13 @@ export class Marker<TUserData = unknown> {
* @internal
*/
private performUpdate() {
if (!this.map || !this.mapObserver_) {
console.warn('marker update skipped: missing map');
if (!this.map) {
this.markerView_.map = null;

return;
}

const attrs = this.computedAttributes_;
const attrs = this.computedAttributes_ as ComputedMarkerAttributes;
const position = attrs.position;

// if the marker doesn't have a position, we can skip it entirely and
Expand All @@ -254,44 +251,15 @@ export class Marker<TUserData = unknown> {
this.markerView_.map = this.map_;
}

this.updateColors(attrs);

// FIXME: in cases where there's an `if` here, we need to make sure that
// state updates might require us to reset the state (i.e. icon changes
// from 'some-icon' to undefined).

this.markerView_.position = position;
this.markerView_.draggable = attrs.draggable;
this.markerView_.title = attrs.title;
this.markerView_.zIndex = attrs.zIndex;
this.markerView_.collisionBehavior = attrs.collisionBehavior;
this.pinView_.scale = attrs.scale;

if (attrs.icon) {
let namespace = 'default';
let iconId = attrs.icon;

if (attrs.icon.includes(':')) {
[namespace, iconId] = attrs.icon.split(':');
}

const provider = Marker.iconProviders.get(namespace);
if (provider) {
this.pinView_.glyph = provider(iconId);
} else {
const nsText =
namespace === 'default' ? '' : `with namespace '${namespace}' `;

warnOnce(
`An icon is set but no icon provider ${nsText}is configured.\n` +
`You can register an icon-provider using e.g. ` +
`\`Marker.iconProvider = MaterialIcons()\` to use the material ` +
`icons webfont.`
);
}
} else if (attrs.glyph) {
this.pinView_.glyph = attrs.glyph;
}
this.updateColors_(attrs);
this.updateGlyph_(attrs);
}

/**
Expand All @@ -300,7 +268,7 @@ export class Marker<TUserData = unknown> {
*
* @param attributes
*/
private updateColors(attributes: Partial<StaticAttributes>) {
private updateColors_(attributes: ComputedMarkerAttributes) {
let {color, backgroundColor, borderColor, glyphColor} = attributes;

if (color) {
Expand All @@ -321,23 +289,63 @@ export class Marker<TUserData = unknown> {
this.pinView_.glyphColor = glyphColor;
}

/** Binds the required dom-events to the marker-instance. */
private bindMarkerEvents = () => {
// fixme: do we want those to be always bound?
// a) add/remove listeners when the marker is added to the map?
// b) should there be a property to control wether we have these
// events at all?
/**
* Updates the pinview glyph based on the `icon` and `glyph` attributes.
*
* @param attrs
*/
private updateGlyph_(attrs: ComputedMarkerAttributes) {
if (!attrs.icon) {
this.pinView_.glyph = attrs.glyph || undefined;

this.addListener('pointerenter', () => {
this.markerState_.hovered = true;
this.update();
});
return;
}

this.addListener('pointerleave', () => {
this.markerState_.hovered = false;
this.update();
});
};
let namespace = 'default';
let iconId = attrs.icon;

if (attrs.icon.includes(':')) {
[namespace, iconId] = attrs.icon.split(':');
}

const provider = Marker.iconProviders.get(namespace);
if (provider) {
this.pinView_.glyph = provider(iconId);
} else {
const nsText =
namespace === 'default' ? '' : `with namespace '${namespace}' `;

warnOnce(
`An icon is set but no icon provider ${nsText}is configured.\n` +
`You can register an icon-provider using e.g. ` +
`\`Marker.iconProvider = MaterialIcons()\` to use the material ` +
`icons webfont.`
);
}
}

/** Binds the required dom- and map-events to the marker-instance. */
private bindEvents_() {
assertNotNull(this.mapObserver_);

this.mapEventListeners_ = [
this.mapObserver_.addListener(() => this.update()),
this.addListener('pointerenter', () => {
this.markerState_.hovered = true;
this.update();
}),
this.addListener('pointerleave', () => {
this.markerState_.hovered = false;
this.update();
})
];
}

/** Unbinds all event listeners from the marker. */
private unbindEvents_() {
for (const listener of this.mapEventListeners_) listener.remove();
this.mapEventListeners_ = [];
}

/**
* Retrieve the parameters to be passed to dynamic attribute callbacks. This
Expand All @@ -352,11 +360,9 @@ export class Marker<TUserData = unknown> {
} {
assertNotNull(this.mapObserver_, 'this.mapObserver_ is not defined');

const mapState = this.mapObserver_.getMapState();

return {
data: this.data_,
map: mapState,
map: this.mapObserver_.getMapState(),
marker: this.markerState_
};
}
Expand Down Expand Up @@ -428,3 +434,10 @@ enum MarkerEvents {
drag = 'drag',
dragend = 'dragend'
}

interface GoogleMapsAMVEventMap {
click: google.maps.MapMouseEvent;
dragstart: google.maps.MapMouseEvent;
drag: google.maps.MapMouseEvent;
dragend: google.maps.MapMouseEvent;
}

0 comments on commit 0e02b40

Please sign in to comment.