[SuperEditor] Fix list item vertical alignment (Resolves #1834) #1920
Conversation
|
@angelosilvestre is there a reason why we'd want different indentation based on font size? That sounds like a bug to me. |
| shape: BoxShape.circle, | ||
| color: component.styleBuilder({}).color, | ||
| child: Text.rich( | ||
| // Place a zero-width joiner before the bullet point to make it properly aligned. Without this, |
There was a problem hiding this comment.
Can you point out what specifically happens if we don't include this zero-width character? Also, should we move this comment immediately above the text: line? Because right now this comment appears to apply to the whole span, which includes the dot.
There was a problem hiding this comment.
Without the zero-width character the dot appears misaligned for small font sizes:
Without the zero-width character:
With the zero-width character:
@matthew-carroll This seems to be done on purpose. This is the method that computes then indentation: double _defaultIndentCalculator(TextStyle textStyle, int indent) {
return (textStyle.fontSize! * 0.60) * 4 * (indent 1);
} |
|
@angelosilvestre it seems likely that that behavior was designed while thinking about a list where all list items are the same size. If all list items have really big text, then it indents more, if all list items have tiny text, it indents less. I doubt this work grappled with varying text sizes. It's obvious from your screenshots that the result of displaying list items at varying text sizes looks horrible and is unexpected. So we need to fix that. Seems like we can either make it constant, or we'll need to search up and down the list to figure out the largest font size and base all list item indentations on that font size. |
|
@matthew-carroll Based on the google docs example that I posted before I would say that we should be fine using a constant indentation.
|
|
Ok, then let's do that. |
|
@matthew-carroll For ordered lists I'm not sure how we should compute the indentation. Here's an example from Google Docs:
We can see that the numeral is left-aligned, but the distance between the numeral and the content isn't fixed. |
|
Hm. That's pretty weird. Can you post a few screenshots from a few other apps so we can compare? |
Apple Notes
The numerals look right-aligned, but it's also weird with bigger font sizes. Also, with two digits numerals it looks even more weird:
Microsoft Word online
Numerals are left-aligned and the indentation increases on bigger font sizes Evernote
Doesn't scale the numeral. Canva Docs
Numerals are left-aligned and the indentation increases on bigger font sizes Other appsNotion, Obsidian and ClickUp don't look like they support setting font sizes. |
|
Looking at those examples, I think right aligning the numerals looks best. The problem with the Apple version is that they allow different widths for the numerals. What if we make all the numerals in the same list take up the same width? To accomplish that, we could have each list item take its font, font size, font style, and then immediately measure every numeral in the font: 0-9. That list item takes the widest value and multiplies it by the number of numerals used by that list item. Then all list items respect the widest value. We can cache the measurement so that the same font, at the same size, with the same style never needs to re-measure. That would work fine most lists. What do you think? |
|
@matthew-carroll Can we a separate PR's to avoid the fix for the vertical alignment with the horizontal alignment change? Or do you prefer to keep all on the same PR? |
|
@angelosilvestre we can do them separately. Please file the appropriate issue and let me know when you think this PR has what it should. |
|
@matthew-carroll I filed #1950 Could you please re-review this PR? |
| color: component.styleBuilder({}).color, | ||
| child: Text.rich( | ||
| TextSpan( | ||
| // Place a zero-width joiner before the bullet point to make it properly aligned. Without this, |
There was a problem hiding this comment.
I won't hold up on the PR on this, but can you file an issue with Flutter including repro steps to show mis-alignment in the absence of the spacer? No need to include attributed text in that - just use direct Flutter stuff that demonstrates both options.
|
@angelosilvestre it looks like the cherry pick failed here. Can you do it manually? |
[SuperEditor] Fix list item vertical alignment. Resolves #1834
The dot in unordered list items is a bit misaligned when using a line height multiplier greater than
1.0. Here's one example with a3.0multiplier:The misalignment is worse when using different font sizes (this also affects ordered list items)
This PR changes the list component to place the dot inside a
WidgetSpanto properly align it. Also, the list components were changed use the attributions of the first character. This change was made because the list item might contain aFontSizeAttribution, which will affect the line height.This was the result:
The alignment change caused some golden tests to fail, because now the dot is slightly above from where it was before.
We are still rendering differently from Google Docs. It scales the dot and doesn't seem to indent based on the font size:
Avoiding scaling the dot was intentional, due to previous conversations. We could change that if needed.