Skip to content

[Map] Fix default values of Stimulus Map Controller #2420

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.

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 1 commit into from
Dec 3, 2024

Conversation

Kocal
Copy link
Member

@Kocal Kocal commented Dec 2, 2024

Q A
Bug fix? yes
New feature? no
Issues Fix #2417
License MIT

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.

@carsonbot carsonbot added Bug Bug Fix Map Status: Needs Review Needs to be reviewed labels Dec 2, 2024
Copy link

github-actions bot commented Dec 2, 2024

📊 Packages dist files size difference

ℹ️ No difference in dist packagesFiles.

@Kocal
Copy link
Member Author

Kocal commented Dec 2, 2024

Should be better now, TIL Stimulus values' default value does not behave like Vue props' default value 😅

I reverted my changes and used this.has... instead, they correctly switch from false or true if they absent or not (html attributes are not rendered when their value is null).

@Kocal Kocal force-pushed the 2417-map branch 2 times, most recently from 6b0847e to 0e1bbf4 Compare December 2, 2024 18:30
@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Dec 2, 2024
@Kocal Kocal merged commit 56cef50 into symfony:2.x Dec 3, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Bug Fix Map Status: Reviewed Has been reviewed by a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Map] Console error when displaying a map using a Live Component
3 participants