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 controlled component #88

Merged
merged 6 commits into from
Aug 7, 2015
Merged

Conversation

tomchentw
Copy link
Owner

Why

Rewrite to controlled components according to discussions in #68.

Breaking Changes

This commit rewrite this module from scratch.

  • GoogleMaps -> GoogleMap
    • (Others Component names are the same)
  • OverlayView
    • Now only accepts single child
  • Remove asynchronous loading support
  • The props are not directly served as options for all google.maps instance.
    • Instead, we convert setters to props and also expose getters on component instance.
  • To set an option directly, you can now pass options object
    just the same way as using Google Maps JavaScript APIs.
  • Expose props have two representation: controlled & uncontrolled
    • uncontrolled props will have a default${ PropName } name
    • controlled props will be ${ propName } as you expected
  • Uncontrolled props will be used only when the instance first mounted
  • Controlled props will call its corresponding setters every time rendered

MIGRATION GUIDE:

It introduces controlled property concept into the module. Most of the case, your application would like to have uncontrolled property. So change your component like this:

Before (v1.x.x):

<Marker
      key={this.props.key}
      position={this.props.position}
      animation={this.props.animation}
      onRightclick={this.handleMarkerRightclick} />

After (v2.x.x):

<Marker
      key={this.props.key}
      defaultPosition={this.props.position}
      defaultAnimation={this.props.animation}
      onRightclick={this.handleMarkerRightclick} />

The properties with default- prefix is uncontrolled property. It will only be set ONCE when the component is first-time mounted. Any further changes to your React props/state will NOT affect this marker (So it's uncontrolled from the view of React). Who can change the marker, you may ask. The answer is, only the component from google.maps.

But sometimes, we may want the marker's position changed according to current state. In that case, you have to provide controlled property to the <Marker [position={this.state.position}>. By doing so, the marker's position will change every time the React props/state changes. However, it will not intercept the changes made by the component from google.maps. This is because for the <Marker> itself, it doesn't know what events from google.maps will change its component. So the consumer who uses "react-google-maps" will have to manually handle this in their codebase.

@tomchentw tomchentw force-pushed the refactor-controlled-component branch 3 times, most recently from e717bc3 to ce42aaf Compare August 7, 2015 12:30
Closes #62

BREAKING CHANGE: This commit rewrite this module from scratch.

* GoogleMaps -> GoogleMap
	- (Others Component names are the same)
* OverlayView
      - Now only accepts single child
* Remove asynchronous loading support
* The props are not directly served as options for all google.maps instance.
	- Instead, we convert setters to props and also expose getters on component instance.
* To set an option directly, you can now pass options object
	just the same way as using Google Maps JavaScript APIs.
* Expose props have two representation: controlled & uncontrolled
	- uncontrolled props will have a `default${ PropName }` name
	- controlled props will be `${ propName }` as you expected
* Uncontrolled props will be used only when the instance first mounted
* Controlled props will call its corresponding setters **every** time rendered

MIGRATION GUIDE:

It introduces **controlled property** concept into the module. Most of the case, your application would like to have **uncontrolled** property. So change your component like this:

Before (v1.x.x):

```js
<Marker
      key={this.props.key}
      position={this.props.position}
      animation={this.props.animation}
      onRightclick={this.handleMarkerRightclick} />
```

After (v2.x.x):

```js
<Marker
      key={this.props.key}
      defaultPosition={this.props.position}
      defaultAnimation={this.props.animation}
      onRightclick={this.handleMarkerRightclick} />
```

The properties with *default-* prefix is **uncontrolled** property. It will only be set **ONCE** when the component is first-time mounted. Any further changes to your React props/state will **NOT** affect this marker (So it's **uncontrolled** from the view of React). Who can change the marker, you may ask. The answer is, only the component from google.maps.

But sometimes, we may want the marker's position changed according to current state. In that case, you have to provide **controlled** property to the `<Marker [position={this.state.position}>`. By doing so, the marker's position will change *every* time the React props/state changes. However, it will not intercept the changes made by the component from google.maps. This is because for the `<Marker>` itself, it doesn't know what events from google.maps will change its component. So the consumer who uses "react-google-maps" will have to manually handle this in their codebase.
* Also add this support to InfoBox as well
* Only one child is supported inside <InfoWindow> or <InfoBox>
* Closes #69 and thanks to @thetiby
@tomchentw tomchentw force-pushed the refactor-controlled-component branch from ce42aaf to 2c0dc02 Compare August 7, 2015 12:31
tomchentw added a commit that referenced this pull request Aug 7, 2015
@tomchentw tomchentw merged commit 2ccd2a4 into master Aug 7, 2015
@tomchentw tomchentw deleted the refactor-controlled-component branch August 7, 2015 12:40
@tomchentw
Copy link
Owner Author

Released v2.0.0 and v2.0.1.

@ksavenkov
Copy link

Migrating to 2.0.1. I have 0replaced mapPane option for OverlayView with defaultMapPaneName (as it is definitely uncontrolled, and now it halts on mapPaneName is not defined. If I use mapPaneName, then there's a react warning that required defaultMapPaneName property is not set. As I understand, I shouldn't specify them both, right?

@tomchentw
Copy link
Owner Author

@ksavenkov yes, I believe we could safely remove isRequired attribute in the OverlayViewCreator: http://git.io/v3vQZ . Is that correct?

@tomchentw
Copy link
Owner Author

Released v2.0.2.

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