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

Child Span does not inherit TextColor or FontAttributes from parent Label #21034

Open
BlueRaja opened this issue Mar 6, 2024 · 10 comments · May be fixed by #21191
Open

Child Span does not inherit TextColor or FontAttributes from parent Label #21034

BlueRaja opened this issue Mar 6, 2024 · 10 comments · May be fixed by #21191
Assignees
Labels
area-controls-label Label, Span migration-compatibility Xamarin.Forms to .NET MAUI Migration, Upgrade Assistant, Try-Convert platform/android 🤖 s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Milestone

Comments

@BlueRaja
Copy link

BlueRaja commented Mar 6, 2024

Description

In Xamarin, child <Span> would inherit font properties from the parent <Label>. In Maui, the child inconsistently inherits TextDecorations, but not FontColor or FontAttributes (not sure about other attributes).

Steps to Reproduce

<Label TextColor="Blue" TextDecorations="Underline" FontAttributes="Bold" BackgroundColor="Gray">
  <Label.FormattedText>
    <FormattedString>
      <Span Text="Blue" />
      <Span Text="Green"
            TextColor="Green"
            TextDecorations="None"
            FontAttributes="None"/>
    </FormattedString>
  </Label.FormattedText>
</Label>

Expected output:
Text that is blue, bold, and underlined; then text that is green and none of those things

Actual output:
2024-03-06 00_42_04-Android Emulator - pixel_5_-_api_33_5554

Link to public reproduction project repository

No response

Version with bug

8.0.3 GA

Is this a regression from previous behavior?

Yes, this used to work in Xamarin.Forms

Last version that worked well

Unknown/Other

Affected platforms

Android, I was not able test on other platforms

Affected platform versions

No response

Did you find any workaround?

Copy+paste all label attributes to all of its child spans

Relevant log output

No response

@BlueRaja BlueRaja added the t/bug Something isn't working label Mar 6, 2024
@samhouts samhouts added the migration-compatibility Xamarin.Forms to .NET MAUI Migration, Upgrade Assistant, Try-Convert label Mar 11, 2024
@pjcollins
Copy link
Member

For projects that were created from the net8.0 project template, there is a new <Style TargetType="Span"> style that sets a default TextColor for all Span elements. If you remove this entry from Resources/Styles/Styles.xaml then Span.TextColor should inherit from the parent Label.TextColor as you are expecting on both Android and iOS.

There are some behavior differences with TextDecorations on iOS that I am looking at, it looks like we have at least one case where we could improve style inheritance consistency with Android there. I can also see a case where we may want to inherit font attributes if none are set on both platforms and will see about getting a PR up with these changes.

pjcollins added a commit that referenced this issue Mar 13, 2024
Fixes: #21034

Span elements are inconsistently inheriting attributes from their parent
Label on Android and iOS.  Parent TextDecorations would be inherited on
Android if unset, but not on iOS.  Additionally, neither platform would
inherit FontAttributes if unset.  Spans on Android and iOS have been
updated to use the parent values for TextDecorations and FontAttributes
if these values are not set in the Span itself.
@jaosnz-rep
Copy link
Collaborator

Can repro on 17.10 Preview 2.0 with .net8.0 Windows/Android platform, and the solution provided by pjcollins can be solved.

@jaosnz-rep jaosnz-rep added s/verified Verified / Reproducible Issue ready for Engineering Triage s/triaged Issue has been reviewed labels Mar 15, 2024
@hansmbakker
Copy link

hansmbakker commented Mar 15, 2024

@pjcollins that looks good - is there a low-key way we can try this solution from your PR e.g. using a handler or would we need to build MAUI ourselves for that?

@pjcollins
Copy link
Member

I still need to get a test added to the PR, but if you wanted to quickly test the PR build on a machine with a recent .NET 8 MAUI install I think you could do the following:

  1. Download and unzip the nuget artifact from https://dev.azure.com/xamarin/public/_build/results?buildId=111019&view=artifacts&pathAsName=false&type=publishedArtifacts
  2. Add a nuget.config file to your test project with contents along the lines of:
    <?xml version="1.0" encoding="utf-8"?>
    <configuration>
      <packageSources>
        <add key="local" value="/Users/peter/Downloads/nuget" />
        <add key="nuget" value="https://api.nuget.org/v3/index.json" />
      </packageSources>
    </configuration>
  3. Build a .NET 8 project overriding MauiVersion to point to the PR build packages (and run on a connected Android device) with:
    dotnet build -v:n mauprtest.csproj -p:MauiVersion=8.0.20-ci.net8.27026 -t:Run -f net8.0-android

@hansmbakker
Copy link

hansmbakker commented Mar 15, 2024

@pjcollins I tried it, but it does not seem to fix the FontFamily inheritance issue I had

Issue

In this image you see several attempts where the FontFamily setter value of the Heading style class is not properly inherited in the spans. The font family should be PoppinsSemiBold - you can see that the cases using spans are not bold:
image

Test cases (corresponding to the labels in the Code section)

  1. converts a string containing sections marked with * to a FormattedString with Spans - the ConverterParameter sets the TextColor of marked Spans
  2. manual creation of FormattedString containing multiple Spans
  3. string is converted to FormattedString containing one Span using implicit operator of FormattedString
  4. only a Label with Text (not FormattedText) gets the correct FontFamily

