-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
fix(google-maps): unable to subscribe to events after initialization #17845
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.
Extremely good change, just one naming nit
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.
Looks great! If it's not too much trouble, can you update the most recent component that was just added, MapPolyline as well?
As things are set up right now in the `google-map`, `map-marker` and `map-info-window` components we choose whether to bind an event based on whether there are subscribers to its observable. The problem is that if the user decides to subscribe after init (e.g. by getting a hold of object with `ViewChild` and subscribing) it'll never emit because the native event wasn't added. These changes fix the issue by switching the event to be observables and binding the event when something has subscribed. I've also introduced a `MapEventManager` class to make it easier to keep track of the events and to avoid duplication between the three components.
2de37ac
to
1758d40
Compare
I've addressed the feedback @jelbourn @mbehrlich. |
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.
LGTM
Looks like a regression from angular#17845 where the name of the `zoom_changed` even was mistyped.
Looks like a regression from angular#17845 where the name of the `zoom_changed` even was mistyped. Fixes angular#18026.
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
As things are set up right now in the
google-map
,map-marker
andmap-info-window
components we choose whether to bind an event based on whether there are subscribers to its observable. The problem is that if the user decides to subscribe after init (e.g. by getting a hold of object withViewChild
and subscribing) it'll never emit because the native event wasn't added.These changes fix the issue by switching the event to be observables and binding the event when something has subscribed. I've also introduced a
MapEventManager
class to make it easier to keep track of the events and to avoid duplication between the three components.