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] Parent PR: Add Setting to Set/Remove a Featured Image Directly from Image Block #16427

Merged
merged 82 commits into from
Aug 2, 2021

Conversation

SiobhyB
Copy link
Contributor

@SiobhyB SiobhyB commented May 3, 2021

Fixes the iOS side of the following issue: wordpress-mobile/gutenberg-mobile#1011

gutenberg: WordPress/gutenberg#31345
gutenberg-mobile: wordpress-mobile/gutenberg-mobile#3449

Description

Introduces an option to set/remove an image as featured from the image block's settings, with a "featured" banner to denote when an image is already featured. The end goal is to make it easier for users to both identify and set/remove featured images directly from the post editor.

This PR builds on the work done in WordPress/gutenberg#30806 and WordPress/gutenberg#28854 to introduce this functionality to Android. For iOS, the work has been split up into the following PRs:

Gutenberg PR Gutenberg Mobile PR iOS PR Merged
Display a Featured banner over any image block containing a post's featured image wordpress-mobile/gutenberg-mobile#3390 #16332
Include a Set/Remove as Featured option in the image block's settings wordpress-mobile/gutenberg-mobile#3450 #16426

- Once the PR has been merged it will be updated with a

How has this been tested? Screenshots? Types of changes?

Please refer to the first PR in each row (the Gutenberg PR) for test cases, screenshots, and further details of the types of changes involved.

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).

It also adds another flow for users to set/remove featured images, so there is some potential for it to unintentionally have a negative impact on the existing flow (found via Post Settings).

A third unintended area of impact is the wpios_editor_post_featured_image_changed Track event. This event currently only fires when a featured image is changed via Post Settings. With this PR, it should also be fired when a featured image is changed via the image block, with a gutenberg property to differentiate it.

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

The steps taken to test whether the intended flow works as expected can be found in the Gutenberg PRs here and here, with additional steps for verifying the Tracks event fires as expected listed in this PR.

  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.

Siobhan added 18 commits April 19, 2021 14:20
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.
The intention behind this change is to make the function's purpose clearer. It also moves the naming inline to this feature's Android counterpart. (See: wordpress-mobile/WordPress-Android#14451.)
…eId"

This change is a bid align closer to the naming conventions of other functions in iOS.
This function will handle the logic for setting/removing a featured when the button is tapped.
This function accepts a media ID for an image and uses the "existingMediaWith" to determine its media object, which can then be used to assign the image as the post's featured image. Lastly, the ID of the new featured image is then sent to Gutenberg via featuredImageIdNativeUpdated().
This commit includes different notices that will be sent to Gutenberg, depending on whether an image is being removed (which will be the case when the media ID is zero) or set.
This function will be called when a post already has a featured image and will prompt the user on whether they wish to replace the current featured image or not.
This commit introduces logic into the gutenbergDidRequestToSetFeaturedImage() function. Specifically, an if/else statement is used to either set, remove, or show an alert to confirm a replacement, depending on the state of featured images in the post.
@SiobhyB SiobhyB added the Gutenberg Editing and display of Gutenberg blocks. label May 3, 2021
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented May 3, 2021

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

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented May 3, 2021

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

@SiobhyB SiobhyB changed the title [RNMobile] Main PR: Image Block: Add Functionality to Set/Remove Featured Images [RNMobile] Main PR: Add Setting to Set/Remove a Featured Image Directly from Image Block May 3, 2021
@SiobhyB SiobhyB changed the title [RNMobile] Main PR: Add Setting to Set/Remove a Featured Image Directly from Image Block [RNMobile] Parent PR: Add Setting to Set/Remove a Featured Image Directly from Image Block May 3, 2021
Siobhan and others added 2 commits June 24, 2021 21:45
With this commit, the iPad-specific popoverController is centred in the middle of the screen. The "permittedArrowDirections" value is also changed to remove any arrow and ensure the dialog's message is displayed in full (rather than only the title).
…featured-button

[RNMobile] Add "Set as Featured" Button to Image Block
@SiobhyB SiobhyB force-pushed the gutenberg/add/featured-functionality-to-image-block branch from d552f4a to 674bcc7 Compare June 30, 2021 10:57
@SiobhyB SiobhyB force-pushed the gutenberg/add/featured-functionality-to-image-block branch 2 times, most recently from 1c973d5 to d552f4a Compare July 1, 2021 21:22
@SiobhyB
Copy link
Contributor Author

SiobhyB commented Jul 1, 2021

👋 @illusaen, do you perhaps have any availability to review this PR either this week or next? This is the main feature branch for the Set/Remove Featured Image project that you've already helped me with, so all the code here has already been reviewed through other PRs and it's in need of a final overall review/test.

There are two known issues that will be worked on separately and I don't believe to be blockers to this one being merged:

  • There are some occasions where the notice that's displayed after clicking the button flickers. I'll be creating a separate issue for this, but it can't be reproduced consistently and I've seen it in other places where the notice is called, so I don't believe it to be specific to this PR.
  • The undo/redo functionality doesn't work correctly when used immediately after setting a featured image, which I'm working on in [RNMobile] Prevent "Undo Level" after Setting Featured Image via Image Block WordPress/gutenberg#33057.

Let me know how that sounds to you and thanks in advance!

@SiobhyB SiobhyB marked this pull request as ready for review July 1, 2021 22:32
@SiobhyB
Copy link
Contributor Author

SiobhyB commented Jul 7, 2021

@illusaen, just pinging again on this. :) Do you have any availability to review this PR?

@SiobhyB SiobhyB requested a review from illusaen July 22, 2021 15: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.

Looks good to me!

@SiobhyB
Copy link
Contributor Author

SiobhyB commented Jul 28, 2021

@illusaen, thank you! Would you also be able to approve the Gutenberg and Gutenberg Mobile PRs? I can then go ahead to merge these. :)

@SiobhyB SiobhyB added this to the 18.0 milestone Aug 2, 2021
@SiobhyB SiobhyB enabled auto-merge August 2, 2021 18:01
@SiobhyB SiobhyB merged commit 6eb7807 into develop Aug 2, 2021
@SiobhyB SiobhyB deleted the gutenberg/add/featured-functionality-to-image-block branch August 2, 2021 18:42
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. [Type] Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants