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] Fix loss of center alignment in image captions on Android #16722

Merged
merged 1 commit into from
Jul 24, 2019

Conversation

mchowning
Copy link
Contributor

@mchowning mchowning commented Jul 23, 2019

This PR is related to WordPress/gutenberg-mobile#1243.

Description

Addresses an issue where an unwanted <div> tag was added to image captions because the defaultProps for RichText specified a div tagName. This was causing the loss of center alignment on image captions in mobile.

Background

In order to implement quote blocks, RichText was given a default "div" tagName. This created a problem on Android because AztecEditor-Android creates a HiddenHtmlSpan out of div tags, and because HiddenHtmlSpans extend IAztecParagraphStyle they default to "normal" text alignment. This overrides the center gravity that is set on the native EditText as a result of the text-align prop being set from the image block.

This problem exhibited itself in the loss of center alignment anytime RichText sent html with div tags to native for rendering on a view with center alignment (i.e., the image caption). Explicitly setting the image block's tagName to be an empty string prevents the default "div" tagName from being applied. It would be nice to address this on the native layer (so that div tags didn't override the view's gravity), but any fix at that level would have been very invasive.

Test steps

From gutenberg-mobile Issue 1070

  1. Select an image block from the demo content in the demo app
  2. Add some text in the caption. Notice that the text is centered as expected.
  3. Select all caption text and tap on the "Bold" toolbar button
  4. Verify that the image caption remains horizontally centered.
Before After
Screen Shot 2019-07-23 at 1 53 39 PM Screen Shot 2019-07-23 at 1 45 25 PM

From gutenberg-mobile Issue 1113

  1. Add this snipped to gutenberg-mobile sample app:
<!-- wp:image {"align":"center"} -->
<div class="wp-block-image"><figure class="aligncenter"><img src="https://cldup.com/cXyG__fTLN.jpg" alt="Beautiful landscape"/><figcaption>Give it a try.<br> Press the "wide" button on the image toolbar.</figcaption></figure></div>
<!-- /wp:image -->
  1. Run the example app.
  2. Verify that the image caption is horizontally centered.
Before After
Screen Shot 2019-07-23 at 1 52 29 PM Screen Shot 2019-07-23 at 1 55 23 PM

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.

Using a `div` tag on Android causes the text's alignment to be reset to
normal alignment unless the div has a text-align style attached to it.
This is a problem on image captions which need to be center aligned.
@mchowning mchowning 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 Jul 23, 2019
@mchowning mchowning self-assigned this Jul 23, 2019
@mchowning mchowning requested review from etoledom and marecar3 July 23, 2019 18:12
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.

Works perfectly! Thank you @mchowning 🎉

Tested on both example and WPAndroid apps

@mchowning mchowning merged commit 70a1149 into master Jul 24, 2019
@mchowning mchowning deleted the rnmobile/image-block-ignore-default-tagname branch July 24, 2019 12:59
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.

2 participants