Skip to content

[Map] Add Rectangle support #2845

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 4 commits into from
Jun 22, 2025

Conversation

Valmonzo
Copy link
Contributor

@Valmonzo Valmonzo commented Jun 17, 2025

Q A
Bug fix? no
New feature? yes
Docs? yes
Issues
License MIT

Adding a new Rectangle element to Map following GoogleMaps and Leaflet features.

Capture d’écran 2025-06-17 à 13 43 36 Capture d’écran 2025-06-17 à 13 44 23

Copy link
Contributor

github-actions bot commented Jun 17, 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
abstract_map_controller.d.ts 5.75 kB / 1.25 kB 6.6 kB+15% 📈 / 1.35 kB+8% 📈
abstract_map_controller.js 4.48 kB / 1.14 kB 5.01 kB+12% 📈 / 1.2 kB+6% 📈
Map (Bridge Google)
map_controller.d.ts 3.03 kB / 866 B 3.35 kB+11% 📈 / 909 B+5% 📈
map_controller.js 12.54 kB / 2.72 kB 13.67 kB+9% 📈 / 2.86 kB+5% 📈
Map (Bridge Leaflet)
map_controller.d.ts 2.63 kB / 781 B 2.93 kB+12% 📈 / 829 B+6% 📈
map_controller.js 11.06 kB / 2.77 kB 12.18 kB+10% 📈 / 2.92 kB+6% 📈

@Valmonzo Valmonzo force-pushed the feat/add-rectangle-support-for-map branch 2 times, most recently from e0b3f91 to e76d201 Compare June 17, 2025 11:47
@Valmonzo Valmonzo marked this pull request as ready for review June 17, 2025 11:49
@Valmonzo Valmonzo requested a review from Kocal as a code owner June 17, 2025 11:49
@carsonbot carsonbot added Feature New Feature Map Status: Needs Review Needs to be reviewed labels Jun 17, 2025
@Valmonzo Valmonzo changed the title [Map] Add support for rectangle [Map] Add Rectangle support Jun 17, 2025
@Valmonzo Valmonzo requested a review from Kocal June 22, 2025 10:38
@Kocal Kocal force-pushed the feat/add-rectangle-support-for-map branch from dd132b1 to 861c5c9 Compare June 22, 2025 14:54
@Kocal Kocal force-pushed the feat/add-rectangle-support-for-map branch from 861c5c9 to 93c31b0 Compare June 22, 2025 14:55
Comment on lines +111 to +118
foreach ($attrs['circles'] as $key => $circle) {
$attrs['circles'][$key]['@id'] = $computeId($circle);
}

foreach ($attrs['rectangles'] as $key => $rectangle) {
$attrs['rectangles'][$key]['@id'] = $computeId($rectangle);
}

Copy link
Member

Choose a reason for hiding this comment

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

The @id computation was missing from last PR and this one, it was not possible to use them correctly when using the Map with a LiveComponent.

I've improved our tests to assert that @id are present and nicely computed, for the next Elements instances properties.

Copy link
Member

Choose a reason for hiding this comment

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

Modifying this file was also forgotten in the previous PR and this one. Without these modifications, it was not possible to use <twig:ux:map circles="{{ ... }}"> or ux_map(circles=[]). Same for rectangles.

Copy link
Member

@Kocal Kocal left a comment

Choose a reason for hiding this comment

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

I had to fix some things, but it led to an opportunity to improve tests :)

@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Jun 22, 2025
@Kocal
Copy link
Member

Kocal commented Jun 22, 2025

Thank you @Valmonzo.

@Kocal Kocal merged commit 919f896 into symfony:2.x Jun 22, 2025
22 of 23 checks passed
@Valmonzo Valmonzo deleted the feat/add-rectangle-support-for-map branch June 22, 2025 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New Feature Map Status: Reviewed Has been reviewed by a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants