Add a `style` prop to the <img> about react-static-google-map HOT 7 CLOSED

I agree with your statements about having two different style props, I don't like that either. I have tried both of the solutions that you suggested but ran into trouble with both.

When using a className, the user cannot set dynamic values like {height: this.state.height}. I actually used the className solution recently and it worked fine because I didn't need any variables and a simple class worked, but I did notice that I couldn't pass variables if I had needed to.

I need to try the as prop again. I think I misused it like

<StaticGoogleMap as={<img  style={...}/>} size="600x400" key="xx">
  <Marker location="x" />
</StaticGoogleMap>

I'll do that and get back to you. This may be as simple as adding some docs.

from react-static-google-map.

Interesting. I can see why users might want this but I think having two different type of style props, especially when the intuitive one doesn't do what the user would assume it does might be a tad confusing. Although, one might argue that by default the props are all for Google static maps save for a few.

Currently, we have two ways users can achieve this.

Using className

<StaticGoogleMap size="600x400" className="any-class-you-want" key="xx">
  <Marker location="x" />
</StaticGoogleMap>

Using as prop

const  StyledImage = styled.img`
  margin: 0px auto;
`

<StaticGoogleMap as={StyledImage} size="600x400" key="xx">
  <Marker location="x" />
</StaticGoogleMap>

Is it too much friction for the user to create a custom image component or write a class they want to use to render the image?

Downsides to this is that users might intuitively think that the style prop belongs to the image, although the documentation does mention that it does not, it's still unconventional though. We might want to rename the Google Style prop to something else but then, that's not backward compatible.

from react-static-google-map.

OK. So I poked around at this a bit more.

With your example,

const  StyledImage = styled.img`
  margin: 0px auto;
`

<StaticGoogleMap as={StyledImage} size="600x400" key="xx">
  <Marker location="x" />
</StaticGoogleMap>

The user still cannot dynamically set the style property, because the props are just intrinsically passed to the component and there is no style prop being passed. So you can only set style statically like you have above.

But, this solution does work when you manage the prop manually. For example,

<StaticGoogleMap as={props => <img {...props} style={... style from state} />} size="600x400" key="xx">
  <Marker location="x" />
</StaticGoogleMap>

Seem like a good solution to add to docs?

from react-static-google-map.

Yes, I think it's fine to document that for now, but I think it's a hassle as you had earlier stated.

What do you think about changing the Google style prop to mapStyle and make the style prop work for the actual image component?

It's a breaking change but I think that'll be more intuitive. Also, we want to avoid as many re-renders of the component as much as we can, as in this case, are "expensive" api calls.

from react-static-google-map.

I am a little unclear how this causes more renders, but I do totally agree with that sentiment. Unlike the the Directions component that was actually calling the directions api every render (thus the cache thing I added awhile back), the browser handles caching the images...I think...unless the headers specify otherwise, but I am pretty sure I have noticed subsequent mounts (not just renders) are faster which would indicate browser caching. But maybe I am missing something, and I totally agree we want to limit renders.

So we have talked about a couple of potentially breaking changes, such as rewriting the lib to handle latest versions of React and getting rid of the outdated react-promise lib. If we are going to make a breaking change, let's make them all at once. Maybe we should start a v2 issue/branch.

I'll go ahead and document what we have above now. Should just be a basic commit to master and no need for a version bump or npm release.

from react-static-google-map.

See: d59079c

I am going to close this issue. Feel free to reopen it if need be.

from react-static-google-map.

You're right. The browser does handle caching well and the api responds with appropriate cache headers. We are fine as long as we generate the same URL given the same props.

The doc looks good and I agree we can make all breaking changes at once. Really, more of a v1 though since we're in 0.x.

from react-static-google-map.