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

Add <StreetViewGoogleMap /> component #83

Open
DaddyWarbucks opened this issue Aug 13, 2020 · 4 comments
Open

Add <StreetViewGoogleMap /> component #83

DaddyWarbucks opened this issue Aug 13, 2020 · 4 comments

Comments

@DaddyWarbucks
Copy link
Collaborator

Google supports a streetview image: https://developers.google.com/maps/documentation/streetview/overview

I am currently using this in two projects where I am hardcoding the URL, but I think this could be a nice addition to the library.

<StreetViewGoogleMap
  size="600x600"
  apiKey="***"
  location="6.4488387,3.5496361"
/>

Unlike StaticGoogleMap, I don't think this component has any children. At first, I thought this would be another child component/strategy of StaticGoogleMap but its not. This component has a different rootURL and also cannot be composed with other elements like markers, etc. It is its own "base" component I believe.

Any thoughts?

@bondz
Copy link
Owner

bondz commented Aug 17, 2020

I think this is a great idea. I do think we can do a bit more than just generating a URL if we slightly modify the api to be

<StreetViewGoogleMap
  size="600x600"
  apiKey="***"
  location="6.4488387,3.5496361"
  fallback={AnotherComponent}
/>

By default, it would check if the Image Metadata for the url exists and then render the image if it does, or render the fallback if it does not.

Still not sure the fallback prop should be a function or just a component. Thinking about how it would affect having an as prop or onGenerate callback.

What do you think?

@DaddyWarbucks
Copy link
Collaborator Author

Interesting. That's a nice find on the Metadata API. I have been experiencing the "Sorry, we have no imagery here..." and just lived with it. It's also nice to see that the Metadata API does not consume any of your Google quota.

I am not sure we should get into fallbacks. I recently revisited #76 (comment). React offers us the onLoad and onError events for img tags. The user can use those to manage their own state for placeholders, fallbacks, etc. That keeps it in user land. Perhaps we could handle this with something like

metadataApi.find(...).catch(err => props.onError(error))

and let the user handle it instead of the lib managing fallbacks. I like that because the component DUCKS like a regular img tag so the user can bring their own progressive stuff. With that said, I am not against a more integrated fallback solution, just pointing out a basic one already exists.

Should the Metadata API be opt in? Maybe the user does't want to deal with fallbacks or metadata and is fine with the synchronous URL and img tag.

The Metadata API should also probably be cached in a similar manner to the caching mechanism for the Directions component. Even though this is free, we don't want to make an asynchronous HTTP request on every render. The caching for directions was a bit wonky because I was just trying to make it fit into the current structure and is another one of those V1 reworks.

@bondz
Copy link
Owner

bondz commented Aug 18, 2020

Great. I'm all for leveraging the built-in events rather than adding additional props we have to maintain.

I just want to point out that, technically, the image itself did not error. Without the metadata check, the api would still load a valid image and that's what would happen if a user opts out of the metadata check. I don't think it's an issue though especially since we know it's not the image the user wants that'll be loaded.

Another concern, and the reason I added an as prop in the StaticGoogleMap component was to allow the user to render the component as whatever they want, like <source /> in a picture element for example and not assume the component would always be an img. That way, it should technically work even in other renderers, like ReactNative for example and that's why there's no dependency on react-dom.

@DaddyWarbucks
Copy link
Collaborator Author

Understood on the image "failure". I was actually getting 404's when initially developing my little component because I am hardcoding the URL and hardcoded them wrong 🤷. So I went ahead and added the onError handler even though they do not "error" now that the component is more mature and the URL is correct, it just shows the "No imagery..."

I was stating that we could call onError when the metadata API fails also, "mocking" a more familiar error. That may not be a great idea though... maybe it is better to differentiate the two and offer the fallback like you suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants