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

refactor(GoogleMapLoader): introduce loader component interface #157

Merged
merged 4 commits into from
Nov 22, 2015

Conversation

tomchentw
Copy link
Owner

Breaking Changes

Why for this change?

This PR decouples the creation logic of google.maps.Map instance and the mounting logic for <GoogleMap> component. As a result, we'll have:

The correct lifecycle for <GoogleMap> component

Because when it's mounted (ref callback invoked with the component instance), it's ready for use: invoking public APIs, reading values with getters ...etc

Consistent API with ScriptjsLoader

See more in the example below.

GoogleMap with props.containerProps is now deprecated.

Use GoogleMapLoader with props.googleMapElement instead

We also suggest switching to callback based ref so that you'll get the component instance when it is mounted.

Before:

<GoogleMap containerProps={{
    ...this.props,
    style: {
      height: "100%",
    },
  }}
  ref="map"
  defaultZoom={3}
  defaultCenter={{lat: -25.363882, lng: 131.044922}}
  onClick={::this.handleMapClick}>
  {this.state.markers.map((marker, index) => {
    return (
      <Marker
        {...marker}
        onRightclick={this.handleMarkerRightclick.bind(this, index)} />
    );
  })}
</GoogleMap>

After:

<GoogleMapLoader
  containerElement={
    <div
      {...this.props}
      style={{
        height: "100%",
      }}
    />
  }
  googleMapElement={
    <GoogleMap
      ref={(map) => console.log(map)}
      defaultZoom={3}
      defaultCenter={{lat: -25.363882, lng: 131.044922}}
      onClick={::this.handleMapClick}>
      {this.state.markers.map((marker, index) => {
        return (
          <Marker
            {...marker}
            onRightclick={this.handleMarkerRightclick.bind(this, index)} />
        );
      })}
    </GoogleMap>
  }
/>

ScriptjsLoader will delegate to GoogleMapLoader when the script is loaded

Before:

<ScriptjsLoader
  hostname={"maps.googleapis.com"}
  pathname={"/maps/api/js"}
  query={{v: `3.${ AsyncGettingStarted.version }`, libraries: "geometry,drawing,places"}}
  loadingElement={
    <div {...this.props} style={{ height: "100%" }}>
      <FaSpinner />
    </div>
  }
  googleMapElement={
    <GoogleMap
      containerProps={{
        ...this.props,
        style: {
          height: "100%",
        },
      }}
      ref={googleMap => {
        // Wait until GoogleMap is fully loaded. Related to #133
        setTimeout(() => {
          googleMap && console.log(`Zoom: ${ googleMap.getZoom() }`);
        }, 50);
      }}
      defaultZoom={3}
      defaultCenter={{lat: -25.363882, lng: 131.044922}}
      onClick={::this.handleMapClick}
    >
      <Marker
        {...this.state.marker}
        onRightclick={this.handleMarkerRightclick}
      />
    </GoogleMap>
  }
/>

After:

<ScriptjsLoader
  hostname={"maps.googleapis.com"}
  pathname={"/maps/api/js"}
  query={{v: `3.${ AsyncGettingStarted.version }`, libraries: "geometry,drawing,places"}}
  loadingElement={
    <div {...this.props} style={{ height: "100%" }}>
      <FaSpinner />
    </div>
  }
  containerElement={
    <div {...this.props} style={{ height: "100%" }} />
  }
  googleMapElement={
    <GoogleMap
      ref={googleMap => {
        googleMap && console.log(`Zoom: ${ googleMap.getZoom() }`);
      }}
      defaultZoom={3}
      defaultCenter={{lat: -25.363882, lng: 131.044922}}
      onClick={::this.handleMapClick}
    >
      <Marker
        {...this.state.marker}
        onRightclick={this.handleMarkerRightclick}
      />
    </GoogleMap>
  }
/>

Update examples/gh-pages

You could see the real running code for these new behavior in [examples/gh-pages] folder

* Closes #141
* Closes #133

BREAKING CHANGE: GoogleMap with props.containerProps is now deprecated. Use GoogleMapLoader with props.googleMapElement instead

We also suggest switching to callback based ref so that you'll get the component instance when it is mounted.

Before:

```js
<GoogleMap containerProps={{
    ...this.props,
    style: {
      height: "100%",
    },
  }}
  ref="map"
  defaultZoom={3}
  defaultCenter={{lat: -25.363882, lng: 131.044922}}
  onClick={::this.handleMapClick}>
  {this.state.markers.map((marker, index) => {
    return (
      <Marker
        {...marker}
        onRightclick={this.handleMarkerRightclick.bind(this, index)} />
    );
  })}
</GoogleMap>
```

After:

```js
<GoogleMapLoader
  containerElement={
    <div
      {...this.props}
      style={{
        height: "100%",
      }}
    />
  }
  googleMapElement={
    <GoogleMap
      ref={(map) => console.log(map)}
      defaultZoom={3}
      defaultCenter={{lat: -25.363882, lng: 131.044922}}
      onClick={::this.handleMapClick}>
      {this.state.markers.map((marker, index) => {
        return (
          <Marker
            {...marker}
            onRightclick={this.handleMarkerRightclick.bind(this, index)} />
        );
      })}
    </GoogleMap>
  }
/>
```
* Ref #92

BREAKING CHANGE: ScriptjsLoader will delegate to GoogleMapLoader when the script is loaded

