Gutenberg-mobile: Remove extra inner padding on text blocks

[iamthomasbishop]

Since we last solved these issues looks like there might have been a regression in one of the last releases. There is some extra padding applied inside the body of text blocks (paragraph, heading, etc). It's slightly different between platforms:

iOS

There appears to be a slight bit of additional left/right padding. It's hard to tell if there's also extra padding on top/bottom edges of the blocks, so we should double check. Same thing applied to the title (only on iOS).

Android

It looks like extra top/bottom padding, but left/right looks aligned properly.

(Note: title block looks to have proper padding.)

Note

All blocks have inner padding of 12 top/bottom, 16 left/right, so the total vertical spacing between blocks is always 24.

[koke]

Removed the 1.5 milestone, let's only use milestones for PRs

[iamthomasbishop]

Sounds good, apologies!

[maxme]

On Android, I did a quick sanity check - loaded the a test post with the block editor in the Layout inspector. Then I took a screenshot with Material Grid and added the layout inspector over it. Here is the result of this:

From this screen shot:

• It looks like we have a margin on each block.
• And some padding on each text "area" (title / heading / paragraph).
• The block toolbar (up / down - trash buttons) height looks good.

I think that removing the padding on text area would fix this problem on Android. I'll investigate more to check where this is coming from (RichText component? Aztec? glue code?).

[koke]

There might be some related changes in #1379, or at least I was discussing block margins there yesterday

[maxme]

Note: I found a source of extra padding in the example app, I was able to remove it, but apparently there is a different issue in the wpandroid integration.

[maxme]

By setting adding aztecText.setPadding(0, 0, 0, 0); in the ReactAztecManager.createViewInstance() call, I'm able to remove the padding on RichText in wpandroid integration:

But for some reason when using bigger font in a RichText (see the heading block in this screen shot) there is some extra vertical white space which is not a padding. See here:

Note: I also checked the ReactAztecText wasn't wrapped in another padded view.

[maxme]

This extra vertical white space is probably coming from heading rendering code in Aztec. For reference here is the screenshot of the Aztec version of this post.

[maxme]

I confirm the heading issue must be fixed on the Aztec side. It's currently not parametrized, but it can be. I'll update Aztec, do a new release and then update the PR here.

(I removed the vertical space in the "after" screenshot)

[iamthomasbishop]

Interesting notes above. @maxme. Do you know what the line-heights are set to for headings? I wonder if that has something to do with the additional optical spacing. For example, here's what the measurements of a Heading 2 block look like (22px font-size 28px line-height:

For reference, here is the type scale I'm following (font-size/line-height):

• H1: 24/32 (~1.33)
• H2: 22/28 (~1.27)
• H3: 20/26 (1.3)
• H4: 18/22 (~1.22)
• H5: 16/20 (1.25)
• H6: 14/18 (~1.29)
• Paragraph: 16/24 (1.5)

If we need to, we can define a standard formula/ratio and stick to it. FWIW, I propsed a refinment to the base (paragraph) alignment, but the headings need to be a little bit tighter, so they could be set to something between to 1.2 or 1.4. IIRC, there is also a difference in how Android and iOS handle line-height equivalents, so I wonder if we might need to define this differently for each platform.

Inspecting core GB, it looks like some headings use a line-height of 1.8 and some use 1.4 – not sure if they're following a scale or not.

[maxme]

Do you know what the line-heights are set to for headings? I wonder if that has something to do with the additional optical spacing

The line-height implementation is defined in the Android TextView (we use it as the base class for Aztec). And we set these values here for all text in an Aztec instance.

We could change that for each block but I'm not sure that's what you want (line-height and block padding are different things. line-height is the parameter that drives the vertical space between 2 lines when a long heading line is wrapped).

I'll take screenshots with long lines after updating the headings padding in Aztec and check if that's what we want.

[hypest]

This ticket looks closed by @mchowning but I only see Android side work on it. Aren't there still some iOS side adjustments that are pending?

[iamthomasbishop]

@hypest Looking at the latest internal beta, we seem mostly good on iOS, except the title seems to suffer the same issue that I mentioned above.

Also realized that the Separator block line also isn't quite wide enough (screenshot), so I'll report that separately.

[etoledom]

I have created a ticket for the Post Title issue on iOS to keep this one closed.

[maxme]

Aren't there still some iOS side adjustments that are pending?

I'm pretty sure @SergioEstevao made some adjustments on the iOS side, I couldn't find the PR though.