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

fix(GoogleMapsAPIWrapper): run Google Maps Apis outside of Angular #1714

Merged

Conversation

mpienkowski
Copy link
Contributor

Wrap all Google Maps APIs calls in the wrapper with runOutsideAngular

Fixes NgZone stability (issue #815)

Wrap all Google Maps APIs calls in the wrapper with runOutsideAngular

Fixes NgZone stability (issue sebholstein#815)
@codecov
Copy link

codecov bot commented Aug 30, 2019

Codecov Report

Merging #1714 into master will decrease coverage by 0.58%.
The diff coverage is 1.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1714      +/-   ##
==========================================
- Coverage   50.45%   49.86%   -0.59%     
==========================================
  Files          43       43              
  Lines        1881     1903      +22     
  Branches      170      170              
==========================================
  Hits          949      949              
- Misses        928      950      +22     
  Partials        4        4
Impacted Files Coverage Δ
packages/core/services/google-maps-api-wrapper.ts 27.02% <1.44%> (-6.69%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3549ccf...e876048. Read the comment docs.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Looks good overall, I'll need to review this more in detail

@ghost ghost added the needs: review label Sep 17, 2019
@mpienkowski
Copy link
Contributor Author

@doom777 any news on this? Can I help you somehow?

@lansana
Copy link

lansana commented Oct 22, 2019

@doom777 Would really like to see this as well. I'm adding multiple maps to a page, and there is a performance bottleneck from all the logic being ran inside Angular's zone, which causes change detection to be ran very frequently while the maps are being built.

@lansana
Copy link

lansana commented Nov 5, 2019

@doom777 @mpienkowski Hi, any update on this? If this PR won't go through, can I make another? I'd love to see this change.

@ghost ghost removed the needs: review label Nov 5, 2019
@ghost ghost merged commit 57685f2 into sebholstein:master Nov 5, 2019
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants