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

Label sometimes displays hugely wrong font size and spacing #6801

Closed
philipag opened this issue May 4, 2022 · 5 comments · Fixed by #7219
Closed

Label sometimes displays hugely wrong font size and spacing #6801

philipag opened this issue May 4, 2022 · 5 comments · Fixed by #7219
Assignees
Labels
area/controls 🎮 Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor control-label Label, Span fixed-in-6.0.400 Look for this fix in 6.0.400! p/1 Work that is critical for the release, but we could probably ship without platform/android 🤖 t/bug Something isn't working
Milestone

Comments

@philipag
Copy link

philipag commented May 4, 2022

Description

As shown in the attached repro sample, "theLabel.FormattedText" is set twice; once after init and then again after 2 seconds.

Even though the Label font size settings is not changed and even though the same FormattedText is used, after 2 seconds the label text changes significantly when it should not. This is a regression (XF did not have this issue) and manifests in many different ways. The repro just shows one such example.

layoutbug8.zip

Steps to Reproduce

  1. Compile and run the sample under Android 11
  2. notice the label layout/size change completely after 2 seconds - even though nothing should change.

Version with bug

Release Candidate 2 (current)

Last version that worked well

Unknown/Other

Affected platforms

Android, I was not able test on other platforms

Affected platform versions

Android 11

Did you find any workaround?

no, and similar strange things happen using different layouts.

Relevant log output

No response

@philipag philipag added s/needs-verification Indicates that this issue needs initial verification before further triage will happen t/bug Something isn't working labels May 4, 2022
@jfversluis jfversluis added platform/android 🤖 area/controls 🎮 Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor and removed s/needs-verification Indicates that this issue needs initial verification before further triage will happen labels May 4, 2022
@kristinx0211 kristinx0211 added the s/verified Verified / Reproducible Issue ready for Engineering Triage label May 5, 2022
@kristinx0211
Copy link

verified repro on android 11 with above project.

@Redth Redth added this to the 6.0.300-servicing milestone May 6, 2022
@Redth Redth added the p/1 Work that is critical for the release, but we could probably ship without label May 6, 2022
@mattleibow
Copy link
Member

I fixed a bug like this a while back, could we check again with RC 3?

@mattleibow mattleibow added s/needs-verification Indicates that this issue needs initial verification before further triage will happen and removed s/verified Verified / Reproducible Issue ready for Engineering Triage labels May 12, 2022
@jfversluis jfversluis removed the s/needs-verification Indicates that this issue needs initial verification before further triage will happen label May 12, 2022
@jfversluis
Copy link
Member

@kristinx0211 could you check again please?

@kristinx0211
Copy link

@jfversluis sure, I can repro it on android 11 and 12 with vs 17.3.0 Preview 2.0 [32511.531.main].

@mattleibow
Copy link
Member

OK, so I am looking at this and I see the issue. The initial text is wrong as it is small font, but the FontSize was 35. When the text changes, then it gets the correct size.

@mattleibow mattleibow self-assigned this May 16, 2022
mattleibow added a commit that referenced this issue May 16, 2022
This initial value of Nan (or could be 0) indicates to the layout engine that the size must come from the Label. If we set it here, then there is no way of knowing that the size was not actually set so uses the size. If the size is Nan, it falls back to the "default font size" which is not really default but actually the font size of the label.
@mattleibow mattleibow added the control-label Label, Span label May 16, 2022
mattleibow added a commit that referenced this issue May 17, 2022
* Reduce line length so we can work

* Set initial Span.FontSize => double.NaN Fixes #6801

This initial value of Nan (or could be 0) indicates to the layout engine that the size must come from the Label. If we set it here, then there is no way of knowing that the size was not actually set so uses the size. If the size is Nan, it falls back to the "default font size" which is not really default but actually the font size of the label.

* Correctly apply span values to spans Fixes #7220

All:
- set the default parameter values for line height to be -1 as that is what is the default currently
Android:
- don't capture the TextView.Paint as that is always wrong initially since none of the other properties are set
- don't fall back to the Label.LineHeight to the spans as that is always applied - regardless of span values
- pass the Label.CharacterSpacing down to be consistent
- pass the Label.TextDecorations down as well
- for text decorations, use the platform spans
- split a LetterSpacingSpan out of the FontSpan so that they can be individually applied

* Update src/Controls/src/Core/Platform/Android/Extensions/FormattedStringExtensions.cs

Co-authored-by: campersau <buchholz.bastian@googlemail.com>
@dotnet dotnet locked as resolved and limited conversation to collaborators Jun 16, 2022
@samhouts samhouts added the fixed-in-6.0.400 Look for this fix in 6.0.400! label Feb 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/controls 🎮 Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor control-label Label, Span fixed-in-6.0.400 Look for this fix in 6.0.400! p/1 Work that is critical for the release, but we could probably ship without platform/android 🤖 t/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants