-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Vertical Text Alignment #10658
Comments
This seems to be font related. Perhaps the font metrics aren't being processed correctly in all cases? vid.mp4 |
@chylex looks like it can be solved by adding Can you double-check it on your end? |
VerticalTextAlignment to center is non ideal especially for TextBox. This is because is cannot work for multi-line text. FluentUI upstream hardcodes some internal margins to make the first line of text appear as center even though it is actually top aligned. I need to check the actual control templates to validate this though. If true, we are going to have to pull out a lightweight styling resource to adjust this based on font. Ideally, it could be calculated with measurements of the text controls though. |
@robloo if you know you have a single line TB, it's probably a valid "workaround" but in general, I think there is no easy solution for it. A dynamic padding can be a way, but it's not that easy probably. Many things to consider here... |
@timunie Yea, it's tricky for sure and there are case-by-case workarounds (incliding shipping a standard font). However a general solution is going to be needed for this. On my end it is even more severe in appearance using FluentAvalonia with default macOS fonts. This all needs to work out of the box in a cross-platform, generalized way. It should be possible now that we have full text measuring too. |
@robloo my concerns are:
|
@timunie I think the first step is narrowing down exactly which controls have the issue. I think ComboBox can be fixed with center vertical alimentment in a general way. TextBox and TextBlock need something more complex or at least different like a margin resource.
TextDensity would have to just redefine the lightweight styling resource that controls the left/top text padding in certain controls.
If a dev customizes a height they are then responsible to control alignment. So we ignore this case in text heavy controls as always. There are three solutions and each control may need a different one.
Keep in mind if this is really all driven just due to different fonts we inherit the issue from WinUI. They didn't need to solve it and just hard-coded padding and margins for the windows font. So something new is needed for sure. That said, I'm still not 100% convinced there isn't something else going on as in WPF this kind of stuff just seems to work. |
I thought of another solitution here too. Have a special converter that calculates a thickness to be used as left/top margin based on given font pproperties. The control itself could be passed as a parameter to get the attached font properties and then run the calculations. Not sure how ugly this would look in practice but it should be possible. |
@timunie These textboxes already have VerticalContentAlignment set to Center. None of the other values work either, they make the text even more off-center than it already is. |
@chylex The issue is the Padding property and TextControlThemePadding styling resource. Try overriding either of these and you should be able to center it better. The defaults are carryover from WinUI which assumes Segoe fonts. You'll have to measure Consalas font though to calculate the correct values... |
@robloo Will that not mess up centering of fallback fonts on systems that don't have Consolas? |
In that case a converter is proably needed. The converter should get TextBox as input and Padding as output. A Behavior could also work imo. |
This should be built-in directly and allow changes in the future if a better idea comes up. So a DefaultTextPaddingConverter is the way to go for now I think. This would require changing a handful of control templates and will effectively make the TextControlThemePadding useless. This also does cause a potential issue with compact density styles. Not sure how you would know to calculate smaller values for that case yet. |
Yes, it will break for any other fonts with significantly different metrics... |
Actually, the converter can special case the Fluent theme. It will search for a TextContropThemePadding in the control resources and then use that to detect compact is needed. If the entire control is passed to the converter parameter we have access to all this information including resources and attached font properties including family. Obviously the converter will be Fluent theme specific though. |
Related WinUI issues upstream: |
@Gillibald what's your feeling about this topic? Do we want such a converter inside Avalonia build in? |
Such a converter needs access to the underlying glyph layout. So this is something Avalonia needs to provide. Calculating some padding based on the bounding box of black pixels aka glyph bounds is possible. So you could get centered text all the time. When it comes to multi-line content this strategy gets more complicated. So maybe this only applies to the first line or the MaxLines count. |
Yeah I guess this should be only calculated for single line textbox for now. |
I have found a Workaround for TextBox using a public class TextCenteringBehavior : Behavior<TemplatedControl>
{
/// <summary>
/// Defines the <see cref="AdditionalPadding" /> property
/// </summary>
public static readonly DirectProperty<TextCenteringBehavior, Thickness> AdditionalPaddingProperty =
AvaloniaProperty.RegisterDirect<TextCenteringBehavior, Thickness>(
nameof(AdditionalPadding),
o => o.AdditionalPadding,
(o, v) => o.AdditionalPadding = v);
private Thickness additionalPadding;
/// <summary>
/// Gets or sets an additional padding to the calculated one
/// </summary>
public Thickness AdditionalPadding
{
get { return additionalPadding; }
set
{
if (SetAndRaise(AdditionalPaddingProperty, ref additionalPadding, value))
{
UpdatePadding();
}
}
}
protected override void OnAttached()
{
base.OnAttached();
if (AssociatedObject is not null)
{
// listen to property changes
AssociatedObject.PropertyChanged += AssociatedObjectOnPropertyChanged;
}
}
protected override void OnDetaching()
{
if (AssociatedObject is not null)
{
AssociatedObject.PropertyChanged -= AssociatedObjectOnPropertyChanged;
}
base.OnDetaching();
}
private void AssociatedObjectOnPropertyChanged(object? sender, AvaloniaPropertyChangedEventArgs e)
{
if (e.Property == TextBox.TextProperty
|| e.Property == ContentControl.ContentProperty
|| e.Property == TemplatedControl.FontFamilyProperty
|| e.Property == Visual.BoundsProperty
|| e.Property == TemplatedControl.FontSizeProperty)
{
UpdatePadding();
}
}
private void UpdatePadding()
{
if (AssociatedObject is null)
return;
var typeface = new Typeface(
AssociatedObject.FontFamily,
AssociatedObject.FontStyle,
AssociatedObject.FontWeight,
AssociatedObject.FontStretch);
var textLayout = new TextLayout(null, typeface, AssociatedObject.FontSize, null);
var verticalPadding = (AssociatedObject.Bounds.Height - textLayout.TextLines[0].Height) / 2;
AssociatedObject.Padding = new Thickness(0, verticalPadding) + additionalPadding;
}
} usage: <TextBox Text="{Binding Greeting}"
VerticalAlignment="Center"
Height="50"
FontSize="35"
FontFamily="Consolas">
<Interaction.Behaviors>
<behaviors:TextCenteringBehavior AdditionalPadding="10,0" />
</Interaction.Behaviors>
</TextBox> |
Using an internal text padding and top alignment is entirely for multi-line text. If we only had to worry about one line we would just center and remove the default paddings. What this means is the converter or behavior solution will work for both single and multi-line text. That said, this isn't going to work with text runs and multiple font families and sizes. So perhaps controls should be calculating everything internally in the long term. A TextPadding property could be calculated by each relevant control. |
So, I think controls will need to expose their internal text area bounds publicly as read-only properties. I think two pieces of information are needed (naming is a placeholder):
TextBounds would be everything -- all lines -- and allow XAML control templates usage of that information if it's needed. TextFirstLineBounds is what is needed to actually fix this issue. But it alone isn't enough, a converter is needed to calculate how to center that TextFirstLineBounds within the overall control bounds. Someday XAML is going to have to support basic arithmetic. Anyway, each text-based control will have to calculate those properties based on their internal text presentation. That could include multiple font sizes, font families and lines (different runs). This seems like the cleanest, general-purpose approach. A converter or behavior that can reach into the internal control template and calculate this information seems worse long-term. Also note:
Thoughts? This is entirely new territory. |
EDIT: Nevermind, I found it in NuGet package
In my attempt to use your workaround I failed by not having the |
Is there any update regarding a permanent solution (restoring the old behavior)? I want to know if it is worth waiting for a fix or if I have to start making padding adjustments across my entire application!? |
@chkr1011 maybe you want try the linked PR |
Can now be tested using nightly builds if anyone wants to do it. |
I tested it with the latest nightly build but it still looks the same. Interestingly enough it only affects the Font "Consolas" on Windows. Other monospace fonts work perfectly on Windows and Linux. I was not able to test "Consolas" on Linux. |
I get a similar issue with the Oxanium font. It seems to be calculating the bounds height as too large, so the text appears vertically offset towards the top |
Some good discussion here: It seems someone figured out exactly what is going on too: |
Describe the bug
I've seen this a few different places now with vertically aligned text. I'm just going to throw in some screen captures below.
What it looks like to me is text is vertically centered by calculating the height as (ascent - descent). However, this visually doesn't look quite right as the vast majority of text follows the baseline and doesn't go all the way to the descent. Therefore, I think vertical centering probably should be (ascent - baseline) for height.
Interestingly, TextBox looks correct to me. So this might unfortunately be a more general issue. TextBox is aware of how to calculate text vertical position using baseline instead of descent. However, other controls just use bounds and are not aware of more text-specific considerations... that opens up a can of worms if it's what is actually happening.
I have not noticed this before in WPF/UWP so I think they also calculate using the baseline in all controls; I could be wrong though and just wanted to point this out for those that know more than me in this area.
Expected behavior
Text should be vertically centered "visually" instead of "mathematically". Avalonia is currently accurate but doesn't quite look right to the eye.
Screenshots
DatePicker:
ColorPicker > Components:
12px above, 14/15px below
ComboBox
Desktop (please complete the following information):
Additional context
Add any other context about the problem here.
The text was updated successfully, but these errors were encountered: