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] Add Featured Banner to Image Block #16332

Conversation

SiobhyB
Copy link
Contributor

@SiobhyB SiobhyB commented Apr 19, 2021

Paritial fix for: wordpress-mobile/gutenberg-mobile#1011

gutenberg: WordPress/gutenberg#31347
gutenberg-mobile: wordpress-mobile/gutenberg-mobile#3390

This PR introduces a Featured banner on iOS devices, which overlays a post's featured image when it's added to an image block. This will make it clear to users which of their images, if any, are set as the post's featured image.

The main PR for the branch this individual PR will be merged into, gutenberg/add/featured-functionality-to-image-block-ios, can be found here: #16427

To test: Please refer to the Gutenberg PR as the central place for steps to tests and screenshots.

Regression Notes

  1. Potential unintended areas of impact

As this PR adds a banner over the image block, there is some potential for it to unintentionally have a negative impact on the existing UI and/or animations that take place when an image is replaced within the block (depending on whether it's featured or not).

  1. What I did to test those areas of impact (or what existing automated tests I relied on)

The steps I took to test whether the intended flow works as expected can be found in the linked Gutenberg PR. As this is a UI change, I also tested that the display looks correct in landscape and in dark mode.

  1. What automated tests I added (or what prevented me from doing so)

No automated tests.

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • 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.

@SiobhyB SiobhyB added [Type] Enhancement Gutenberg Editing and display of Gutenberg blocks. labels Apr 19, 2021
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Apr 19, 2021

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

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Apr 19, 2021

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

Siobhan added 3 commits April 22, 2021 12:12
This protocol includes a method named didSetFeaturedImage, which will send a message to the JS side when a featured image is set from Post Settings.
@SiobhyB SiobhyB changed the base branch from develop to gutenberg/add/featured-functionality-to-image-block April 23, 2021 10:35
…eId"

This change is a bid align closer to the naming conventions of other functions in iOS.
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Apr 29, 2021

Warnings
⚠️ This PR is tagged with 'DO NOT MERGE'.
⚠️ PR is not assigned to a milestone.

Generated by 🚫 dangerJS

@SiobhyB SiobhyB force-pushed the gutenberg/add/featured-badge branch 2 times, most recently from 3d2cafa to 6d67735 Compare April 29, 2021 20:35
Copy link
Contributor

@illusaen illusaen left a comment

Choose a reason for hiding this comment

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

LGTM! Just remember to change the commit after merging the others :) Also update the GBMobile one after merging gutenberg.

@SiobhyB SiobhyB merged commit 75fce42 into gutenberg/add/featured-functionality-to-image-block May 6, 2021
@SiobhyB SiobhyB deleted the gutenberg/add/featured-badge branch May 6, 2021 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Gutenberg Editing and display of Gutenberg blocks. [Status] DO NOT MERGE [Type] Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants