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 Android Place Photos Support #154

Open
wants to merge 12 commits into
base: place-photos
Choose a base branch
from

Conversation

thomasw
Copy link

@thomasw thomasw commented Jul 6, 2018

As discussed in #149 (comment) (with some slight modifications), this adds the following Google Place Photo API wrappers for android:

> RNGooglePlaces.getPlacePhotosMetadata(placeID)
[{
  placeId: '...',
  photoId: '...',
  attributions: '...',
  maxWidth: ...,
  maxHeight: ....
},
...
]
// Same as above, but it saves all the images to disk and returns a uri for each.
> RNGooglePlaces.getPlacePhotos(placeID)
[{
  placeId: '...',
  photoId: '...',
  attributions: '...'
  maxWidth: ...,
  maxHeight: ...,
  uri: 'file://...'
},
...
]
> RNGooglePlaces.getPlacePhoto(placePhotoMeta)
'file://...'
> RNGooglePlaces.getScaledPlacePhoto(placePhotoMeta, height, width)
'file://...'

thomasw added 12 commits June 29, 2018 15:11
Files should always end with new lines to avoid introducing extraneous diffs.
This introduces preliminary support for 1. retrieving photo metadata for a place and 2. persisting the associated photos to disk where they can be accessed via react-native using an `<Image>` tag.

Example usage via JS:

```javascript
const placePhotos = await RNGooglePlaces.getPlacePhotos(placeId)
const firstPhotoUri = await RNGooglePlaces.getPlacePhoto(placePhotos[0])

console.info(firstPhotoUri)
```
Place photos shouldn't be persisted indefinitely. We write them to a cache directory so that the OS knows that it can clean these files up if necessary.

This also removes specifying context.MODE_PRIVATE when creating the FileOutputStream, which doesn't seem necessary.
This makes the return type consistent with the method's current naming.
File.toURI returns a URI, not a Uri.
ResultCallbacks (vs ResultCallback) allows us to factor out status checks because we can define specific handlers for success/failure cases.

ResultTransform allows us to neatly chain API calls without needing to pass promise references down deeply into multiple methods.
I'd believed that this wasn't required when using .then(...).andFinally(...) pattern, but according to the console warnings this was resulting in, it is indeed required.
@devlinb
Copy link

devlinb commented Sep 7, 2018

Any chance of this getting merged? This is great functionality that I'd love to see added!

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