[RNMobile] Add "Set as Featured" Button to Image Block #16426

SiobhyB · 2021-05-03T09:32:48Z

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

gutenberg: WordPress/gutenberg#31415
gutenberg-mobile: wordpress-mobile/gutenberg-mobile#3450

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

Description

This PR will add a Set as Featured button to the image block's settings on iOS devices, with the purpose being to make it simpler for users to set a featured image within the post's editor. Users will also be able to Remove as Featured directly from the block's setting.

Testing

Main Test Flows

The Gutenberg PR can be referred to as the central, most up-to-date repository for the main test flows. In addition, we should verify that the Tracks events edited/introduced with this PR work as expected.

Tracks Verification

After following the steps to set and remove an image as featured directly from the image block, navigate to the internal Tracks tool and choose the Live View option from the menu.
Enter wpios_editor_post_featured_image_changed into the tool's Events field and then the username associated with the account you tested with in the Username field. Click Search.
Verify that the event was logged correctly. Note, there is usually a delay of ~5 minutes before an event is logged in this tool.
Click Props and scroll down to the eventprops section. The following props should be logged, depending on your actions:
-- action should either log removed or added, depending on whether you have removed or set a featured image.
-- via should log gutenberg when you set/remove a featured image via the image block or settings when this is done via Post Settings.

Regression Notes

Potential unintended areas of impact

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

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

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 PR with additional steps for verifying the Tracks event fires as expected listed above.

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

N/A

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.

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.

… data over the bridge

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.

…to gutenberg/add/featured-badge

This function will handle the logic for setting/removing a featured when the button is tapped.

peril-wordpress-mobile · 2021-05-03T09:32:52Z

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

peril-wordpress-mobile · 2021-05-03T09:32:53Z

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

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.

guarani · 2021-06-02T22:40:46Z

Just a heads-up that the unit tests seem to be failing and I'm seeing an error when the block editor opens. Once this is done I can test the Tracks events which I'd forgotten to do in the first review.

…to gutenberg/add/set-as-featured-button

It's necessary to dismiss then present controller at the moment due to an issue with BottomSheet. See hee for further discussion: https://github.com/wordpress-mobile/WordPress-iOS/pull/16426/files#r643544481 With this commit, the logic around dismissing the controller is refactored to make use of the completion handler. An if/else statment is used to account for any cases where a controller isn't present (i.e. if the issue with BottomSheet is fixed in the future).

SiobhyB · 2021-06-03T17:56:31Z

@guarani, sorry for missing that error. 🤦‍♀️

I've double-checked and everything should work without error with the latest changes on this branch now.

guarani · 2021-06-03T20:14:37Z

Tracks Verification

Tested 287307b:

Tracks events logged correctly when adding and removing a feature image via the Image block
Tracks events logged correctly when adding and removing a feature image via Post Settings

iPads display action sheets inside a popover, which wasn't accounted for in the code, leading to a crash when the 'replace featured' dialog was called. The crash is fixed with this commit, which introduces support for iPad's popovers.

…to gutenberg/add/set-as-featured-button

This commit refactors the code for iPad-specific popoverController. It's now more inline with similar code elsewhere in the GutenbergViewController.swift file, for example here: https://github.com/wordpress-mobile/WordPress-iOS/blob/develop/WordPress/Classes/ViewRelated/Gutenberg/GutenbergViewController.swift#L746

SiobhyB · 2021-06-21T23:37:30Z

@guarani, this is ready for the next review when you have a chance. :) I've updated all the PRs in this chain so that they're up-to-date following HACK week and my recent AFK. The most recent changes also include a fix for the iPad-specific crash you surfaced.

SiobhyB · 2021-06-24T10:18:40Z

Highlighting that I found an issue where the button breaks the editor's undo/redo functionality, which I'll work on in a separate issue. I'd like to propose merging this issue into the main feature branch (if no other issues are found in this PR chain) irrespective of that particular known issue, as it'll be worked on separately to this.

guarani

I tested this on an iPad and the replace action sheet is being shown right at the top of the screen which looks incorrect.

RPReplay_Final1624555904.MP4

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

SiobhyB · 2021-06-24T20:51:28Z

Thanks @guarani, I've gone ahead to make some changes in 8f47364 so that the dialog is always centred in the screen. I also removed the arrow so that the message is displayed (rather than only the title, which seems to happen when an arrow is specified), which I think looks a bit better in this case:

Let me know what you think!

guarani

Code looks great and works as expected! 💯
Approved via WordPress/gutenberg#31415 (review)
Thanks @SiobhyB!

SiobhyB added 12 commits April 19, 2021 14:20

Pass ID of post's featured image over the bridge when editor first loads

9c88408

Declare new "FeaturedImageDelegate" protocol

4933e76

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.

Call didSetFeaturedImage when a featured image is set

da1a801

Call didSetFeaturedImage in GutenbergViewController.swift

869aa3d

Establish connection between PostSettings and GutenbergViewController

19d7afd

Place didSetFeaturedImage() amongst other functions that send/receive…

8847ad6

… data over the bridge

Set didSetFeaturedImage() to nil when featured image is removed

01145b8

Remove trailing whitespace

6275afc

Rename didSetFeaturedImage() to sendToJSFeaturedImageId()

da208ce

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

