There aresome 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 eventtilesloaded.

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

Jonathan Weiss added4 commits

July 12, 2018 10:36

Add proponTilesLoaded

0d6808d

(Adjust package name

3400e36

Increase version

659969d

Make linter happy

5139b72

itsmichaeldiego reviewed

Jul 17, 2018

View reviewed changes

Copy link

Member

itsmichaeldiego left a comment•
edited
Loading

Choose a reason for hiding this comment

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

Good work, left a couple of comments

package.json Outdated

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

Copy link

Member

Choose a reason for hiding this comment

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

Lets remove the@jfw

Copy link

ContributorAuthor

Choose a reason for hiding this comment

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

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

package.json Outdated

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

Copy link

Member

Choose a reason for hiding this comment

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

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

Copy link

ContributorAuthor

Choose a reason for hiding this comment

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

Same here.

itsmichaeldiego reviewed

Jul 17, 2018

View reviewed changes

API.md Outdated

		@@ -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)

Copy link

Member

Choose a reason for hiding this comment

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

What about the info?

Jonathan Weiss added4 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

Copy link

ContributorAuthor

jonathanweiss commentedJul 19, 2018

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

itsmichaeldiego reviewed

Jul 22, 2018

View reviewed changes

Copy link

Member

itsmichaeldiego left a comment

Choose a reason for hiding this comment

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

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

CHANGELOG.md Outdated

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

		###[Unreleased]

Copy link

Member

Choose a reason for hiding this comment

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

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

Copy link

ContributorAuthor

Choose a reason for hiding this comment

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

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?

Jonathan Weiss added2 commits

July 25, 2018 07:25

Remove section header

53c1e42

v2.0.0

1f1c9fe

Copy link

Member

itsmichaeldiego commentedJul 25, 2018

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

Revert version number

80abcd3

itsmichaeldiego reviewed

Jul 27, 2018

View reviewed changes

src/google_map.js Outdated

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

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

Copy link

Member

Choose a reason for hiding this comment

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

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

Copy link

ContributorAuthor

Choose a reason for hiding this comment

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

At first I thought that this is just a micro optimisation, but this event is triggereda 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. 👍

Copy link

Member

itsmichaeldiego commentedJul 27, 2018

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

Only listen to event if prop is used

b123964

Copy link

ContributorAuthor

jonathanweiss commentedJul 30, 2018

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

Copy link

Member

itsmichaeldiego commentedJul 31, 2018•
edited
Loading

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

Copy link

ContributorAuthor

jonathanweiss commentedJul 31, 2018

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

Copy link

Member

itsmichaeldiego commentedAug 3, 2018

@jonathanweiss This is fine like this, thanks though!

itsmichaeldiego merged commit6d142cb intogoogle-map-react:master

Aug 3, 2018

itsmichaeldiego mentioned this pull request

Aug 3, 2018

Bump version to 1.0.6#621

Merged

Copy link

jsharland commentedSep 4, 2018

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 mentioned this pull request

Sep 5, 2018

Added missing onTilesLoadedDefinitelyTyped/DefinitelyTyped#28639

Merged

Copy link

vertic4l commentedSep 5, 2018

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

DonovanDeSmedt added a commit to DonovanDeSmedt/google-map-react that referenced this pull request

Nov 21, 2018

Merge branch 'master' of github.com:google-map-react/google-map-react

ad9e522

* '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)

Copy link

lockbot commentedDec 1, 2019

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.

lockbot locked asresolvedand limited conversation to collaborators

Dec 1, 2019

Labels

None yet

4 participants

Movatterモバイル変換

Add proponTilesLoaded#615

Add proponTilesLoaded#615

Uh oh!

Conversation

jonathanweiss commentedJul 16, 2018

Uh oh!

itsmichaeldiego left a comment• editedLoading 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 commentedJul 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 commentedJul 25, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

itsmichaeldiego commentedJul 27, 2018

Uh oh!

jonathanweiss commentedJul 30, 2018

Uh oh!

itsmichaeldiego commentedJul 31, 2018• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

jonathanweiss commentedJul 31, 2018

Uh oh!

itsmichaeldiego commentedAug 3, 2018

Uh oh!

jsharland commentedSep 4, 2018

Uh oh!

vertic4l commentedSep 5, 2018

Uh oh!

lockbot commentedDec 1, 2019

Uh oh!

Uh oh!

Add prop`onTilesLoaded`#615

Add prop`onTilesLoaded`#615

itsmichaeldiego left a comment•
edited
Loading

itsmichaeldiego commentedJul 31, 2018•
edited
Loading