Add prop `onTilesLoaded` #615

jonathanweiss · 2018-07-16T04:01:54Z

There are some events that are fired by the Google Maps JavaScript API, but there are not available on the React component. With this PR, I'm passing through the event tilesloaded.

Please let me know what you think about this. I'd love to see my changes upstream.

itsmichaeldiego

Good work, left a couple of comments

itsmichaeldiego · 2018-07-17T20:57:25Z

package.json

@@ -1,6 +1,6 @@
 {
-  "name": "google-map-react",
-  "version": "1.0.5",
+  "name": "@jfw/google-map-react",


Lets remove the @jfw

Sorry for that. I needed to publish the component in order to use it in a project. I'll remove it on master.

itsmichaeldiego · 2018-07-17T20:57:42Z

package.json

-  "name": "google-map-react",
-  "version": "1.0.5",
+  "name": "@jfw/google-map-react",
+  "version": "1.0.6",


Lets not make a version from this PR, version will be bumped when necessary with one or multiple PRs

itsmichaeldiego · 2018-07-17T20:58:41Z

API.md

@@ -138,6 +138,8 @@ When the user changes the map type (HYBRID, ROADMAP, SATELLITE, TERRAIN) this fi
 #### onGoogleApiLoaded (func)
 Directly access the maps API - *use at your own risk!*

+#### onTilesLoaded (func)
+


What about the info?

jonathanweiss · 2018-07-19T22:02:00Z

I've applied all changes, please have another look @itsmichaeldiego.

itsmichaeldiego

Great work man! I just left a very minor comment, looking forward to merge this asap.

itsmichaeldiego · 2018-07-22T12:00:11Z

CHANGELOG.md

@@ -1,5 +1,8 @@
 Add [google-map-clustering-example](https://github.com/istarkov/google-map-clustering-example)

+###[Unreleased]


@jonathanweiss Can we move this section to the bottom? And also, no need of add Unreleased as I am going to release it very soon after merged.

Sure thing, I've removed that header. Not sure if I understand you correctly: you want me to move the entries for unreleased things to the bottom of the change log file?

itsmichaeldiego · 2018-07-25T22:50:37Z

Can you delete the latest commit? Only the maintainers set the version releases

itsmichaeldiego · 2018-07-27T14:07:58Z

src/google_map.js

@@ -663,6 +662,10 @@ export default class GoogleMap extends Component {
          this.heatmap.setMap(map);
        }

+        maps.event.addListener(map, 'tilesloaded', () => {


Should we add the listener all the times, or only when the user has set an onTilesLoaded prop? What do you think?

At first I thought that this is just a micro optimisation, but this event is triggered a lot when the user is changing the viewport of the map. So it makes sense not to listen to the event if we don't need to. 👍

itsmichaeldiego · 2018-07-27T14:08:33Z

@jonathanweiss Changes are looking great now! Thank you, I left a question for you before we merge this.

jonathanweiss · 2018-07-30T05:30:51Z

Now we don't need _onTilesLoaded() any longer. 😉 Do you want to keep it for consistency?

itsmichaeldiego · 2018-07-31T10:28:23Z

@jonathanweiss What do you mean by that we don't need it any longer?

jonathanweiss · 2018-07-31T22:27:58Z

Well, all that it does is checking if this.props.onTilesLoaded is truthy before calling it. Since we're now only attaching the listener when that prop is truthy, we could directly call this.props.onTilesLoaded(); in line 667.

itsmichaeldiego · 2018-08-03T07:49:27Z

@jonathanweiss This is fine like this, thanks though!

jsharland · 2018-09-04T15:07:23Z

Can someone please update the typescript definition to match as I could really do with the onTilesLoaded event.

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/google-map-react/index.d.ts

vertic4l · 2018-09-05T15:06:53Z

@jsharland Type Definition are now up2date.
@jonathanweiss @itsmichaeldiego thank you!

* 'master' of github.com:google-map-react/google-map-react: Bump to 1.1.1 (google-map-react#680) Revert "Added feature: update heat map on data change + fix linting" (google-map-react#679) Bump version to 1.1.0 (google-map-react#671) Added feature: update heat map on data change + fix linting (google-map-react#593) Pass map instance to onDrag handler (google-map-react#656) add math abs to avoid negative values when calculating zoom (google-map-react#655) Bump version to 1.0.9 (google-map-react#651) Custom div style options (google-map-react#634) Bump version to 1.0.8 (google-map-react#646) Revert 643 fix/map context (google-map-react#645) Bump version to 1.0.7 (google-map-react#644) Add passive scroll (google-map-react#631) Use React 16 portal to render map overlay (google-map-react#643) Fix old examples links and add one to new examples (google-map-react#633) Bump version to 1.0.6 (google-map-react#621) Add prop `onTilesLoaded` (google-map-react#615) Fix typo, and call fromContainerPixelToLatLng() as you would expect. (google-map-react#620) Update API.md (google-map-react#611) Upgrade version to 1.0.5 (google-map-react#607) Remove marker jiggle. (google-map-react#603)

lock · 2019-12-01T10:14:36Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

Jonathan Weiss added 4 commits July 12, 2018 10:36

Add prop onTilesLoaded

0d6808d

(Adjust package name

3400e36

Increase version

659969d

Make linter happy

5139b72

itsmichaeldiego reviewed Jul 17, 2018

View reviewed changes

Jonathan Weiss added 4 commits July 19, 2018 08:37

Remove scope from package name and reset version number

c2ec301

Add description for new prop

796366c

Remove "directories" entry

aa4036a

Correct changelog

b70a488

itsmichaeldiego reviewed Jul 22, 2018

View reviewed changes

Jonathan Weiss added 2 commits July 25, 2018 07:25

Remove section header

53c1e42

v2.0.0

1f1c9fe

Revert version number

80abcd3

itsmichaeldiego reviewed Jul 27, 2018

View reviewed changes

Only listen to event if prop is used

b123964

itsmichaeldiego merged commit 6d142cb into google-map-react:master Aug 3, 2018

itsmichaeldiego mentioned this pull request Aug 3, 2018

Bump version to 1.0.6 #621

Merged

vertic4l mentioned this pull request Sep 5, 2018

Added missing onTilesLoaded DefinitelyTyped/DefinitelyTyped#28639

Merged

lock bot locked as resolved and limited conversation to collaborators Dec 1, 2019

		@@ -1,5 +1,8 @@
		Add [google-map-clustering-example](https://github.com/istarkov/google-map-clustering-example)

		###[Unreleased]

Add prop onTilesLoaded #615

Add prop onTilesLoaded #615

Uh oh!

Conversation

jonathanweiss commented Jul 16, 2018

Uh oh!

itsmichaeldiego left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonathanweiss commented Jul 19, 2018

Uh oh!

itsmichaeldiego left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

itsmichaeldiego commented Jul 25, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

itsmichaeldiego commented Jul 27, 2018

Uh oh!

jonathanweiss commented Jul 30, 2018

Uh oh!

itsmichaeldiego commented Jul 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonathanweiss commented Jul 31, 2018

Uh oh!

itsmichaeldiego commented Aug 3, 2018

Uh oh!

jsharland commented Sep 4, 2018

Uh oh!

vertic4l commented Sep 5, 2018

Uh oh!

lock bot commented Dec 1, 2019

Uh oh!

Uh oh!

Add prop `onTilesLoaded` #615

Add prop `onTilesLoaded` #615

itsmichaeldiego left a comment •

edited

Loading

itsmichaeldiego commented Jul 31, 2018 •

edited

Loading