[RNMobile] Add "Set as Featured Image" Button to Image Block (Android Only)#31705
Merged
[RNMobile] Add "Set as Featured Image" Button to Image Block (Android Only)#31705
Conversation
|
Size Change: 0 B Total Size: 1.03 MB ℹ️ View Unchanged
|
…red| button The function returns a button (in the form of a BottomSheet.Cell component) based on whether "isFeaturedImage" is true or false. The function called within the image block's settings. In its current form, the button is unresponsive and only includes a label to "Set as Featured Image" or "Remove as Featured Image", depending on whether the current image is already featured or not.
For now, the button will only work on Android devices, so an "androidOnly" conditional is necessary to avoid bugs/complications on iOS.
The getStylesFromColorScheme hook is used to define CSS selectors for the button, depending on whether the device uses dark or light mode. The hook's "featuredButtonStyle" variable is then called via BottomSheet's labelStyle attribute.
This commit uses the "getStylesFromColorScheme" hook to set up CSS selectors for the "Set as Featured Image" button, as its style will vary slightly from the base button.
This includes CSS for both the light mode and dark mode versions of the button.
This function will fire when the "Set/Remove as Featured Image" button is pressed and its functionality will be fleshed out in future commits.
setFeaturedImage is imported from '@wordpress/react-native-bridge' and called via the onSetFeatured, as it'll be utilised when the button is pressed.
…edImage() When a featured image is being removed, the mediaId will be zero, and when it's being set it'll be the "attributes.id" of the image in the block.
6 tasks
Contributor
👋 @SiobhyB , happy to review! Can you share some info about what's not "finished" yet keeping the PR in Draft mode? Anything in particular you'd like me to offer feedback on? Thanks! |
Contributor
Author
|
@hypest, thanks for taking a look! The I'm keeping the PR as a draft as there will likely be some commits coming as part of the efforts to resolve the failing tests, but I believe that these will mainly be merges and updates to references. I don't currently have any other major changes to the code in mind, so seeking feedback on the overall PRs, aside from the failing tests on |
Contributor
Author
|
@hypest, just an FYI that I'm taking this out of draft now as all tests have now passed. :D 🎉 |
hypest
reviewed
May 14, 2021
Contributor
Author
|
@hypest, just an FYI that the bug I mentioned in the above comment has been fixed now. :) |
With this commit, a MEDIA_ID_NO_FEATURED_IMAGE_SET constant is added to make the reason we pass a zero when a featured image is removed clearer, and also make it easier to change in the future if needed.
hypest
reviewed
May 26, 2021
…functions These functions had accidentally been given the oppositive names (that is, the function that removes a featured image was named "setFeaturedButton" and vice versa). This commit corrects that mistake.
Contributor
Author
|
👋 @ajitbohra, it seems like you were added as a reviewer to this PR following a change to the Most of the changes in the PR itself are on the native side and are already under review by two developers from the mobile team. Would you still need to review the changes in this instance or is it okay to proceed with the current reviewers? I'm not that familiar with the way reviews are assigned in the project, so any insight you can offer around the correct processes would be appreciated. Thanks! |
Contributor
|
Tested all the flows via WPAndroid and works perfect! 🎉 ✅ Test Case 1: General Flow |
Contributor
|
✅ Also verified via WPiOS that the feature is not available there as expected (Android only for the time being, iOS is worked on a different set of PRs). |
hypest
approved these changes
May 27, 2021
Contributor
hypest
left a comment
There was a problem hiding this comment.
Looks good to me! Awesome work here @SiobhyB , thanks for persevering and addressing all feedback!
Seems like there are some web E2E test job failing but don't look relevant to the changes here. Perhaps you can update from trunk and see if that fixes them (trunk seems green at the time of writing this).
I'll leave it to the other reviewers of this PR to conclude their rounds and I guess we can start merging the domino! 👏
This was referenced May 30, 2021
jd-alexander
approved these changes
Jun 1, 2021
Contributor
jd-alexander
left a comment
There was a problem hiding this comment.
Hi @SiobhyB I ran tests after merging in the refactor and all flows work as expected! 🎉
WP Android
✅ Test Case 1: General Flow
✅ Test Case 2: "Replace Existing" Flow
✅ Test Case 3: "Editor Loads" Flow
✅ Test Case 4: "Post Settings" Flow
✅ Test Case 5: "Replace in Image Block" Flow
WP iOS
✅ Verified that the functionality does not work on iOS and that setting the featured image in Post Settings does not cause the editor to crash.
bc00de7 to
4e24d0a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Partial fix for: wordpress-mobile/gutenberg-mobile#1011
gutenberg-mobile: wordpress-mobile/gutenberg-mobile#3116WordPress-Android: wordpress-mobile/WordPress-Android#14503Description
Building on the work done in #30806, this PR will add a Set as Featured button to the image block's settings, 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 settings.
Note: This PR introduces the Set as Featured button to Android devices only, support for iOS will follow in future iterations, too.
How has this been tested?
Test Case 1: General Flow
To start with, the general flow for setting a featured image via the image block on a post without an existing image is as follows.
Test Case 2: "Replace Existing" Flow
It's also possible for users to replace an existing featured image via the following flow.
Test Case 3: "Editor Loads" Flow
Next, we should verify that a **Remove as Featured** option appears when a post with a featured image is first loaded. To test this, follow these steps.
Test Case 4: "Post Settings" Flow
The image block should be updated to reflect whether its image is featured or not even when the featured image is set via the general Post Settings flow.
Test Case 5: "Replace in Image Block" Flow
Finally, it's possible to replace an image directly within the image block's settings. We need to verify that the featured settings update as expected depending on the image that's chosen for an image block.
Screenshots
Types of changes
This PR introduces a new feature (non-breaking change which adds functionality). The feature it introduces is a Set as Featured button in the image block's setting, which enables a user to either set an image as featured or remove it as featured.
A high-level overview of the code changes involved is as follows:
getSetFeaturedButton. This component outputs a button with either aSet as Featuredor aRemove as Featuredlabel, depending on whetherisFeaturedImageis true or not.onSetFeaturedwill be called when the button is tapped, this function utilizes another function namedsetFeaturedImageto send an image's media ID over the bridge when a request is made to either set a new featured image (in which case the media ID is that of the selected image) or to remove an existing featured image (in which case the media ID is zero). A call to close the settings screen is also made withinonSetFeatured.featuredImageIdis updated after confirmation of the change on the native side, as native is our main source of truth when it comes to confirming the change to the post's featured image ID. The changes topackages/edit-post/src/editor.native.jsin the linked PR can be referenced for further details around how this is handled using thegetEditedPostAttribute()function.Checklist: