Skip to content
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

[Map] Add Clustering Algorithms #2554

Open
wants to merge 4 commits into
base: 2.x
Choose a base branch
from

Conversation

smnandre
Copy link
Member

@smnandre smnandre commented Feb 7, 2025

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

Provide an interface and the first two PHP implementations of Clustering algorithms

  • GridClustering
  • Morton

Performances: < 1ms per 1000 Points *

  • tested on a non production server (... my iMac 😅 )

Next Step will require JS code.

But this is already usefull with PHP classes only, for whomever has large amount of markers

Provide an interface and the first two PHP implementations of Clustering algorithms
- GridClustering
- Morton

Performances: < 1ms per 1000 Point

Next Step will require aditional JS code, but this is already very usefull to display maps from large amount of points
@smnandre smnandre requested a review from Kocal as a code owner February 7, 2025 22:54
@carsonbot carsonbot added Bug Bug Fix Feature New Feature Map Status: Needs Review Needs to be reviewed labels Feb 7, 2025
@smnandre smnandre removed the Bug Bug Fix label Feb 7, 2025
@smnandre
Copy link
Member Author

smnandre commented Feb 9, 2025

(cannot ask for your review @Kocal as you're code owner, i would have otherwise ;) )

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.

Can you provide some examples/use-cases and documentation? I'm not really sure to understand what you are solving right now.

When you say "next step is JS", do you mean https://github.com/Leaflet/Leaflet.markercluster & https://developers.google.com/maps/documentation/javascript/marker-clustering?hl=fr implementation?

@smnandre
Copy link
Member Author

smnandre commented Feb 9, 2025

You have 30000 markers in database. Sending them all in HTML is non sense. Clustering them in backend is definitively the way to go. That way you only send data you want to display, and fix the performance problem some people had with too many infowindow for instance.

Moreover it enforces the initial problem this bundle save: abstract the map services and manipulate data / UI from backend.

People that do want to use custom GoogleMaps SDK Methods wont install this Bundle, so -personal opinion- it’s a mistake to base our choices on what they would do there.

Also this would not coexist with Live trait for now.

So Next step is to continue the path and provide the missing link: making the LiveComponent able to react on map events: zoom or pan, clicks, etc.

That’s where I am right now, I Hope i’ve clarified my vision and goal for both this feature and more generally this bundle.

And would love to integrate this Into yours, if you have a différent one :)

@Kocal
Copy link
Member

Kocal commented Feb 9, 2025

Thanks for the explanations! I was asking because I didn't see any modifications on Map nor in the documentation, and so I was wondering what you really wanted to do with this PR.

@smnandre
Copy link
Member Author

smnandre commented Feb 9, 2025

Yeah i'm sorry I miss time and need to push more often, or i end with hundreds of "features ready at 80%" on my computer and never find time to release them.

Here, it's because two unrelated people asked me for the same exact thing last week, I've reopened my "geo" archives and started to prepare the foundations.

I'd like to end this (documentation included) before 2.24 😅

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

Kocal commented Mar 4, 2025

After a small rebase, that would be perfect 🤓

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