Renamed "sendToJSFeaturedImageId" to "gutenbergDidRequestFeaturedImag…

6d67735

…eId" This change is a bid align closer to the naming conventions of other functions in iOS.

Merge branch 'gutenberg/add/featured-functionality-to-image-block' in…

4f5d3a1

…to gutenberg/add/featured-badge

Add gutenbergDidRequestToSetFeaturedImage() function

9799b3e

This function will handle the logic for setting/removing a featured when the button is tapped.

SiobhyB added 5 commits May 3, 2021 10:35

Build out notices to send when a new featured image is set

6960847

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.

Add showAlertForReplacingFeaturedImage() function

ef85b39

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.

Build out alertController for showAlertForReplacingFeaturedImage()

72955ee

SiobhyB changed the title ~~RNMobile] Add "Set as Featured" Button to Image Block~~ [RNMobile] Add "Set as Featured" Button to Image Block May 3, 2021

SiobhyB added 2 commits May 3, 2021 17:10

Remove trailing whitespace

8933851

Tidy up if/else logic in gutenbergDidRequestToSetFeaturedImage()

e333fdb

SiobhyB mentioned this pull request May 6, 2021

[RNMobile] Add "Set as Featured Image" Button to Image Block (iOS Only) wordpress-mobile/gutenberg-mobile#3450

Merged

2 tasks

SiobhyB added Gutenberg Editing and display of Gutenberg blocks. Part of a WIP Feature [Status] DO NOT MERGE labels May 6, 2021

SiobhyB added 2 commits June 2, 2021 21:24

Update Gutenberg Mobile reference

d7c11a1

Podfile changes

f265b0a

SiobhyB added 6 commits June 3, 2021 09:43

Merge branch 'gutenberg/add/featured-functionality-to-image-block' in…

aa5b398

…to gutenberg/add/set-as-featured-button

Podfile changes

b800356

Podfile changes

5b886aa

Merge branch 'gutenberg/add/featured-functionality-to-image-block' in…

d606737

…to gutenberg/add/set-as-featured-button

Remove trailing whitespace

287307b

SiobhyB marked this pull request as ready for review June 3, 2021 17:43

SiobhyB added 7 commits June 4, 2021 14:03

Podfile and Podfile.lock changes

41308dd

Make 'replace' dialog work with iPad's popovers

74393ca

iPads display action sheets inside a popover, which wasn't accounted for in the code, leading to a crash when the 'replace featured' dialog was called. The crash is fixed with this commit, which introduces support for iPad's popovers.

Merge branch 'gutenberg/add/featured-functionality-to-image-block' in…

3086263

…to gutenberg/add/set-as-featured-button

Remove trailing whitespace

68187f3

Podfile changes

cb9e202

Podfile changes

791ebd7

retrigger checks

2283a4e

guarani reviewed Jun 24, 2021

View reviewed changes

guarani approved these changes Jun 24, 2021

View reviewed changes

SiobhyB merged commit e1c668b into gutenberg/add/featured-functionality-to-image-block Jun 25, 2021

SiobhyB deleted the gutenberg/add/set-as-featured-button branch June 25, 2021 09:19

SiobhyB removed Part of a WIP Feature [Status] DO NOT MERGE labels Jul 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[RNMobile] Add "Set as Featured" Button to Image Block #16426

[RNMobile] Add "Set as Featured" Button to Image Block #16426

Uh oh!

SiobhyB commented May 3, 2021 •

edited

Loading

Uh oh!

peril-wordpress-mobile bot commented May 3, 2021 •

edited

Loading

Uh oh!

peril-wordpress-mobile bot commented May 3, 2021 •

edited

Loading

Uh oh!

guarani commented Jun 2, 2021

Uh oh!

SiobhyB commented Jun 3, 2021 •

edited

Loading

Uh oh!

guarani commented Jun 3, 2021 •

edited

Loading

Tracks Verification

Uh oh!

SiobhyB commented Jun 21, 2021

Uh oh!

SiobhyB commented Jun 24, 2021

Uh oh!

guarani left a comment

Uh oh!

SiobhyB commented Jun 24, 2021

Uh oh!

guarani left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[RNMobile] Add "Set as Featured" Button to Image Block #16426

[RNMobile] Add "Set as Featured" Button to Image Block #16426

Uh oh!

Conversation

SiobhyB commented May 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Main Test Flows

Tracks Verification

Regression Notes

Uh oh!

peril-wordpress-mobile bot commented May 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peril-wordpress-mobile bot commented May 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

guarani commented Jun 2, 2021

Uh oh!

SiobhyB commented Jun 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

guarani commented Jun 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tracks Verification

Uh oh!

SiobhyB commented Jun 21, 2021

Uh oh!

SiobhyB commented Jun 24, 2021

Uh oh!

guarani left a comment

Choose a reason for hiding this comment

Uh oh!

SiobhyB commented Jun 24, 2021

Uh oh!

guarani left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SiobhyB commented May 3, 2021 •

edited

Loading

peril-wordpress-mobile bot commented May 3, 2021 •

edited

Loading

peril-wordpress-mobile bot commented May 3, 2021 •

edited

Loading

SiobhyB commented Jun 3, 2021 •

edited

Loading

guarani commented Jun 3, 2021 •

edited

Loading