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

expose Public API's to ScriptjsGoogleMap #145

Closed
wants to merge 1 commit into from

Conversation

mattjstar
Copy link

I'd love to hear your thoughts on this. We're using the async ScriptjsGoogleMap implementation and we need access to the Public Api's specifically fitBounds.

Also the warning doesn't support object equality, so I decided to hide it:

warning(0 === changedKeys.length, `ScriptjsGoogleMap doesn't support mutating props after initial render. Changed props: %s`, `[${ changedKeys.join(", ") }]`);`

@tomchentw
Copy link
Owner

The warning issue is fixed by #143.

For the issue of < ScriptjsGoogleMap>, I believe I've done it wrong. It should be just a wrapper component. Let me do a PR and ask for you to review.

tomchentw added a commit that referenced this pull request Nov 19, 2015
* Closes #145
* Ref #92

BREAKING CHANGE: rename from async/ScriptjsGoogleMap to async/ScriptjsLoader and changed behavior from wrapping to delegation of GoogleMap element

To migrate the code follow the example below (extracted from examples/gh-pages migration):

Before:

<ScriptjsLoader
   hostname={"maps.googleapis.com"}
   pathname={"/maps/api/js"}
   query={{v: `3.${ AsyncGettingStarted.version }`, libraries: "geometry,dr
awing,places"}}
   }
  // <GoogleMap> props
  defaultZoom={3}
  defaultCenter={{lat: -25.363882, lng: 131.044922}}
  onClick={::this._handle_map_click}
/>

After:

<ScriptjsLoader
  hostname={"maps.googleapis.com"}
  pathname={"/maps/api/js"}
  query={{v: `3.${ AsyncGettingStarted.version }`, libraries: "geometry,drawing,places"}}

  googleMapElement={
    <GoogleMap
      defaultZoom={3}
      defaultCenter={{lat: -25.363882, lng: 131.044922}}
      onClick={::this._handle_map_click}
    />
  }
/>
@tomchentw
Copy link
Owner

Please help me review on #150, thanks!

tomchentw added a commit that referenced this pull request Nov 19, 2015
* Mark async/ScriptjsGoogleMap as deprecated
* Closes #145
* Ref #92

BREAKING CHANGE: migrate from async/ScriptjsGoogleMap to async/ScriptjsLoader and changed its behavior from implicit inheritance to simple delegation

To migrate the code follow the example below (extracted from examples/gh-pages migration):

Before:

<ScriptjsLoader
   hostname={"maps.googleapis.com"}
   pathname={"/maps/api/js"}
   query={{v: `3.exp`, libraries: "geometry,dr
awing,places"}}
   }
  // <GoogleMap> props
  defaultZoom={3}
  defaultCenter={{lat: -25.363882, lng: 131.044922}}
  onClick={::this._handle_map_click}
/>

After:

<ScriptjsLoader
  hostname={"maps.googleapis.com"}
  pathname={"/maps/api/js"}
  query={{v: `3.exp`, libraries: "geometry,drawing,places"}}

  googleMapElement={
    <GoogleMap
      defaultZoom={3}
      defaultCenter={{lat: -25.363882, lng: 131.044922}}
      onClick={::this._handle_map_click}
    />
  }
/>
tomchentw added a commit that referenced this pull request Nov 19, 2015
* Mark async/ScriptjsGoogleMap as deprecated
* Closes #145
* Ref #92

BREAKING CHANGE: migrate from async/ScriptjsGoogleMap to async/ScriptjsLoader and changed its behavior from implicit inheritance to simple delegation

To migrate the code follow the example below (extracted from examples/gh-pages migration):

Before:

```js
<ScriptjsLoader
  hostname={"maps.googleapis.com"}
  pathname={"/maps/api/js"}
  query={{v: `3.exp`, libraries: "geometry,drawing,places"}}
  //
  // <GoogleMap> props
  defaultZoom={3}
  defaultCenter={{lat: -25.363882, lng: 131.044922}}
  onClick={::this._handle_map_click}
/>
```

After:

```js
<ScriptjsLoader
  hostname={"maps.googleapis.com"}
  pathname={"/maps/api/js"}
  query={{v: `3.exp`, libraries: "geometry,drawing,places"}}
  //
  googleMapElement={
    <GoogleMap
      defaultZoom={3}
      defaultCenter={{lat: -25.363882, lng: 131.044922}}
      onClick={::this._handle_map_click}
    />
  }
/>
```
@tomchentw tomchentw closed this in ccfadd4 Nov 21, 2015
@tomchentw
Copy link
Owner

Released v4.5.0

@tomchentw
Copy link
Owner

Thanks for the inspiration from @mattjstar. I also update the synchronous version to a loader approach in #157. Make sure to check it out for latest update of async API as well.

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.

2 participants