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 support for JSON response capture #2

Merged
merged 10 commits into from
Aug 1, 2022
Merged

Conversation

depsimon
Copy link
Contributor

@depsimon depsimon commented Jul 29, 2022

This PR adds support for capturing the full JSON response from the autocompletion.

Useful when we don't want to type all the values in fromValues().

Solves #1

Copy link
Member

@JamesHemery JamesHemery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What you are looking to do is get the raw answer from the google api? As it stands locationObject does not contain the google response, but a reworked object (based on withValues).

See https://github.com/YieldStudio/nova-google-autocomplete/blob/main/resources/js/components/GoogleAutocomplete/FormField.vue#L44

I would have thought of adding a template variable {{response}} which like the others would simply add the raw google response.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@depsimon
Copy link
Contributor Author

I don't actually want the raw response, but the retrieved address (based on the values passed) so this is fine.

It could be an addition to add support for the raw response, but at that point we'd probably need support to ignore some keys, I'm not sure. I guess if you want all the fields, you can still add them all to withValues().

What do you think?

Copy link
Member

@JamesHemery JamesHemery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from these last details, it seems coherent to me.

src/AddressMetadata.php Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@depsimon
Copy link
Contributor Author

Updated with the latest requests, thanks for reviewing 👍

@JamesHemery
Copy link
Member

Thank you for your contribution 🙌

@JamesHemery JamesHemery merged commit c2782e5 into YieldStudio:main Aug 1, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants