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] Enable edit button over Image blocks for Android #21165

Conversation

ashiagr
Copy link
Contributor

@ashiagr ashiagr commented Mar 26, 2020

Description

This PR removes showMediaEditorButton const references to enable edit button over Image blocks for Android as well.

The button was originally added as part of these PRs #19723, wordpress-mobile/gutenberg-mobile#1782 and was enabled only for iOS till now.

gutenberg-mobile related PR: wordpress-mobile/gutenberg-mobile#2062
WPAndroid Issue: wordpress-mobile/WordPress-Android#11147

(I don't have permissions on the main gutenberg repo, creating it in a fork-branch)

How has this been tested?

Creating it as a draft PR as it still needs to be tested.

Screenshots

To be included once tested.

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@@ -607,8 +606,7 @@ export class ImageEdit extends React.Component {
! isUploadInProgress &&
! isUploadFailed &&
finalWidth &&
finalHeight &&
showMediaEditorButton && (
finalHeight && (
Copy link
Contributor

@mchowning mchowning Mar 27, 2020

Choose a reason for hiding this comment

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

You're deleting a ( here, but I don't see that you're deleting the matching )?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so 🤔. Bracket was added to the previous line.

gutenberg_code

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right. Don't know what I was thinking. 🤦‍♂

Copy link
Contributor

@mchowning mchowning left a comment

Choose a reason for hiding this comment

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

These changes are working well! 👍

I've confirmed that the text alignment issue in the bottom sheet that you observed is not due to any changes in this PR, so I think we can merge this.

@etoledom etoledom added the Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) label Mar 30, 2020
@etoledom etoledom marked this pull request as ready for review March 30, 2020 08:01
Copy link
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

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

Agreed with @mchowning !
Tested on Android and WPAndroid via wordpress-mobile/WordPress-Android#11557

Working like a charm! 🎉

Thank you so much for this PR @ashiagr !

@etoledom etoledom merged commit 0b3fa53 into WordPress:master Mar 30, 2020
@github-actions github-actions bot added this to the Gutenberg 7.9 milestone Mar 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants