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

AgmMarkerCluster breaks info windows #1126

Closed
cdarken opened this issue Aug 31, 2017 · 10 comments · Fixed by #1140
Closed

AgmMarkerCluster breaks info windows #1126

cdarken opened this issue Aug 31, 2017 · 10 comments · Fixed by #1140

Comments

@cdarken
Copy link

cdarken commented Aug 31, 2017

After I add marker clustering to my map, instead of the info window opening, I get this error:

Uncaught (in promise): TypeError: Cannot read property 'then' of undefined
TypeError: Cannot read property 'then' of undefined at info-window-manager.js:40

angular2 & angular-google-maps version
Angular 4.3.6
Agm 1.0.0-beta1

@jigfox jigfox self-assigned this Sep 1, 2017
jigfox added a commit to jigfox/angular-google-maps that referenced this issue Sep 8, 2017
the marker cluster needs it's own `InfoWindowManager`, otherwise
`InfoWindowManager` would be injected with the root `MarkerManager`
instead of the `ClusterManager`

fixes sebholstein#1126
@BayardSmart
Copy link

Works perfectly with the 9afdeda, Thanks @jigfox

@sergamers
Copy link

How can i load this commit?

@cdarken
Copy link
Author

cdarken commented Sep 15, 2017

@sergamers clone the repo, checkout the branch jigfox-fix-info-window, executa yarn, then npm run build.
then in your project yarn install <relative path to>/dist/@agm/core <relative path to>/@dist/@agm/js-marker-clusterer then build your project

sebholstein pushed a commit that referenced this issue Sep 17, 2017
the marker cluster needs it's own `InfoWindowManager`, otherwise
`InfoWindowManager` would be injected with the root `MarkerManager`
instead of the `ClusterManager`

fixes #1126
@72gm
Copy link

72gm commented Sep 27, 2017

given this is a fairly fundamental problem, if you want use clustering, is this likely to be released anytime soon? the product isn't really usable without it

@cdarken
Copy link
Author

cdarken commented Sep 27, 2017

@72gm personally I don't see an issue with building it myself because anyway you need a new build phase before deploying

@72gm
Copy link

72gm commented Sep 27, 2017

I'm guessing you work by yourself? If you worked in a team of many you'd likely realise that isn't an answer that can be used

Looks like there is a lot of good stuff going on with this product.. its a shame that it is so badly documented

@cdarken
Copy link
Author

cdarken commented Sep 27, 2017

Yeah, but even if you work in a team you can add your package from other sources. You don't have to use npmjs. For instance you can build once and put your package as an archive on your internal server. For instance with yarn you can create a tgz: https://yarnpkg.com/lang/en/docs/cli/add/

@72gm
Copy link

72gm commented Sep 27, 2017

i do appreciate what you are saying but this adds a level of maintenance/effort that really shouldn't be necessary...

this is such an obvious bug that it should never have happened and should have been patched immediately??

@cdarken
Copy link
Author

cdarken commented Sep 27, 2017

Don't know about that, people have other stuff to do sometimes, that's life. I just offered a viable solution until they'll release a new version, instead of waiting and complaining. This is open source software, after all, they did almost all the work, running some scripts to make our own builds is not that hard.

@72gm
Copy link

72gm commented Sep 27, 2017

fair point... it's (too) easy to forget that on a lot of these projects the people are working on it after their day job has finished.. and it's free so can't expect the level you might get from a paid product

i'll shut up :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants