[Android] CameraRoll support for Videos #13715
Conversation
|
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 up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks! If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact [email protected] if you have any questions. |
kesha-antonov
changed the title
facebook-github-bot
added
the
CLA Signed
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
|
Also I've noticed that vertical videos do have wrong dimensions. My workaround for now is to listen By wrong dimensions I mean: Maybe you guys can tell me how to get correct dimensions straight from android CameraRoll? |
|
Hey @kesha-antonov , nice beard. |
|
Hey! @zr0n Thanks! Did you build android after merging? |
|
@kesha-antonov Yes, twice. But when I open the picker, keep showing only photos. Even if I use the assetType="All" it does not show video files. |
|
I'd suggest to pass some other strings "somestring" as "assetType" Telling this because it's working for me in simulator and a real device. |
Summary: Currently, Android camera roll videos cannot be retrieved in RN since 1) `CameraRollManager.java` doesn't do anything with the `assetType` param 2) Unspecifying MIME types doesn't show videos This diff allows videos to be shown in the `CameraRoll.getPhotos(..)` call by reading `assetType`. Future diffs will come where the thumbnail and other info will be returned as well. Reviewed By: furdei Differential Revision: D5019202 fbshipit-source-id: a920273761b31f1a59ba6b8bc49c05852506829c
|
@kesha-antonov Any Idea of what is going wrong? |
|
@kesha-antonov Whats the status of this PR? |
| @@ -289,69 +324,70 @@ protected void doInBackgroundGuarded(Void... params) { | |||
| // setting a limit at all), but it works because this specific ContentProvider is backed by | |||
| // an SQLite DB and forwards parameters to it without doing any parsing / validation. | |||
| try { | |||
| Cursor photos = resolver.query( | |||
| Images.Media.EXTERNAL_CONTENT_URI, | |||
| Cursor medias = resolver.query( | |||
There was a problem hiding this comment.
change to media. Medias is a wrong phrase.
There was a problem hiding this comment.
Same. Why media? We get multiple items
| private final @Nullable ReadableArray mMimeTypes; | ||
| private final Promise mPromise; | ||
|
|
||
| private GetPhotosTask( | ||
| private GetMediasTask( |
There was a problem hiding this comment.
Change to GetMediaTask. I'm pretty sure that one medium, multiple medias. Check: https://english.stackexchange.com/questions/314099/should-i-use-media-or-medias
There was a problem hiding this comment.
I think here we get medias - multiple. Isn't it? Previously we get photos. What's wrong here?
There was a problem hiding this comment.
Media is already plural, the singular is medium.
Reviewed By: brosenfeld Differential Revision: D5560628 fbshipit-source-id: b1457493e8429958fbd7bc9c490cffaa33b4a95a
Summary: I added troubleshooting docs for connection to the development server from device, otherwise it's maybe hard to find out why it's not working. Due to a disparity in what GitHub's API return the original PR (#14885) couldn't get imported, so I created a new PR as hramos suggested. Closes #15304 Differential Revision: D5537615 Pulled By: hramos fbshipit-source-id: b94e887b1b771be9e93854124bd0a56b27fd0097
Reviewed By: furdei Differential Revision: D5558385 fbshipit-source-id: 11f00acbbdf61bfa4b7e86675b0912d6edc39e9b
Summary: Same as #11819 but for Android. I didn't notice the bug initially in my app because I was using different animations on Android which did not trigger this issue. **Test plan** Created a simple repro example and tested that this fixes it. https://gist.github.com/janicduplessis/0f3eb362dae63fedf99a0d3ee041796a Closes #12842 Differential Revision: D5556439 Pulled By: hramos fbshipit-source-id: d13f4ad258d03cca46c793751ebc49d942b99152
Reviewed By: ericvicenti Differential Revision: D5519379 fbshipit-source-id: a07ea0629f98c23e1027202cc7a7957233780643
Summary: What existing problem does the pull request solve? Ensure users of the React Native platform are aware of transform properties they must have while using `useNativeDriver` with the Animated API. Not applicable. I've been messing around with this for a couple of days at this point and feel a bit silly. See this issue [here](#13522). This would have saved me some time and will likely save others time as well. Thank you for taking the time to look over this PR. Cheers! Closes #13524 Differential Revision: D5567584 Pulled By: hramos fbshipit-source-id: 4be9f1ba0f21148106f596efb0be791d4d364a66
Summary: Currently the default way to setup _fastlane_ for ReactNative projects is to set it up in the `ios` or `android` subfolder. This PR updates the path and also the URL to the new fastlane docs page. Closes #13261 Differential Revision: D5567604 Pulled By: hramos fbshipit-source-id: 89c27328bb2748ff1772812786e2821963dc1779
… values when removing props on Android Differential Revision: D5556439 fbshipit-source-id: dc0e4c1db25ec7f3631e6f684f9497962f2adc7b
Summary: Several changes here. The `Text.md` and `PixelRatio.md` files were appended to the autodocs during site generation. This seemed excessive for just two files, so I've just merged the content into the autodocs themselves. It should help us simplify the website generation process in the future. I've also merged IssueGuidelines.md and PullRequestGuidelines.md into the Contribution/Maintainers guidelines to improve their visibility. Finally, I renamed Help to Community in the nav bar. Ran the website locally, and verified every page rendered as expected: the Community page, Contributing page, Maintainers page. Closes #15374 Differential Revision: D5567400 Pulled By: hramos fbshipit-source-id: e06056edb12c9a17319fe1af46b2ef3a2e1b5854
Summary: Adding the proper way to setup Stetho debugging, based on this post: https://medium.com/andr3wjack/stetho-with-react-native-87642e5d7131#.352jqqavc with updated OkHttp API. **Test plan** + Manual test, DYI. Closes #12232 Differential Revision: D5568733 Pulled By: hramos fbshipit-source-id: 6006b9daa48024742b948f5fd33a07545549397c
Summary:
This avoids a crash when we try to load a PHAsset with nil image url. Specifically, the following condition evaluates to true when `imageURL` is nil:
```objc
if ([imageURL.scheme caseInsensitiveCompare:@"assets-library"] == NSOrderedSame) {
assetID = [imageURL absoluteString];
results = [PHAsset fetchAssetsWithALAssetURLs:@[imageURL] options:nil];
}
```
The crash will be "attempt to insert nil object from objects[0]" when we build the `@[imageURL]` array literal.
We've seen this emerge as a very common crash among Expo users, so I wanted to at least provide a clear error message instead of terminating the app.
Load an image from the photo library with a nil request url.
Closes #15952
Differential Revision: D5835219
Pulled By: ericnakagawa
fbshipit-source-id: 7be00a15e674a0905cf5c27c526ce9085d1b308f
|
Hi! |
Summary: React Native v0.48.0 shipped `WebSocketModule` support with `BlobModule` as dependency. But `BlobModule` is not mocked in jest which will cause render tests failed. Reference implantation: [BlobModule.java](https://github.com/facebook/react-native/blob/ed903099b42259958d8a6eb3af1a6460c1bc8b2c/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.java) Related PR: #11417 Related issue: #15810 Passed CI tests. Need render a component in jest with WebSocketModule as dependency. Closes #15843 Differential Revision: D5783263 Pulled By: shergin fbshipit-source-id: 2386692f4a644796da2fd66b3135da9f5911663e
Summary: Basic implementation of the proposal in #15271 Note that this should not affect facebook internally since they are not using OSS releases. Points to consider: - How strict should the version match be, right now I just match exact versions. - Wasn't able to use haste for ReactNativeVersion because I was getting duplicate module provider caused by the template file in scripts/versiontemplates. I tried adding the scripts folder to modulePathIgnorePatterns in package.json but that didn't help. - Redscreen vs. warning, I think warning is useless because if the app crashes you won't have time to see the warning. - Should the check and native modules be __DEV__ only? **Test plan** Tested that it works when version match and that it redscreens when versions don't before getting other errors on Android and iOS. Closes #15518 Differential Revision: D5813551 Pulled By: hramos fbshipit-source-id: 901757e25724b0f22bf39de172b56309d0dd5a95
Summary: This lets us import fishhook like `<fishhook/fishhook.h>`. Closes #16192 Differential Revision: D5971639 Pulled By: shergin fbshipit-source-id: c46503953a08a74bdd10ed35cbd30029f1666c7d
Differential Revision: D5839414 fbshipit-source-id: 7a0c7b3d27be728c74e08d3a8209deb3ca68dd63
Reviewed By: yungsters Differential Revision: D5857305 fbshipit-source-id: 2fc20a848fa4dce5c1ac3fb7e986536618e25548
… is present Summary: In some enviroments PlatformConstants native module may not be presented in a project, which results in a call to undefined property and a RedBox Reviewed By: javache Differential Revision: D5960879 fbshipit-source-id: 80aecbe2f2a61cb410abd5f0dce8ba855e166991
Summary: Fails on my machine due to fact that `replace` returns an instance of a String, rather than an instance of ShellString (that includes `to` on its prototype). Solution is to use an explicit `writeFileSync`. You can see that change in the wild on 0.50-stable branch. CC janicduplessis (edit by hramos) Closes #16303 Differential Revision: D6031331 Pulled By: hramos fbshipit-source-id: 41c583d53df75bea1a55fa19174d912e414209c0
kesha-antonov
requested review from
grabbou,
hramos,
janicduplessis,
Kureev,
mhorowitz and
shergin
as code owners
@facebook-github-bot label Documentation @facebook-github-bot large-pr Attention: @hramos, @janicduplessis, @shergin, @mhorowitz, @grabbou, @Kureev Generated by 🚫 dangerJS |
|
Hey, I missed the notifications here. Looks like a merge with master added a bunch of unrelated commits. If you fix this, in a new PR, maybe we can continue working on getting this merged. |
|
Hi,
How can I fix it ? |
|
Hi @WemagineGuillaume Try this (new PR) #16429 I do not support this anymore |
Motivation
[Android] CameraRoll does not support videos. This PR adds ability to get videos too.
Test Plan
I don't know how to test it automatically. I even wasn't able to build source code and ran just
./gradlew :ReactAndroid:installArchives && cp ./android ...everytime I changed the code.And I didn't find any tests for CameraRoll so can't provide any tests. I'm a beginner in java dev.
Please guide me in this.
P.S.
This is a fork from this PR: #11877
See some details there