[RNMobile] Audio block I #27401
Merged
[RNMobile] Audio block I #27401
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
etoledom
added
the
Mobile App - i.e. Android or iOS
2 tasks
|
Size Change: 0 B Total Size: 1.37 MB ℹ️ View Unchanged
|
This was referenced Dec 1, 2020
etoledom
force-pushed
the
rnmobile/audio-block-I-b
branch
from
6acd4ef to
276b07d
Compare
guarani
reviewed
Dec 7, 2020
There was a problem hiding this comment.
Sorry @etoledom I forgot to hit send on this from last Friday. Just minor comments (I'm not approving since I wasn't added as a reviewer here, just to wordpress-mobile/WordPress-iOS#15420).
|
@guarani - Thanks for the review. I guess you are reviewer now! 🎉 😄 I agree with the changes requested. @jd-alexander could you please take over this PR? Thanks! 🙏 |
Of course I can @etoledom 😄 |
6 tasks
3 tasks
jd-alexander
force-pushed
the
rnmobile/audio-block-I-b
branch
from
03b3144 to
d9a6041
Compare
3 tasks
|
To the Android reviewer, the changes here might be a lot but most of it has been reviewed including the caption PR (https://github.com/WordPress/gutenberg/pull/27689/files) that was merged into this has already been reviewed. The primary need here is to verify the logic changes in the Java/Android code that's related to supporting the Audio Block. The changes involved extending the To test the overall behavior of these changes in action, use this WP Android PR. Once those changes are verified then this PR and the associated PRs can be merged in. |
|
Hey @jd-alexander . The Android associated code all looks good in this PR but I wasn't able to test the media upload functionality successfully.
Heads up that we can't use the PR autogenerated APK to test because the feature is DEV only. The branch from the PR was useful for building locally to test though. Right now with any type of audio I try and upload from the device, I get an error. My guess is that uploads are failing for me because I need a test site with a premium plan, can you confirm?
|
Thanks for the review @cameronvoell
Indeed! Yes, the only way I have seen where I can use the autogenerated APK is to remove the
Yes, uploads are supposed to fail on the free sites until a fix is created that will prevent the media pickers from selecting MIME types that aren't allowed. Let me add this note to the top of the PR. |
|
Resolved a conflict that came in from master ae50b5b Re-running the End-toEnd Tests that failed. |
cameronvoell
approved these changes
Jan 22, 2021
There was a problem hiding this comment.
This is looking good from an Android perspective. I was successfully able to upload audio from my device. The media uploader correctly identified audio files on the device to choose from. And after adding audio to the block I was able to update the settings and play the audio while previewing the post. Cool feature!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
gutenberg-mobilePR: wordpress-mobile/gutenberg-mobile#2854WPiOSPR: wordpress-mobile/WordPress-iOS#15420WPAndroidPR: wordpress-mobile/WordPress-Android#13808Description
This PR implements the base for the Audio block.
Done:
BlockCaptioncomponent): [RNMobile] Audio Block: Proper caption field #27689In Progress:
To Do:
How has this been tested?
(On iOS for now)
ADD AUDIOWordPress Media Library:Screenshots
Types of changes
New mobile block.
Checklist: