Skip to content

Handle user pressing cancel button, on Android #65

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

Closed
wants to merge 2 commits into from

Conversation

shiv19
Copy link
Member

@shiv19 shiv19 commented Nov 1, 2017

If this PR is merged,
When user cancels image capture, the promise is rejected with a message "cancelled"
{onAndroid}

If this PR is merged,
When user cancels image capture, the promise is rejected with a message "cancelled"
@andreacappadona17
Copy link

andreacappadona17 commented Nov 3, 2017

Hi, I tried adding those lines to the plugin, to check if everything was right, but on my Samsung J5 2017 the app still crashed.

Debugging, I found out the callback for "activityResult" was called twice, when you take a picture after pressing cancel button the first time.

Managed to make the app work by adding

appModule_1.android.off("activityResult");

before

appModule_1.android.on("activityResult"...

@shiv19
Copy link
Member Author

shiv19 commented Nov 3, 2017

@andreacappadona17 Thanks for pointing it out.
I've incorporated the changes. Now the PR can be merged :)

@shiv19 shiv19 changed the title Handle user pressing cancel button Handle user pressing cancel button, onAndroid Nov 3, 2017
@shiv19 shiv19 changed the title Handle user pressing cancel button, onAndroid Handle user pressing cancel button, on Android Nov 3, 2017
Copy link

@DimitarTachev DimitarTachev left a comment

Choose a reason for hiding this comment

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

Great work, @shiv19 and @andreacappadona17. Could you just fix the linting error from the Travis report in order to merge the PR. ('ERROR: camera.android.ts[87, 1]: trailing whitespace')

@jlooper
Copy link

jlooper commented Nov 3, 2017

Congratulations on this quality PR! Have a badge!
contrib

DimitarTachev pushed a commit that referenced this pull request Nov 3, 2017
@DimitarTachev
Copy link

DimitarTachev commented Nov 3, 2017

@shiv19 @andreacappadona17 I cherry-picked the changes and rebased them as we already merged PR #66 and there were some conflicts. The changes will be merges with PR #67

DimitarTachev pushed a commit that referenced this pull request Nov 3, 2017
@DimitarTachev
Copy link

The changes were merged with PR #67

@shiv19
Copy link
Member Author

shiv19 commented Nov 3, 2017

@DimitarTachev and @jlooper Thank you! 😄

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

Successfully merging this pull request may close these issues.

4 participants