Skip to content

feat(android): add support for mediaType option for android devices #277

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

xuhcc
Copy link
Contributor

@xuhcc xuhcc commented Jul 8, 2019

PR Checklist

What is the current behavior?

mediaType option only works on iOS.

What is the new behavior?

mediaType option supported on Android too.

Implements #175.

@cla-bot
Copy link

cla-bot bot commented Jul 8, 2019

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign the CLA at https://www.nativescript.org/cla.
CLA has not been signed by users: @xuhcc.
After signing the CLA, you can ask me to recheck this PR by posting @cla-bot check as a comment to the PR.

@xuhcc
Copy link
Contributor Author

xuhcc commented Jul 8, 2019

@cla-bot check

1 similar comment
@xuhcc
Copy link
Contributor Author

xuhcc commented Jul 8, 2019

@cla-bot check

@cla-bot
Copy link

cla-bot bot commented Jul 8, 2019

The cla-bot has been summoned, and re-checked this pull request!

@cla-bot cla-bot bot added the cla: yes label Jul 8, 2019
Copy link
Contributor

@tgpetrov tgpetrov left a comment

Choose a reason for hiding this comment

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

@xuhcc
Thank you for your PR. However, the demo app, currently crashes with Cannot read property 'Image' of undefined error. It seem like the ImagePickerMediaType enumeration is not recognised in the imagepicker.android.ts file. Can you check?

@xuhcc
Copy link
Contributor Author

xuhcc commented Jul 23, 2019

Hi @tgpetrov

I just tried both npm run demo.android and npm run demo.ng.android and they are working fine for me.

@xuhcc
Copy link
Contributor Author

xuhcc commented Jul 23, 2019

I also merged latest commits from master into this branch. Unfortunately npm run demo.ng.android started to crash after that, but it does not seem to be caused by my commit:

Exception in thread "main" java.lang.RuntimeException: Class not found!
	at org.nativescript.staticbindinggenerator.generating.parsing.classes.hierarchy.generics.impl.GenericsAwareClassHierarchyParserImpl.getJavaClassFromCache(GenericsAwareClassHierarchyParserImpl.java:320)
	at org.nativescript.staticbindinggenerator.generating.parsing.classes.hierarchy.generics.impl.GenericsAwareClassHierarchyParserImpl.getClassHierarchyRecursively(GenericsAwareClassHierarchyParserImpl.java:137)
	at org.nativescript.staticbindinggenerator.generating.parsing.classes.hierarchy.generics.impl.GenericsAwareClassHierarchyParserImpl.getClassHierarchy(GenericsAwareClassHierarchyParserImpl.java:54)
	at org.nativescript.staticbindinggenerator.Generator.createExtendedClassGenericHierarchyView(Generator.java:314)
	at org.nativescript.staticbindinggenerator.Generator.writeBinding(Generator.java:290)
	at org.nativescript.staticbindinggenerator.Generator.generateBinding(Generator.java:171)
	at org.nativescript.staticbindinggenerator.Generator.processRows(Generator.java:234)
	at org.nativescript.staticbindinggenerator.Generator.generateBindings(Generator.java:121)
	at org.nativescript.staticbindinggenerator.Generator.writeBindings(Generator.java:97)
	at org.nativescript.staticbindinggenerator.Main.main(Main.java:48)

@tgpetrov
Copy link
Contributor

@xuhcc It seems like the issue I was hitting was related to a braking change in NativeScript CLI 6.0, so I had to include the ImagePickerMediaType in a ts file (and not only d.ts as until now). When I applied your changes, the checks were successful. Thank you for your contribution!
P.S. Due to a limitation, our checks never pass when a PR is created from fork, so I'm going to merge the PR I created and close this one.

@tgpetrov tgpetrov closed this Jul 24, 2019
@xuhcc
Copy link
Contributor Author

xuhcc commented Jul 24, 2019

OK, thank you!

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

Successfully merging this pull request may close these issues.

3 participants