Skip to content

Commit

Permalink
minor #2406 [Map] Fix and improve TypeScript types, refactor same log…
Browse files Browse the repository at this point in the history 
…ic into dedicated methods (to reduce file size) (Kocal)

This PR was squashed before being merged into the 2.x branch.

Discussion
----------

[Map] Fix and improve TypeScript types, refactor same logic into dedicated methods (to reduce file size)

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Issues        | Fix #... <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT

<!--
Replace this notice by a description of your feature/bugfix.
This will help reviewers and should be a good start for the documentation.

Additionally (see https://symfony.com/releases):
 - Always add tests and ensure they pass.
 - For new features, provide some code snippets to help understand usage.
 - Features and deprecations must be submitted against branch main.
 - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry
 - Never break backward compatibility (see https://symfony.com/bc).
-->

This PRs is purely internal, and aims to:
- Making TypeScript happy by fixing types definitions and usages, but also simplify them
- Refactoring methods containing the same logic (ex: `this.create*` or `this....ValueChanged`) methods) to dedicated methods:
```js
    createMarker(definition) {
        this.dispatchEvent('marker:before-create', { definition });
        const marker = this.doCreateMarker(definition);
        this.dispatchEvent('marker:after-create', { marker });
        marker['`@id`'] = definition['`@id`'];
        this.markers.set(definition['`@id`'], marker);
        return marker;
    }
    createPolygon(definition) {
        this.dispatchEvent('polygon:before-create', { definition });
        const polygon = this.doCreatePolygon(definition);
        this.dispatchEvent('polygon:after-create', { polygon });
        polygon['`@id`'] = definition['`@id`'];
        this.polygons.set(definition['`@id`'], polygon);
        return polygon;
    }
    createPolyline(definition) {
        this.dispatchEvent('polyline:before-create', { definition });
        const polyline = this.doCreatePolyline(definition);
        this.dispatchEvent('polyline:after-create', { polyline });
        polyline['`@id`'] = definition['`@id`'];
        this.polylines.set(definition['`@id`'], polyline);
        return polyline;
    }
```
becomes
```js
        this.createMarker = this.createDrawingFactory('marker', this.markers, this.doCreateMarker.bind(this));
        this.createPolygon = this.createDrawingFactory('polygon', this.polygons, this.doCreatePolygon.bind(this));
        this.createPolyline = this.createDrawingFactory('polyline', this.polylines, this.doCreatePolyline.bind(this));
```

and
```js
    markersValueChanged() {
        if (!this.map) {
            return;
        }
        this.markers.forEach((marker) => {
            if (!this.markersValue.find((m) => m['`@id`'] === marker['`@id`'])) {
                this.removeMarker(marker);
                this.markers.delete(marker['`@id`']);
            }
        });
        this.markersValue.forEach((marker) => {
            if (!this.markers.has(marker['`@id`'])) {
                this.createMarker(marker);
            }
        });
        if (this.fitBoundsToMarkersValue) {
            this.doFitBoundsToMarkers();
        }
    }
    polygonsValueChanged() {
        if (!this.map) {
            return;
        }
        this.polygons.forEach((polygon) => {
            if (!this.polygonsValue.find((p) => p['`@id`'] === polygon['`@id`'])) {
                this.removePolygon(polygon);
                this.polygons.delete(polygon['`@id`']);
            }
        });
        this.polygonsValue.forEach((polygon) => {
            if (!this.polygons.has(polygon['`@id`'])) {
                this.createPolygon(polygon);
            }
        });
    }
    polylinesValueChanged() {
        if (!this.map) {
            return;
        }
        this.polylines.forEach((polyline) => {
            if (!this.polylinesValue.find((p) => p['`@id`'] === polyline['`@id`'])) {
                this.removePolyline(polyline);
                this.polylines.delete(polyline['`@id`']);
            }
        });
        this.polylinesValue.forEach((polyline) => {
            if (!this.polylines.has(polyline['`@id`'])) {
                this.createPolyline(polyline);
            }
        });
    }
```
becomes
```js
markersValueChanged() {
        if (!this.isConnected) {
            return;
        }
        this.onDrawChanged(this.markers, this.markersValue, this.createMarker, this.doRemoveMarker);
        if (this.fitBoundsToMarkersValue) {
            this.doFitBoundsToMarkers();
        }
    }
    polygonsValueChanged() {
        if (!this.isConnected) {
            return;
        }
        this.onDrawChanged(this.polygons, this.polygonsValue, this.createPolygon, this.doRemovePolygon);
    }
    polylinesValueChanged() {
        if (!this.isConnected) {
            return;
        }
        this.onDrawChanged(this.polylines, this.polylinesValue, this.createPolyline, this.doRemovePolyline);
    }
```

Commits
-------

1d33a5f [Map] Create "onDrawChanged" to refactor methods "markersValueChanged"/"polygonsValueChanged"/"polylinesValueChanged" (not identical but follow the same pattern)
d9d9ff9 [Map] Create "createDrawingFactory" to refactor methods "createMarker"/"createPolyline"/"createPolygon" (not identical but follow the same pattern)
a443ad8 [Map] Re-organize methods order, simplify types, don't dirty-append ``@id`` anymore
  • Loading branch information
@Kocal
Kocal committed Nov 28, 2024
2 parents dadc81b + 1d33a5f commit 5974bfc
Show file tree
Hide file tree
Showing 10 changed files with 576 additions and 566 deletions.
97 changes: 48 additions & 49 deletions src/Map/assets/dist/abstract_map_controller.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,31 @@ export type Point = {
lat: number;
lng: number;
};
export type MarkerDefinition<MarkerOptions, InfoWindowOptions> = {
'@id': string;
export type Identifier = string;
export type WithIdentifier<T extends Record<string, unknown>> = T & {
'@id': Identifier;
};
export type MarkerDefinition<MarkerOptions, InfoWindowOptions> = WithIdentifier<{
position: Point;
title: string | null;
infoWindow?: Omit<InfoWindowDefinition<InfoWindowOptions>, 'position'>;
infoWindow?: InfoWindowWithoutPositionDefinition<InfoWindowOptions>;
rawOptions?: MarkerOptions;
extra: Record<string, unknown>;
};
export type PolygonDefinition<PolygonOptions, InfoWindowOptions> = {
'@id': string;
infoWindow?: Omit<InfoWindowDefinition<InfoWindowOptions>, 'position'>;
}>;
export type PolygonDefinition<PolygonOptions, InfoWindowOptions> = WithIdentifier<{
infoWindow?: InfoWindowWithoutPositionDefinition<InfoWindowOptions>;
points: Array<Point>;
title: string | null;
rawOptions?: PolygonOptions;
extra: Record<string, unknown>;
};
export type PolylineDefinition<PolylineOptions, InfoWindowOptions> = {
'@id': string;
infoWindow?: Omit<InfoWindowDefinition<InfoWindowOptions>, 'position'>;
}>;
export type PolylineDefinition<PolylineOptions, InfoWindowOptions> = WithIdentifier<{
infoWindow?: InfoWindowWithoutPositionDefinition<InfoWindowOptions>;
points: Array<Point>;
title: string | null;
rawOptions?: PolylineOptions;
extra: Record<string, unknown>;
};
}>;
export type InfoWindowDefinition<InfoWindowOptions> = {
headerContent: string | null;
content: string | null;
Expand All @@ -36,6 +37,7 @@ export type InfoWindowDefinition<InfoWindowOptions> = {
rawOptions?: InfoWindowOptions;
extra: Record<string, unknown>;
};
export type InfoWindowWithoutPositionDefinition<InfoWindowOptions> = Omit<InfoWindowDefinition<InfoWindowOptions>, 'position'>;
export default abstract class<MapOptions, Map, MarkerOptions, Marker, InfoWindowOptions, InfoWindow, PolygonOptions, Polygon, PolylineOptions, Polyline> extends Controller<HTMLElement> {
static values: {
providerOptions: ObjectConstructor;
Expand All @@ -55,50 +57,47 @@ export default abstract class<MapOptions, Map, MarkerOptions, Marker, InfoWindow
polylinesValue: Array<PolylineDefinition<PolylineOptions, InfoWindowOptions>>;
optionsValue: MapOptions;
protected map: Map;
protected markers: globalThis.Map<any, any>;
protected markers: globalThis.Map<string, Marker>;
protected polygons: globalThis.Map<string, Polygon>;
protected polylines: globalThis.Map<string, Polyline>;
protected infoWindows: Array<InfoWindow>;
protected polygons: globalThis.Map<any, any>;
protected polylines: globalThis.Map<any, any>;
private isConnected;
private createMarker;
private createPolygon;
private createPolyline;
protected abstract dispatchEvent(name: string, payload: Record<string, unknown>): void;
connect(): void;
createInfoWindow({ definition, element, }: {
definition: InfoWindowWithoutPositionDefinition<InfoWindowOptions>;
element: Marker | Polygon | Polyline;
}): InfoWindow;
abstract centerValueChanged(): void;
abstract zoomValueChanged(): void;
markersValueChanged(): void;
polygonsValueChanged(): void;
polylinesValueChanged(): void;
protected abstract doCreateMap({ center, zoom, options, }: {
center: Point | null;
zoom: number | null;
options: MapOptions;
}): Map;
createMarker(definition: MarkerDefinition<MarkerOptions, InfoWindowOptions>): Marker;
protected abstract removeMarker(marker: Marker): void;
protected abstract doCreateMarker(definition: MarkerDefinition<MarkerOptions, InfoWindowOptions>): Marker;
createPolygon(definition: PolygonDefinition<PolygonOptions, InfoWindowOptions>): Polygon;
protected abstract removePolygon(polygon: Polygon): void;
protected abstract doCreatePolygon(definition: PolygonDefinition<PolygonOptions, InfoWindowOptions>): Polygon;
createPolyline(definition: PolylineDefinition<PolylineOptions, InfoWindowOptions>): Polyline;
protected abstract removePolyline(polyline: Polyline): void;
protected abstract doCreatePolyline(definition: PolylineDefinition<PolylineOptions, InfoWindowOptions>): Polyline;
protected createInfoWindow({ definition, element, }: {
definition: MarkerDefinition<MarkerOptions, InfoWindowOptions>['infoWindow'];
element: Marker;
} | {
definition: PolygonDefinition<PolygonOptions, InfoWindowOptions>['infoWindow'];
element: Polygon;
} | {
definition: PolylineDefinition<PolylineOptions, InfoWindowOptions>['infoWindow'];
element: Polyline;
}): InfoWindow;
protected abstract doFitBoundsToMarkers(): void;
protected abstract doCreateMarker({ definition, }: {
definition: MarkerDefinition<MarkerOptions, InfoWindowOptions>;
}): Marker;
protected abstract doRemoveMarker(marker: Marker): void;
protected abstract doCreatePolygon({ definition, }: {
definition: PolygonDefinition<PolygonOptions, InfoWindowOptions>;
}): Polygon;
protected abstract doRemovePolygon(polygon: Polygon): void;
protected abstract doCreatePolyline({ definition, }: {
definition: PolylineDefinition<PolylineOptions, InfoWindowOptions>;
}): Polyline;
protected abstract doRemovePolyline(polyline: Polyline): void;
protected abstract doCreateInfoWindow({ definition, element, }: {
definition: MarkerDefinition<MarkerOptions, InfoWindowOptions>['infoWindow'];
element: Marker;
} | {
definition: PolygonDefinition<PolygonOptions, InfoWindowOptions>['infoWindow'];
element: Polygon;
} | {
definition: PolylineDefinition<PolylineOptions, InfoWindowOptions>['infoWindow'];
element: Polyline;
definition: InfoWindowWithoutPositionDefinition<InfoWindowOptions>;
element: Marker | Polygon | Polyline;
}): InfoWindow;
protected abstract doFitBoundsToMarkers(): void;
protected abstract dispatchEvent(name: string, payload: Record<string, unknown>): void;
abstract centerValueChanged(): void;
abstract zoomValueChanged(): void;
markersValueChanged(): void;
polygonsValueChanged(): void;
polylinesValueChanged(): void;
private createDrawingFactory;
private onDrawChanged;
}
100 changes: 39 additions & 61 deletions src/Map/assets/dist/abstract_map_controller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,21 @@ class default_1 extends Controller {
constructor() {
super(...arguments);
this.markers = new Map();
this.infoWindows = [];
this.polygons = new Map();
this.polylines = new Map();
this.infoWindows = [];
this.isConnected = false;
}
connect() {
const options = this.optionsValue;
this.dispatchEvent('pre-connect', { options });
this.createMarker = this.createDrawingFactory('marker', this.markers, this.doCreateMarker.bind(this));
this.createPolygon = this.createDrawingFactory('polygon', this.polygons, this.doCreatePolygon.bind(this));
this.createPolyline = this.createDrawingFactory('polyline', this.polylines, this.doCreatePolyline.bind(this));
this.map = this.doCreateMap({ center: this.centerValue, zoom: this.zoomValue, options });
this.markersValue.forEach((marker) => this.createMarker(marker));
this.polygonsValue.forEach((polygon) => this.createPolygon(polygon));
this.polylinesValue.forEach((polyline) => this.createPolyline(polyline));
this.markersValue.forEach((definition) => this.createMarker({ definition }));
this.polygonsValue.forEach((definition) => this.createPolygon({ definition }));
this.polylinesValue.forEach((definition) => this.createPolyline({ definition }));
if (this.fitBoundsToMarkersValue) {
this.doFitBoundsToMarkers();
}
Expand All @@ -25,30 +29,7 @@ class default_1 extends Controller {
polylines: [...this.polylines.values()],
infoWindows: this.infoWindows,
});
}
createMarker(definition) {
this.dispatchEvent('marker:before-create', { definition });
const marker = this.doCreateMarker(definition);
this.dispatchEvent('marker:after-create', { marker });
marker['@id'] = definition['@id'];
this.markers.set(definition['@id'], marker);
return marker;
}
createPolygon(definition) {
this.dispatchEvent('polygon:before-create', { definition });
const polygon = this.doCreatePolygon(definition);
this.dispatchEvent('polygon:after-create', { polygon });
polygon['@id'] = definition['@id'];
this.polygons.set(definition['@id'], polygon);
return polygon;
}
createPolyline(definition) {
this.dispatchEvent('polyline:before-create', { definition });
const polyline = this.doCreatePolyline(definition);
this.dispatchEvent('polyline:after-create', { polyline });
polyline['@id'] = definition['@id'];
this.polylines.set(definition['@id'], polyline);
return polyline;
this.isConnected = true;
}
createInfoWindow({ definition, element, }) {
this.dispatchEvent('info-window:before-create', { definition, element });
Expand All @@ -58,53 +39,50 @@ class default_1 extends Controller {
return infoWindow;
}
markersValueChanged() {
if (!this.map) {
if (!this.isConnected) {
return;
}
this.markers.forEach((marker) => {
if (!this.markersValue.find((m) => m['@id'] === marker['@id'])) {
this.removeMarker(marker);
this.markers.delete(marker['@id']);
}
});
this.markersValue.forEach((marker) => {
if (!this.markers.has(marker['@id'])) {
this.createMarker(marker);
}
});
this.onDrawChanged(this.markers, this.markersValue, this.createMarker, this.doRemoveMarker);
if (this.fitBoundsToMarkersValue) {
this.doFitBoundsToMarkers();
}
}
polygonsValueChanged() {
if (!this.map) {
if (!this.isConnected) {
return;
}
this.polygons.forEach((polygon) => {
if (!this.polygonsValue.find((p) => p['@id'] === polygon['@id'])) {
this.removePolygon(polygon);
this.polygons.delete(polygon['@id']);
}
});
this.polygonsValue.forEach((polygon) => {
if (!this.polygons.has(polygon['@id'])) {
this.createPolygon(polygon);
}
});
this.onDrawChanged(this.polygons, this.polygonsValue, this.createPolygon, this.doRemovePolygon);
}
polylinesValueChanged() {
if (!this.map) {
if (!this.isConnected) {
return;
}
this.polylines.forEach((polyline) => {
if (!this.polylinesValue.find((p) => p['@id'] === polyline['@id'])) {
this.removePolyline(polyline);
this.polylines.delete(polyline['@id']);
}
this.onDrawChanged(this.polylines, this.polylinesValue, this.createPolyline, this.doRemovePolyline);
}
createDrawingFactory(type, draws, factory) {
const eventBefore = `${type}:before-create`;
const eventAfter = `${type}:after-create`;
return ({ definition }) => {
this.dispatchEvent(eventBefore, { definition });
const drawing = factory({ definition });
this.dispatchEvent(eventAfter, { [type]: drawing });
draws.set(definition['@id'], drawing);
return drawing;
};
}
onDrawChanged(draws, newDrawDefinitions, factory, remover) {
const idsToRemove = new Set(draws.keys());
newDrawDefinitions.forEach((definition) => {
idsToRemove.delete(definition['@id']);
});
idsToRemove.forEach((id) => {
const draw = draws.get(id);
remover(draw);
draws.delete(id);
});
this.polylinesValue.forEach((polyline) => {
if (!this.polylines.has(polyline['@id'])) {
this.createPolyline(polyline);
newDrawDefinitions.forEach((definition) => {
if (!draws.has(definition['@id'])) {
factory({ definition });
}
});
}
Expand Down
Loading

0 comments on commit 5974bfc

Please sign in to comment.