Fix bullet point's vertical alignment of list items #1315
Conversation
Thanks for taking a look @SergioEstevao! It's good to have an Aztec expert take a look 😄 I'll try this out tomorrow, as I didn't get the chance to look today while I was wrapping up Xposts in Gutenberg. |
|
Ok, I've marked the PR as |
There was a problem hiding this comment.
I tested in Gutenberg and it looks good. Like you mention here, the line still jumps, but now the bullet point also moves down to stay vertically centered, which I think is the most important thing. I tested nested lists and both ordered and unordered lists, all works well from my perspective.
On an iPhone 11 running the Gutenberg demo app with this branch of Aztec, I saw there's a slight delay between the line jumping down and the bullet point jumping down. It's only a few hundred milliseconds I'd say, but still noticeable. I'd say it's not worth looking into now, but thought I'd mention it.
|
@fluiddot can this be merged and a new release made on https://github.com/wordpress-mobile/AztecEditor-iOS/releases? I think when that's ready, we can include this in the new Gutenberg Mobile release that is going to be cut today. |
|
I've done some tests and I found out that although the bullet point is vertically centered, in some cases it still looks like a bit displaced to the bottom. This PR improves the way the bullet point is rendered compared to the original bug but unfortunately it doesn't fix it 100%, I think we should review in the future how the bullet point position is calculated. ExamplesOrdered list:
Unordered list:
Text boxAs you can see in the following screenshots the bullet point is centered, the problem is that the text box has more space at the top compared to the bottom. Text without emojis:
Text with emojis:
|
After being discussed, since this is a visual glitch it's not so critical to fix it in the new release, we can wait for the next one. This will give me time to rethink the approach and apply some changes in the following days. |
|
Hello, is this fix still coming? |
I changed this PR to draft a long time ago (#1315 (comment)), my idea was to explore other approaches to solve the issues I spotted (#1315 (comment)). However, it's been a while since I stopped working in the PR and I'm not planning to resume it in the short term. I invite other contributors to take over it if there's interest in addressing the issue, thanks 🙇 ! |
Fixes #1314
Description
Align the bullet point vertically with the text line.
How
The marker's (bullet point) render rectangle calculation has been changed.
Origin rectangle
The line fragment rectangle was used as the origin for the calculation, after some checks I realised that unfortunately it's not providing a uniform size (first and last items are providing different heights). For this reason I swapped it to use the used rectangle version (
lineFragmentUsedRect) which it provides the same height for all items.Vertical alignment
The Y offset now is calculated for center it vertically which also required to set the proper height, calculated from the glyph size.
Screenshot
fix_emoji_bullet_point.mp4
How to test
The issue was spotted in the WordPress app which uses a different font family so the issue has to be tested there or modifying the default font for the content in Aztec's example project.
Expected result:
Bullet points are properly aligned with the text line.