Skip to content

Commit

Permalink
bug #2420 [Map] Fix default values of Stimulus Map Controller (Kocal)
Browse files Browse the repository at this point in the history
This PR was squashed before being merged into the 2.x branch.

Discussion
----------

[Map] Fix default values of Stimulus Map Controller

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Issues        | Fix #2417 <!-- 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 issue happens at `doCreateMap` when we called `->fitBoundsToMarkers()` instead of `->center(...)`.

The value `center` was supposed to be `null`, but since I forgot to configure the default value of `this.centerValue` to `null`, it automatically used `{}` as default value (https://stimulus.hotwired.dev/reference/values#getters) and it breaks the code.

This bug has been introduced in #2385.

Commits
-------

4cb91ef [Map] Fix default values of Stimulus Map Controller
  • Loading branch information
Kocal committed Dec 3, 2024
2 parents ec4cfa1 + 4cb91ef commit 56cef50
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 12 deletions.
7 changes: 7 additions & 0 deletions src/Map/assets/dist/abstract_map_controller.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,13 @@ export default abstract class<MapOptions, Map, MarkerOptions, Marker, InfoWindow
polygonsValue: Array<PolygonDefinition<PolygonOptions, InfoWindowOptions>>;
polylinesValue: Array<PolylineDefinition<PolylineOptions, InfoWindowOptions>>;
optionsValue: MapOptions;
hasCenterValue: boolean;
hasZoomValue: boolean;
hasFitBoundsToMarkersValue: boolean;
hasMarkersValue: boolean;
hasPolygonsValue: boolean;
hasPolylinesValue: boolean;
hasOptionsValue: boolean;
protected map: Map;
protected markers: globalThis.Map<string, Marker>;
protected polygons: globalThis.Map<string, Polygon>;
Expand Down
6 changes: 5 additions & 1 deletion src/Map/assets/dist/abstract_map_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@ class default_1 extends Controller {
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.map = this.doCreateMap({
center: this.hasCenterValue ? this.centerValue : null,
zoom: this.hasZoomValue ? this.zoomValue : null,
options,
});
this.markersValue.forEach((definition) => this.createMarker({ definition }));
this.polygonsValue.forEach((definition) => this.createPolygon({ definition }));
this.polylinesValue.forEach((definition) => this.createPolyline({ definition }));
Expand Down
14 changes: 13 additions & 1 deletion src/Map/assets/src/abstract_map_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,14 @@ export default abstract class<
declare polylinesValue: Array<PolylineDefinition<PolylineOptions, InfoWindowOptions>>;
declare optionsValue: MapOptions;

declare hasCenterValue: boolean;
declare hasZoomValue: boolean;
declare hasFitBoundsToMarkersValue: boolean;
declare hasMarkersValue: boolean;
declare hasPolygonsValue: boolean;
declare hasPolylinesValue: boolean;
declare hasOptionsValue: boolean;

protected map: Map;
protected markers = new Map<Identifier, Marker>();
protected polygons = new Map<Identifier, Polygon>();
Expand Down Expand Up @@ -140,7 +148,11 @@ export default abstract class<
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.map = this.doCreateMap({
center: this.hasCenterValue ? this.centerValue : null,
zoom: this.hasZoomValue ? this.zoomValue : null,
options,
});
this.markersValue.forEach((definition) => this.createMarker({ definition }));
this.polygonsValue.forEach((definition) => this.createPolygon({ definition }));
this.polylinesValue.forEach((definition) => this.createPolyline({ definition }));
Expand Down
10 changes: 7 additions & 3 deletions src/Map/src/Bridge/Google/assets/dist/map_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@ class default_1 extends Controller {
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.map = this.doCreateMap({
center: this.hasCenterValue ? this.centerValue : null,
zoom: this.hasZoomValue ? this.zoomValue : null,
options,
});
this.markersValue.forEach((definition) => this.createMarker({ definition }));
this.polygonsValue.forEach((definition) => this.createPolygon({ definition }));
this.polylinesValue.forEach((definition) => this.createPolyline({ definition }));
Expand Down Expand Up @@ -121,12 +125,12 @@ class map_controller extends default_1 {
super.connect();
}
centerValueChanged() {
if (this.map && this.centerValue) {
if (this.map && this.hasCenterValue && this.centerValue) {
this.map.setCenter(this.centerValue);
}
}
zoomValueChanged() {
if (this.map && this.zoomValue) {
if (this.map && this.hasZoomValue && this.zoomValue) {
this.map.setZoom(this.zoomValue);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/Map/src/Bridge/Google/assets/src/map_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,13 @@ export default class extends AbstractMapController<
}

public centerValueChanged(): void {
if (this.map && this.centerValue) {
if (this.map && this.hasCenterValue && this.centerValue) {
this.map.setCenter(this.centerValue);
}
}

public zoomValueChanged(): void {
if (this.map && this.zoomValue) {
if (this.map && this.hasZoomValue && this.zoomValue) {
this.map.setZoom(this.zoomValue);
}
}
Expand Down
10 changes: 7 additions & 3 deletions src/Map/src/Bridge/Leaflet/assets/dist/map_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@ class default_1 extends Controller {
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.map = this.doCreateMap({
center: this.hasCenterValue ? this.centerValue : null,
zoom: this.hasZoomValue ? this.zoomValue : null,
options,
});
this.markersValue.forEach((definition) => this.createMarker({ definition }));
this.polygonsValue.forEach((definition) => this.createPolygon({ definition }));
this.polylinesValue.forEach((definition) => this.createPolyline({ definition }));
Expand Down Expand Up @@ -112,12 +116,12 @@ class map_controller extends default_1 {
super.connect();
}
centerValueChanged() {
if (this.map && this.centerValue && this.zoomValue) {
if (this.map && this.hasCenterValue && this.centerValue && this.hasZoomValue && this.zoomValue) {
this.map.setView(this.centerValue, this.zoomValue);
}
}
zoomValueChanged() {
if (this.map && this.zoomValue) {
if (this.map && this.hasZoomValue && this.zoomValue) {
this.map.setZoom(this.zoomValue);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/Map/src/Bridge/Leaflet/assets/src/map_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@ export default class extends AbstractMapController<
}

public centerValueChanged(): void {
if (this.map && this.centerValue && this.zoomValue) {
if (this.map && this.hasCenterValue && this.centerValue && this.hasZoomValue && this.zoomValue) {
this.map.setView(this.centerValue, this.zoomValue);
}
}

public zoomValueChanged(): void {
if (this.map && this.zoomValue) {
if (this.map && this.hasZoomValue && this.zoomValue) {
this.map.setZoom(this.zoomValue);
}
}
Expand Down

0 comments on commit 56cef50

Please sign in to comment.