Skip to content

Conversation

ker0x
Copy link
Contributor

@ker0x ker0x commented Oct 1, 2025

Q A
Bug fix? no
New feature? feature
Deprecations? no
Documentation? no
Issues Fix #3116
License MIT

Add additional checks before creating the icon for a marker

@ker0x ker0x requested a review from Kocal as a code owner October 1, 2025 14:44
@carsonbot carsonbot added Bug Bug Fix Map Status: Needs Review Needs to be reviewed labels Oct 1, 2025
@ker0x ker0x force-pushed the fix/issue-3116-bridge-options-icon branch 2 times, most recently from 2a76425 to ecaa959 Compare October 1, 2025 14:50
Copy link
Contributor

github-actions bot commented Oct 1, 2025

📊 Packages dist files size difference

Thanks for the PR! Here is the difference in size of the packages dist files between the base branch and the PR.
Please review the changes and make sure they are expected.

FileBefore (Size / Gzip)After (Size / Gzip)
Map (Bridge Google)
map_controller.js 13.49 kB / 3.18 kB 13.93 kB+3% 📈 / 3.29 kB+3% 📈
Map (Bridge Leaflet)
map_controller.js 13.27 kB / 3.43 kB 13.7 kB+3% 📈 / 3.52 kB+3% 📈

@Kocal
Copy link
Member

Kocal commented Oct 1, 2025

Following #3116 (comment), I suggest instead log a warning message if we saw that the user tried to create an icon from the JavaScript side (through bridgeOptions) when Marker#$icon is not null

@ker0x ker0x force-pushed the fix/issue-3116-bridge-options-icon branch from ecaa959 to fbe1c48 Compare October 2, 2025 09:52
@ker0x ker0x changed the title [Map] Fix customize icon with bridgeOptions [Map] Add warning if customize icon is defined alongside with marker icon Oct 2, 2025
@ker0x
Copy link
Contributor Author

ker0x commented Oct 2, 2025

I updated the PR to trigger a warning, as you suggested 👍!

@smnandre
Copy link
Member

smnandre commented Oct 3, 2025

Should this be done on the PHP part ?

@Kocal
Copy link
Member

Kocal commented Oct 3, 2025

No because we want to verify that the user, through a Stimulus Controller, tried to add an icon by itself with bridgeOptions

@Kocal Kocal force-pushed the fix/issue-3116-bridge-options-icon branch from fbe1c48 to b3d5c4d Compare October 4, 2025 07:19
@Kocal Kocal changed the title [Map] Add warning if customize icon is defined alongside with marker icon [Map] Display warning when trying to define a custom icon for a Marker that already has an Icon Oct 4, 2025
@Kocal Kocal added Feature New Feature and removed Bug Bug Fix labels Oct 4, 2025
Comment on lines 3 to 6
## 2.30

- Ensure compatibility with PHP 8.5
- Display a warning when trying to define `bridgeOptions.content` for a `Marker` that already has an `Icon`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
## 2.30
- Ensure compatibility with PHP 8.5
- Display a warning when trying to define `bridgeOptions.content` for a `Marker` that already has an `Icon`
## 2.31
- Display a warning when trying to define `bridgeOptions.content` for a `Marker` that already has an `Icon`
## 2.30
- Ensure compatibility with PHP 8.5

2.30 has already been release!

Comment on lines 3 to 6
## 2.30

- Ensure compatibility with PHP 8.5
- Display a warning when trying to define `bridgeOptions.icon` for a `Marker` that already has an `Icon`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
## 2.30
- Ensure compatibility with PHP 8.5
- Display a warning when trying to define `bridgeOptions.icon` for a `Marker` that already has an `Icon`
## 2.31
- Display a warning when trying to define `bridgeOptions.icon` for a `Marker` that already has an `Icon`
## 2.30
- Ensure compatibility with PHP 8.5

@Kocal Kocal force-pushed the fix/issue-3116-bridge-options-icon branch from b3d5c4d to 6a6b230 Compare October 4, 2025 11:45
@Kocal
Copy link
Member

Kocal commented Oct 4, 2025

Thank you @ker0x.

@Kocal Kocal merged commit fa60799 into symfony:2.x Oct 4, 2025
27 of 28 checks passed
@ker0x ker0x deleted the fix/issue-3116-bridge-options-icon branch October 4, 2025 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New Feature Map Status: Needs Review Needs to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Map] Unable to customize the icon of a marker with bridgeOptions
4 participants