Skip to content
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

[RNMobile] Cover with in Cover support #13928

Conversation

chipsnyder
Copy link
Contributor

@chipsnyder chipsnyder commented Apr 17, 2020

Fixes: wordpress-mobile/gutenberg-mobile#1739

Description

This PR implements the Cover block within a Cover Block for image uploads. Building off of #13909

This also modifies block parser for Image, Gallery, Media-Text, and Video blocks. (This was done in a separate commit so can be reverted to just Cover block if needed)

Related PRs:

To Test for Cover Block:
  • Close post with an ongoing image upload from a cover block within a cover block )
  • Close post with an ongoing image upload from replacing an image of a cover block that has a nested cover block
  • Close/Re-open post with an ongoing image upload - steps
  • Close post with an ongoing image upload - steps
To Test for other blocks:
  • Image block - Close post with an ongoing image upload - steps
  • Gallery block - Close post with an ongoing image upload - steps
  • Media Text block - Close post with an ongoing image upload - steps
  • Media Text block - Close post with an ongoing video upload - steps
  • Video block - Close post with an ongoing video upload - steps
PR submission checklist:
  • I have considered adding unit tests where possible.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Apr 17, 2020

You can trigger an installable build for these changes by visiting CircleCI here.

@chipsnyder chipsnyder mentioned this pull request Apr 17, 2020
3 tasks
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Apr 17, 2020

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

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.

Hi Chip 👋 😃 I tested this via the description, and it works as described. Also, I ran all unit tests, and everything is passing, and there is good test coverage for many nested cases 👍 .

I'll defer to @SergioEstevao for the code review, since he is very familiar with this area of the code, but the solution generally looks reasonable to me.

Copy link
Contributor

@SergioEstevao SergioEstevao left a comment

Choose a reason for hiding this comment

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

Working great!
Special Thanks for the updates on the Gutenberg block processor! We now are future proof for blocks inside blocks!

@chipsnyder chipsnyder merged commit 745363e into rnmobile/cover-block-uploads Apr 23, 2020
@chipsnyder chipsnyder deleted the rnmobile/cover-block-uploads-nested-blocks branch April 23, 2020 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants