Skip to content

Conversation

@jd-alexander
Copy link
Contributor

@jd-alexander jd-alexander commented Dec 10, 2020

Findings

This PR is the Android side of the iOS implementation for updating the HTML content of the Audio Block without Gutenberg being open.

Solution

The AudioBlockProcessor replaces the <audio src> tag that contains local file URL with a remote media URL once the file upload is complete. The id of the Audio Block is also updated with a remote variant as well.

Testing

  1. Add an Audio block to a new post.
  2. Upload an audio file to the block (I lessened the Cellular Network type and Signal strength to replicate a slow network so I had enough time to close the editor).
  3. Close the editor by pressing the back button.
  4. Let the upload to finish in the background.
  5. When the audio file is uploaded and the post saved, open the post again (on web or mobile).

Verify that the behavior is functioning correctly by :

  • Checking that the block renders properly.
  • Checking that all references to src are updated to the remote URL.
  • Checking that the media ID has been updated to the remote one.

Reviewing

Submitter Checklist

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.
  • I have considered adding accessibility improvements for my changes.
  • If it's feasible, I have added unit tests.

@jd-alexander jd-alexander added this to the 16.4 milestone Dec 10, 2020
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Dec 10, 2020

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Dec 10, 2020

You can test the changes on this Pull Request by downloading the APK here.

@loremattei loremattei modified the milestones: 16.4 ❄️, 16.5 Dec 14, 2020
@loremattei
Copy link
Contributor

Hey! I've moved this to 16.5 because 16.4 has been cut. If you want this to make it to 16.4, please feel free to ping me.

@jd-alexander jd-alexander changed the base branch from develop to gutenberg/audio_block_consolidated_media_picker_integration December 17, 2020 05:12
@jd-alexander jd-alexander requested a review from mkevins December 17, 2020 05:41
@jd-alexander jd-alexander marked this pull request as ready for review December 17, 2020 05:42
Copy link
Contributor

@mkevins mkevins left a comment

Choose a reason for hiding this comment

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

Nice work @jd-alexander 👍

Code looks good, and thanks for writing the tests too! I've tested this on Pixel 3a (physical device) and it's working as expected.

I left one minor suggestion. Other than that, LGTM!

…loadcompletionprocessors/AudioBlockProcessor.kt


Utilized the local ID_ATTRIBUTE instead of the one from the FileBlockProcessor.

Co-authored-by: Matthew Kevins <[email protected]>
@guarani
Copy link
Contributor

guarani commented Jan 4, 2021

👋 Hi @jd-alexander, the Gutenberg editor release 1.44.0 is being cut now (apologies for the late headsup), and I came across this PR. If this doesn't depend on a specific Gutenberg editor release, and can just be picked up by the main apps release in 16.5, no action is needed.
If this does depend on a Gutenberg Mobile PRs that are not currently tagged with the 1.44.0 milestone, but needs to make it into 16.5, please let me know and we'll merge it into the frozen release if needed.

@jd-alexander jd-alexander requested review from mkevins and removed request for mkevins January 5, 2021 22:20
@jd-alexander
Copy link
Contributor Author

I will merge this in when the parent PR is reviewed so that the diff of that PR isn't hard to read.

@jd-alexander jd-alexander modified the milestones: 16.5, 16.6 Jan 8, 2021
Base automatically changed from gutenberg/audio_block_consolidated_media_picker_integration to develop January 18, 2021 16:10
@jd-alexander
Copy link
Contributor Author

I am merging this because the parent PR was mistakenly merged in and then reverted. Since this was already tested, it can be merged in, and when the parent PR is recreated it can branch from develop.

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.

5 participants