Before:

```js
<ScriptjsLoader
  hostname={"maps.googleapis.com"}
  pathname={"/maps/api/js"}
  query={{v: `3.${ AsyncGettingStarted.version }`, libraries: "geometry,drawing,places"}}
  loadingElement={
    <div {...this.props} style={{ height: "100%" }}>
      <FaSpinner />
    </div>
  }
  googleMapElement={
    <GoogleMap
      containerProps={{
        ...this.props,
        style: {
          height: "100%",
        },
      }}
      ref={googleMap => {
        // Wait until GoogleMap is fully loaded. Related to #133
        setTimeout(() => {
          googleMap && console.log(`Zoom: ${ googleMap.getZoom() }`);
        }, 50);
      }}
      defaultZoom={3}
      defaultCenter={{lat: -25.363882, lng: 131.044922}}
      onClick={::this.handleMapClick}
    >
      <Marker
        {...this.state.marker}
        onRightclick={this.handleMarkerRightclick}
      />
    </GoogleMap>
  }
/>
```

After:

```js
<ScriptjsLoader
  hostname={"maps.googleapis.com"}
  pathname={"/maps/api/js"}
  query={{v: `3.${ AsyncGettingStarted.version }`, libraries: "geometry,drawing,places"}}
  loadingElement={
    <div {...this.props} style={{ height: "100%" }}>
      <FaSpinner />
    </div>
  }
  containerElement={
    <div {...this.props} style={{ height: "100%" }} />
  }
  googleMapElement={
    <GoogleMap
      ref={googleMap => {
        googleMap && console.log(`Zoom: ${ googleMap.getZoom() }`);
      }}
      defaultZoom={3}
      defaultCenter={{lat: -25.363882, lng: 131.044922}}
      onClick={::this.handleMapClick}
    >
      <Marker
        {...this.state.marker}
        onRightclick={this.handleMarkerRightclick}
      />
    </GoogleMap>
  }
/>
```
@tomchentw
Copy link
Owner Author

Released v4.6.0

@sergiu-paraschiv
Copy link

Am I doing something wrong or is the ref (with callback) wrong? It's no longer the actual google maps DOM node but the one you're using for the children (OverlayView for example).
So google.maps.event.trigger(ReactDOM.findDOMNode(map), 'resize') won't work.
I'm doing this right now:
let mapElement = $(ReactDOM.findDOMNode(map)).next();
google.maps.event.trigger(mapElement[0], 'resize');
but having a built-in way to get a reference to the actual google maps DOM node would be a lot cleaner I think.

@tomchentw
Copy link
Owner Author

Yes, you should get the component instance instead of the google maps DOM node. To access it, you may get the DOM node of containerElement:

containerElement={
    <div {...this.props} style={{ height: "100%" }} ref={(dom) => console.log(`THIS is dom: ${ dom }`} />
  }
// You may also trigger the event using:
google.maps.event.trigger(dom.firstChild, 'resize');

@vvo
Copy link

vvo commented Dec 10, 2015

Thanks for this, I had to setTimeout(fn, 0) somewhere and was able to remove it :)

vvo pushed a commit to instantsearch/instantsearch-googlemaps that referenced this pull request Dec 10, 2015
@boosh
Copy link

boosh commented Dec 22, 2015

Looks like this example needs updating: https://tomchentw.github.io/react-google-maps/#places/search-box

I'm getting an error when I try to use it with v4.7.0.

@tomchentw
Copy link
Owner Author

@boosh yeah there are many examples need updating. Wanna help out?

@boosh
Copy link

boosh commented Dec 23, 2015

Ah OK. I'll try :-). I wasn't sure if it was one that slipped through the net. Thanks for the great library. Finally google maps is simple to work with 👍

@jedwards1211
Copy link

I think the deprecation error messages are confusing. I accidentally copied the "before" ScriptjsLoader example and I got:

warning: "async/ScriptjsLoader" is now rendering "GoogleMapLoader". Migrate to use "GoogleMapLoader" instead.
The old behavior will be removed in next major release (5.0.0).

So then I tried using GoogleMapLoader instead and I get the following error:

Warning: Make sure you've put a <script> tag in your <head> element to load Google Maps JavaScript API v3.
 If you're looking for built-in support to load it for you, use the "async/ScriptjsLoader" instead.
 See https://github.com/tomchentw/react-google-maps/pull/168

ScriptjsLoader is telling me to use GoogleMapLoader, GoogleMapLoader is telling me to use ScriptjsLoader.

It was only by digging deep through the code that I realized I needed to remove the containerProps from the GoogleMap element I copied.

@tomchentw
Copy link
Owner Author

Sorry for the confusion @jedwards1211

@indurnath
Copy link

If api is not loaded due to proxy settings (if the url is blocked), i need to show some message. Which is the error callback of the scriptjsloader ?
I am getting a timeout error for the url, and meantime its trying to create googlemap object , and i am getting google not defined uncaught exception . Any help is appreciated.

@mishkinf
Copy link

@tomchentw May not be suitable for this specific page but are there any examples of testing the google maps components code using these Scriptjsloader and GoogleMapsloader paradigm?

@tomchentw
Copy link
Owner Author

We're looking into it shortly! We're also looking for maintainers. Involve in #266 to help strengthen our community!

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.

8 participants