-
Notifications
You must be signed in to change notification settings - Fork 818
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
Add clustered markers #1044
Add clustered markers #1044
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What a Good Feature what I want!!!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jigfox wow, thank you, this looks really good!
One note: I would like to keep the @agm/core package as a place for core Google Maps API features, so this package has no other external dependencies (like js-marker-clusterer).
We need to think about how we can structure this repo so that we can host multiple packages (like in the official angular repo).
I would propose the following package name: @agm/js-marker-clusterer
src/core/directives/cluster.ts
Outdated
selector: 'agm-cluster', | ||
providers: [ClusterManager, {provide: MarkerManager, useExisting: ClusterManager}] | ||
}) | ||
export class AgmCluster implements OnDestroy, OnChanges, OnInit { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AgmMarkerCluster
src/core/directives/cluster.ts
Outdated
providers: [ClusterManager, {provide: MarkerManager, useExisting: ClusterManager}] | ||
}) | ||
export class AgmCluster implements OnDestroy, OnChanges, OnInit { | ||
@Input() gridSize: number; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add a description for the inputs and outputs?
src/core/directives/cluster.ts
Outdated
@Input() imagePath: string; | ||
@Input() imageExtension: string; | ||
|
||
constructor(private cluster: ClusterManager) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private _markerClusterManager: MarkerClusterManager
import {Marker} from '../google-maps-types'; | ||
|
||
@Injectable() | ||
export class ClusterManager extends MarkerManager { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MarkerClusterManager
}); | ||
} | ||
|
||
init(options:{}) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add a concrete type here for options?
package.json
Outdated
@@ -44,7 +44,8 @@ | |||
"homepage": "https://angular-maps.com", | |||
"peerDependencies": { | |||
"@angular/common": "^4.0.0 || ^2.0.0", | |||
"@angular/core": "^4.0.0 || ^2.0.0" | |||
"@angular/core": "^4.0.0 || ^2.0.0", | |||
"js-marker-clusterer": "^1.0.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to have a separate @agm package to handle the different dependencies. (See my last comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That should be possible, what name would you suggest? I will try with @agm/cluster, if this is okay with you
@@ -547,13 +547,13 @@ export interface CalculatorResult { | |||
|
|||
export type CalculateFunction = (marker: Marker[], count: number) => CalculatorResult; | |||
|
|||
export interface MarkerClusterer { | |||
export interface IMarkerClusterer { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MarklerClusterer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That ist not that easy because tslint doesn't like it. The js-marker-clusterer
package will create a global var called MarkerClusterer, which I have to declare src/core/services/managers/cluster-manager.ts:12
@@ -7,9 +7,9 @@ import {GoogleMapsAPIWrapper} from './../google-maps-api-wrapper'; | |||
import {AgmMarker} from './../../directives/marker'; | |||
import {AgmCluster} from './../../directives/cluster'; | |||
// tslint:disable-next-line: no-use-before-declare | |||
import {Marker, MarkerClusterer, IClusterOptions} from '../google-maps-types'; | |||
import {Marker, IMarkerClusterer, IClusterOptions} from '../google-maps-types'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ClusterOptions
imageExtension_:string; | ||
} | ||
|
||
export interface IClusterOptions { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ClusterOptions
Hi, thanks for your Feedback. What name would you suggest your message seems like it was cut off:
I will Change all other things like requested |
@jigfox oh yeah, missed that. I would propose "@ agm/js-marker-clusterer". So in the src folder, we would have another folder @ agm/js-marker-clusterer and we would rename the core folder to @ agm/core. Please let me know when the changes are done. Thank you! |
@SebastianM I have some trouble with dependencies while splitting the cluster into its own module: https://github.com/inovex/angular-google-maps/pull/1/files So I need some help here, integrating this. Another much simpler option would be to create a completely new repo for the clusterer. It would not be the same as in angular itself, but it would be easier to maintain. What do you think? |
@jigfox thx. I worked on a multi-package solution yesterday in this PR (#1051) to support Snazzy Info Window. It's nearly done - I will merge this tomorrow. Afterwards, I can do the package modifications or you can also do this if you want to. The important part is that the manager cannot be in the providers array of the AgmMap class, so we need a solution for this. |
okay, I will try this approach, thank you |
a58dba2
to
28c05ea
Compare
@SebastianM: I have created a new package like the already existing. Is this okay, the way it is? Or should I change something? |
@jigfox thx. I will review it tomorrow. |
Any time planing to merge this to master? Thanks |
Also patiently waiting for the merge to be done |
@SebastianM When do you think this will be merged and released? |
@SebastianM Congrats, I totally understand |
Please, a demo of this? and complete docs |
@sebrojas14, I believe you can follow the snazzy window guide
|
thanks @pauljosephatay I took your example Code and put it into the README |
On compilation im getting |
@pauljosephatay getting error as mentioned here #1124 (comment) any thoughts? |
What i'm basically trying to do is add markers on clicking the map, and clustering them in real time, i don't even know if logically speaking, this feature is possible? |
@kuncevic probably dependency with js-marker-clusterer. please check out README installation and usage @sadeajayi, i believe no special treatment needed, like this
|
@pauljosephatay I done the install thing as I pinted in comment I can see the files listed in my My env:
html:
app.module
|
@pauljosephatay all good now. I was actually missing the |
@kuncevic how did you include the js-marker-clusterer module in the app module? |
im still getting error on compilation even after including the wrapper as a provider and importing that. The imports im making in my map-page.component.ts
Imports in my app module.ts
|
@sadeajayi check my But you have an issue with |
Oh okay, I did just that and im still getting the same errors |
@sadeajayi are you importing
|
@kuncevic Oh my God. Youre a life saver. |
Hi, I have this error when trying to compile : ERROR in C:/....../node_modules/@agm/js-marker-clusterer/services/managers/cluster-manager.d.ts (8,22): Class 'ClusterManager' incorrectly extends base class 'MarkerManager'. Any Idea ? |
I'm experiencing the same issue: added the library following the readme instructions and can't compile because of the following
|
@uakgul-talis @MarKco |
I installed with So it seems I'm already running the beta.1. Any chance the change was not deployed? |
|
You were right! I guess this could probably be the way to solve the issue for @uakgul-talis as well! Thank you! |
Hi guys, I have another issue |
This adds a new directive
<agm-cluster>
which allows to group markers into clusters if they are near each other like described and requested in #307. It uses the https://github.com/googlemaps/js-marker-clusterer to achieve this goal.