WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.52k stars 4.21k forks source link

Mobile: Prompt to keep or replace Featured Image when replacing Image block media that is set as the Featured Image #34429

Closed dcalhoun closed 2 years ago

dcalhoun commented 3 years ago

What problem does this address?

This enhancement request builds on the work done to implement a "set as featured" button in the image block's settings in wordpress-mobile/gutenberg-mobile#1011.

After setting an Image block as the post's Featured Image, if one replaces the media for that Image block, there is a disconnect as to what occurs to the post's Featured Image. Currently, taking this action causes the original image to remain as the Featured Image, even though the media is no a part of the post content. While this outcome could be perceived as correct, it could also cause confusion if the user expects the Featured Image to update as well.

What is your proposed solution?

From team discussions, the first thought was that we should default to maintaining the “featured” status even after an image is replaced, but upon further digging we think our suggestion would be to give the option to do one of the following—either:

  1. Keep the current featured image (“detach” the image block’s connection with the featured status), or
  2. Replace the current featured image (apply the new image as the featured image)

In theory, that part of the flow could look like this:

featured-image-block-replace-media-flow

In this example, the top row is what you’d see in the editor UI, on canvas. The bottom row is the featured image setting “behind the scenes”.

Alert/Dialog messaging

We noticed that we already have an ActionSheet implemented that shows a similar message when a user has an existing featured image and tries to apply an image as featured from an Image block’s settings—we can probably re-use that Alert/Dialog in this part, but suggest we refine the messaging a little bit to something like the following:

dcalhoun commented 3 years ago

@iamthomasbishop I notice the proposed solution includes what appears to be "A" and "B" image thumbnails, showcasing the current and potential replacement Featured Images respectively.

Our current implementation relies upon the default—and fairly rudimentary—Android Dialogs and iOS Action Sheets to display this prompt. The former is not easily capable of displaying an image—at least from the React Native context. The latter is not capable of displaying images. In order to display images within the prompt, we would need to complete a fair amount of refactoring, including updating iOS to use an Alert rather than an Action Sheet.

Given that, I propose we focus this Issue on updating the flow to include Keep or Replace choices only. We could repurpose https://github.com/WordPress/gutenberg/issues/30410 to focus on adding image thumbnails to all Featured Image prompts. WDYT?

iamthomasbishop commented 3 years ago

Our current implementation relies upon the default—and fairly rudimentary—Android Dialogs and iOS Action Sheets to display this prompt. The former is not easily capable of displaying an image—at least from the React Native context.

Ah, that's right, totally forgot about this constraint. In that case, would it be possible to move to a BottomSheet in this context? As I understand it, that would be a lot more flexible and a natural extension of the ActionSheet.

In the immediate term, I'm fine with implementing the "keep/replace" messaging within a Dialog/Alert as you mentioned (without images, of course), with the preference of moving over to using BottomSheet so that we can take advantage of its flexibility.

What do you think, @dcalhoun ?

dcalhoun commented 3 years ago

In that case, would it be possible to move to a BottomSheet in this context? As I understand it, that would be a lot more flexible and a natural extension of the ActionSheet.

Definitely. The BottomSheet is completely custom UI, which would allow us to render images.

~One caveat is that we will have to dismiss the Image block BottomSheet in order to display the Featured Image replacement BottomSheet. The BottomSheet relies upon react-native-modal, which is incapable of displaying multiple modals simultaneously. We ran into this issue with the Cover block media settings as well. If you look closely at the following video, you will notice the first BottomSheet closes before the Media Picker opens.~

Cover block - Add image or video https://user-images.githubusercontent.com/438664/132035016-c6a398bc-8376-47bd-86c9-bd1942d328f8.MP4

~Because the "Set as Featured Image" button resides within a BottomSheet, we will have a similar limitation in this instance. We must close the Image block BottomSheet before opening the Featured Image BottomSheet. Do you feel this caveat is acceptable?~

Image block - Set as Featured Image ![image-block-featured-image-button](https://user-images.githubusercontent.com/438664/132035233-45fa0ce2-d6f9-430a-b4b2-8c0bfb7ea799.jpeg)

Edit: We should be able to rely upon navigation within the BottomSheet to display this prompt. I.e tapping the "Set as Featured Image" button will transition to reveal the "keep/replace" prompt within the same Bottom Sheet.

In the immediate term, I'm fine with implementing the "keep/replace" messaging within a Dialog/Alert as you mentioned (without images, of course), with the preference of moving over to using BottomSheet so that we can take advantage of its flexibility.

What do you think, @dcalhoun ?

That makes sense to me. I'll update this issue to focus on the "keep/replace" messaging. I will update https://github.com/WordPress/gutenberg/issues/30410 to focus on leveraging a BottomSheet so that we render images within all Featured Image prompts.

One clarifying point from your last quote—you mention "Dialog/Alert." Did you intend to say "Dialog/Action Sheet" (the UI we use currently) or were you hoping to migrate iOS from Action Sheet to Alert in this work?

Thanks @iamthomasbishop!

iamthomasbishop commented 3 years ago

Apologies for the delay @dcalhoun ! That all makes sense, let's give it a try and see how it looks.

One clarifying point from your last quote—you mention "Dialog/Alert." Did you intend to say "Dialog/Action Sheet" (the UI we use currently) or were you hoping to migrate iOS from Action Sheet to Alert in this work?

Yea, my bad. I had forgotten we were using ActionSheet for this on iOS; my brain went to the Alert, bc it's the iOS equivalent of Android's Dialog component 🤦‍♂️

dcalhoun commented 3 years ago

@iamthomasbishop thanks! I opened https://github.com/WordPress/gutenberg/pull/34666 and will await your design feedback before moving forward. Please review whenever you can. 🙇🏻