Actual behavior

Only the Label not using Spans will have the the correct FontFamily

Expected behavior

Spans not having FontFamily set, inherit the FontFamily from the parent Label.

Code

Typography.xaml

<Style Class="Heading" TargetType="Label">
    <Setter Property="FontFamily" Value="PoppinsSemiBold" />
</Style>
<!-- .... -->
<Style Class="Heading04" TargetType="Label">
    <Setter Property="FontSize" Value="{x:Static local:Typography.Heading04MobileFontsize}" />
    <Setter Property="LineHeight" Value="{x:Static local:Typography.Heading04MobileLineheight}" />
    <Setter Property="CharacterSpacing" Value="{x:Static local:Typography.Heading04MobileLetterspacing}" />
    <Setter Property="SemanticProperties.HeadingLevel" Value="Level4" />
</Style>

Page.xaml

<VerticalStackLayout Spacing="{x:Static styles:Spacing.Xsm}">
    <Label FormattedText="{Binding Source={x:Static strings:Strings.HeadingPermanentWorkers}, Converter={StaticResource markStringWithColorSpanConverter}, ConverterParameter={x:Static styles:ColorTokens.StaticTextOnBgBrandWeak}}"
           StyleClass="Heading, Heading04" />

    <Label StyleClass="Heading, Heading04">
        <Label.FormattedText>
            <FormattedString>
                <Span TextColor="{x:Static styles:ColorTokens.StaticTextOnBgBrandWeak}">Permanent</Span>
                <Span> workers</Span>
            </FormattedString>
        </Label.FormattedText>
    </Label>

    <Label FormattedText="{x:Static strings:Strings.HeadingPermanentWorkers}" StyleClass="Heading, Heading04" />

    <Label Text="{x:Static strings:Strings.HeadingPermanentWorkers}"
           StyleClass="Heading, Heading04" />
</VerticalStackLayout>

@pjcollins
Copy link
Member

@hansmbakker I see, this scenario was not outlined in the original issue description. I'll try to take a look at it in the context of this fix, but we may want to file a separate issue to track it eventually.

@hansmbakker
Copy link

Ah, clear. In general I would see Spans as children of a Label, so as a user I would expect all properties mentioned in Use formatted text to be inherited.

FontFamily is just one of them, but I would say it would make sense to also check the other ones.

@pjcollins pjcollins linked a pull request Mar 19, 2024 that will close this issue
@pjcollins
Copy link
Member

@hansmbakker by my understanding the issue you've described above isn't necessary specific to FontFamily but could affect any property declared in a Style. Unfortunately, Style inheritance is a bit more complicated and as I understand is only supported for types that derive from the TargetType of the style. As Span is not a Label, Spans belonging to a Label will not have the same style applied to them. I think it may be worth filing a separate issue for this where we can continue discussion/investigation.

@hansmbakker
Copy link

hansmbakker commented Mar 19, 2024

@pjcollins I stumbled upon this issue because for our app it was the FontFamily property we hoped to have inheritance for.

However, in my opinion it would make sense for a Span to be consistent with the text look&feel of the parent Label by default, unless specified explicitly.

any property declared in a Style

No, I meant only the (text-related) properties in the documentation section Use formatted text:

  • BackgroundColor, of type Color, which represents the color of the span background.
  • CharacterSpacing, of type double, sets the spacing between characters in the displayed text.
  • FontAttributes, of type FontAttributes, determines text style.
  • FontAutoScalingEnabled, of type bool, defines whether the text will reflect scaling preferences set in the operating system. The default value of this property is true.
  • FontFamily, of type string, defines the font family.
  • FontSize, of type double, defines the font size.
  • LineHeight, of type double, specifies the multiplier to apply to the default line height when displaying text.
  • TextColor, of type Color, defines the color of the displayed text.
  • TextDecorations, of type TextDecorations, specifies the text decorations (underline and strikethrough) that can be applied.
  • TextTransform, of type TextTransform, specifies the casing of the displayed text.

Please note that I did not include the properties Style and Text in this list - they do not make sense to inherit.

Other properties from Label like WidthRequest would also not make sense to inherit.

@pjcollins
Copy link
Member

With the latest changes in #21191, if FontFamily is set on a Label element directly, it should be inherited by any Spans that belong to that label. However, if FontFamily is declared in a Style that is applied to a Label, it will not be applied to child Span elements. The latter is the scenario that would potentially require additional follow up in a new/separate issue.

@PureWeen PureWeen added this to the .NET 8 SR4 milestone Mar 22, 2024
@Eilon Eilon added the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label Mar 29, 2024
@PureWeen PureWeen modified the milestones: .NET 8 SR4, .NET 8 SR5 Apr 9, 2024
@PureWeen PureWeen modified the milestones: .NET 8 SR5, .NET 8 SR6 Apr 25, 2024
@Eilon Eilon removed the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label May 10, 2024
@PureWeen PureWeen modified the milestones: .NET 8 SR6, .NET 8 SR7 Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-controls-label Label, Span migration-compatibility Xamarin.Forms to .NET MAUI Migration, Upgrade Assistant, Try-Convert platform/android 🤖 s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Projects
Status: Possible
Development

Successfully merging a pull request may close this issue.

8 participants