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

feat: Implemented Voice Search #1358

Merged
merged 5 commits into from
Mar 24, 2019
Merged

Conversation

somenath1435
Copy link
Contributor

Fixes #1357

Changes: Voice Search is used.
proper permission is taken.
It is a necessary feature.

Screenshots for the change:

voice

@somenath1435
Copy link
Contributor Author

@nikit19 @iamareebjamal please help

@aggarwalpulkit596
Copy link
Contributor

There is a wildcard import in one of you files which is causing the Travis build fail

@somenath1435
Copy link
Contributor Author

@aggarwalpulkit596 Can you suggest a way to fix it?

@aggarwalpulkit596
Copy link
Contributor

Just remove any generic import ending with .*

@ShridharGoel
Copy link
Member

Build is failing due to format violations

@somenath1435
Copy link
Contributor Author

@ShridharGoel I had carefully refactored the code in android studio before pushing to github... Can you suggest a way to fix the error?

@iamanbansal
Copy link
Contributor

@ShridharGoel I had carefully refactored the code in android studio before pushing to github... Can you suggest a way to fix the error?

Run this command 'gradlew spotlessApply' to fix the error

@ShridharGoel
Copy link
Member

Also, the UI needs to be better. No need to write 'Use Voice Search' just the icon is sufficient and that icon should be at the right end of the 'Enter a location' field, this is what I would suggest.

@somenath1435
Copy link
Contributor Author

@iamareebjamal @nikit19 please review

@iamareebjamal
Copy link
Member

UI is looking really bad. Take a look at existing apps which use voice search

@iamareebjamal
Copy link
Member

Still, do you seriously think it looks good?

@somenath1435
Copy link
Contributor Author

How about this?
Screenshot_20190321-051718

@liveHarshit
Copy link
Member

Just check voice search in play store and other applications.

@somenath1435
Copy link
Contributor Author

@liveHarshit please see this:-
Screenshot_20190323-074349

Copy link
Member

@iamareebjamal iamareebjamal left a comment

Choose a reason for hiding this comment

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

Use vector icon

@somenath1435
Copy link
Contributor Author

@iamareebjamal Used vector icon. PR is updated

@somenath1435
Copy link
Contributor Author

@iamareebjamal Any other changes required?

@somenath1435
Copy link
Contributor Author

@iamareebjamal I think this PR is ready.

@iamareebjamal
Copy link
Member

As told before, ask peer review first

@somenath1435
Copy link
Contributor Author

@liveHarshit What's your opinion?

app/src/main/AndroidManifest.xml Outdated Show resolved Hide resolved
@@ -26,6 +27,6 @@
</activity>
<meta-data
android:name="com.stripe.android.API_KEY"
android:value="${STRIPE_API_TOKEN}"/>
android:value="${STRIPE_API_TOKEN}" />
Copy link
Member

Choose a reason for hiding this comment

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

Extra

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since I've added RECORD_AUDIO permission in manifest, everyline went down by one.
I've checked it in my IDE.
In mine, last line is line30.
No extra space is there.

@somenath1435
Copy link
Contributor Author

@liveHarshit I'll make necessary changes and update accordingly.

@somenath1435
Copy link
Contributor Author

@liveHarshit PR is updated

@somenath1435
Copy link
Contributor Author

@liveHarshit please see the code now

@somenath1435
Copy link
Contributor Author

@iamareebjamal please review

@iamareebjamal iamareebjamal merged commit 5b797ae into fossasia:development Mar 24, 2019
iamareebjamal added a commit that referenced this pull request Mar 24, 2019
iamareebjamal added a commit that referenced this pull request Mar 24, 2019
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.

6 participants