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.

Sign up for GitHub

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

Jump to bottom

Correctly re-apply font formatting to HTML text #14701

Merged
merged 4 commits into from
Apr 27, 2023
Merged

Correctly re-apply font formatting to HTML text #14701

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dfd12f7
Correctly re-apply font formatting to HTML text
mattleibow Apr 21, 2023
22b11a0
not this
mattleibow Apr 21, 2023
956d0dc
Merge remote-tracking branch 'origin/main' into dev/fix-ios-html
mattleibow Apr 24, 2023
f4ca7f3
Merge remote-tracking branch 'origin/main' into dev/fix-ios-html
mattleibow Apr 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 2 additions & 13 deletions src/Controls/src/Core/HandlerImpl/Label/Label.Android.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,32 +33,21 @@ private protected override void OnHandlerChangedCore()
}
}

public static void MapTextType(LabelHandler handler, Label label) => MapText((ILabelHandler)handler, label);
public static void MapTextType(LabelHandler handler, Label label) => MapTextType((ILabelHandler)handler, label);
public static void MapText(LabelHandler handler, Label label) => MapText((ILabelHandler)handler, label);
public static void MapLineBreakMode(LabelHandler handler, Label label) => MapLineBreakMode((ILabelHandler)handler, label);


public static void MapTextType(ILabelHandler handler, Label label)
{
Platform.TextViewExtensions.UpdateText(handler.PlatformView, label);
handler.UpdateValue(nameof(ILabel.Text));
}

public static void MapText(ILabelHandler handler, Label label)
{
Platform.TextViewExtensions.UpdateText(handler.PlatformView, label);
}

// TODO: NET8 make this public
internal static void MapTextColor(ILabelHandler handler, Label label)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No longer needed as the new way of Platform.TextViewExtensions.UpdateText() correctly handles span colors. There is no difference with either HTML of formatted text.

{
handler.PlatformView?.UpdateTextColor(label);

if (label?.HasFormattedTextSpans ?? false)
return;

Platform.TextViewExtensions.UpdateText(handler.PlatformView, label);
}

public static void MapLineBreakMode(ILabelHandler handler, Label label)
{
handler.PlatformView?.UpdateLineBreakMode(label);
Expand Down
4 changes: 2 additions & 2 deletions src/Controls/src/Core/HandlerImpl/Label/Label.Tizen.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ namespace Microsoft.Maui.Controls
{
public partial class Label
{
public static void MapTextType(LabelHandler handler, Label label) => MapText((ILabelHandler)handler, label);
public static void MapTextType(LabelHandler handler, Label label) => MapTextType((ILabelHandler)handler, label);
public static void MapText(LabelHandler handler, Label label) => MapText((ILabelHandler)handler, label);

public static void MapTextType(ILabelHandler handler, Label label)
{
Platform.TextExtensions.UpdateText(handler.PlatformView, label);
handler.UpdateValue(nameof(ILabel.Text));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of calling the same method, we should be triggering the Text property so people can properly intercept it.

}

public static void MapText(ILabelHandler handler, Label label)
Expand Down
2 changes: 1 addition & 1 deletion src/Controls/src/Core/HandlerImpl/Label/Label.Windows.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public static void MapDetectReadingOrderFromContent(ILabelHandler handler, Label
Platform.TextBlockExtensions.UpdateDetectReadingOrderFromContent(handler.PlatformView, label);

public static void MapTextType(ILabelHandler handler, Label label) =>
Platform.TextBlockExtensions.UpdateText(handler.PlatformView, label);
handler.UpdateValue(nameof(ILabel.Text));

public static void MapText(ILabelHandler handler, Label label) =>
Platform.TextBlockExtensions.UpdateText(handler.PlatformView, label);
Expand Down
3 changes: 0 additions & 3 deletions src/Controls/src/Core/HandlerImpl/Label/Label.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ public partial class Label
#if WINDOWS
[PlatformConfiguration.WindowsSpecific.InputView.DetectReadingOrderFromContentProperty.PropertyName] = MapDetectReadingOrderFromContent,
#endif
#if ANDROID
[nameof(TextColor)] = MapTextColor,
#endif
#if IOS
[nameof(TextDecorations)] = MapTextDecorations,
[nameof(CharacterSpacing)] = MapCharacterSpacing,
Expand Down
66 changes: 32 additions & 34 deletions src/Controls/src/Core/HandlerImpl/Label/Label.iOS.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
#nullable disable
using System;
using Microsoft.Maui.Controls.Platform;
using Microsoft.Maui.Handlers;

namespace Microsoft.Maui.Controls
{
public partial class Label
{
public static void MapTextType(LabelHandler handler, Label label) => MapText((ILabelHandler)handler, label);
public static void MapTextType(LabelHandler handler, Label label) => MapTextType((ILabelHandler)handler, label);
public static void MapText(LabelHandler handler, Label label) => MapText((ILabelHandler)handler, label);
public static void MapCharacterSpacing(LabelHandler handler, Label label) => MapCharacterSpacing((ILabelHandler)handler, label);
public static void MapTextDecorations(LabelHandler handler, Label label) => MapTextDecorations((ILabelHandler)handler, label);
Expand All @@ -17,61 +17,48 @@ public partial class Label

public static void MapTextType(ILabelHandler handler, Label label)
{
Platform.LabelExtensions.UpdateText(handler.PlatformView, label);
handler.UpdateValue(nameof(ILabel.Text));
}

public static void MapText(ILabelHandler handler, Label label)
{
Platform.LabelExtensions.UpdateText(handler.PlatformView, label);

MapFormatting(handler, label);
Comment on lines +26 to +27
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the real iOS fix that re-applied formatting.

}

public static void MapTextDecorations(ILabelHandler handler, Label label)
{
if (label?.HasFormattedTextSpans ?? false)
return;

if (label?.TextType == TextType.Html)
{
if (!IsPlainText(label))
return;
}

LabelHandler.MapTextDecorations(handler, label);
}

public static void MapCharacterSpacing(ILabelHandler handler, Label label)
{
if (label?.HasFormattedTextSpans ?? false)
return;

if (label?.TextType == TextType.Html)
{
if (!IsPlainText(label))
return;
}

LabelHandler.MapCharacterSpacing(handler, label);
}

public static void MapLineHeight(ILabelHandler handler, Label label)
{
if (label?.HasFormattedTextSpans ?? false)
return;

if (label?.TextType == TextType.Html)
{
if (!IsPlainText(label))
return;
}

LabelHandler.MapLineHeight(handler, label);
}

public static void MapFont(ILabelHandler handler, Label label)
{
if (label?.HasFormattedTextSpans ?? false)
if (label.HasFormattedTextSpans)
return;

if (label?.TextType == TextType.Html && FontIsDefault(label))
if (label.TextType == TextType.Html && IsDefaultFont(label))
{
// If no explicit font has been specified and we're displaying HTML,
// If no explicit font has been specified and we're displaying HTML,
// let the HTML determine the font
return;
}
Expand All @@ -81,12 +68,12 @@ public static void MapFont(ILabelHandler handler, Label label)

public static void MapTextColor(ILabelHandler handler, Label label)
{
if (label?.HasFormattedTextSpans ?? false)
if (label.HasFormattedTextSpans)
return;

if (label?.TextType == TextType.Html && label.GetValue(TextColorProperty) == null)
if (label.TextType == TextType.Html && label.TextColor.IsDefault())
{
// If no explicit text color has been specified and we're displaying HTML,
// If no explicit text color has been specified and we're displaying HTML,
// let the HTML determine the colors
return;
}
Expand All @@ -104,22 +91,33 @@ public static void MapMaxLines(ILabelHandler handler, Label label)
handler.PlatformView?.UpdateMaxLines(label);
}

static bool FontIsDefault(Label label)
static void MapFormatting(ILabelHandler handler, Label label)
{
handler.UpdateValue(nameof(ILabel.TextColor));
handler.UpdateValue(nameof(ILabel.Font));
}
Comment on lines +94 to +98
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call the properties that need to be re-applied.


static bool IsDefaultFont(Label label)
{
if (label.IsSet(Label.FontAttributesProperty))
{
return false;
}

if (label.IsSet(Label.FontFamilyProperty))
{
return false;
}

if (label.IsSet(Label.FontSizeProperty))
{
return false;
}

return true;
}

static bool IsPlainText(Label label)
{
if (label.HasFormattedTextSpans)
return false;

if (label.TextType != TextType.Text)
return false;

return true;
}
Expand Down
44 changes: 44 additions & 0 deletions src/Controls/tests/DeviceTests/Elements/Label/LabelTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,50 @@ await InvokeOnMainThreadAsync(() =>
});
}

[Fact]
public async Task TextTypeAfterFontStuffIsCorrect()
{
// Note: this is specifically a Controls-level rule that's inherited from Forms
// There's no reason other SDKs need to force font properties when dealing
// with HTML text (since HTML can do that on its own)

var label = new Label
{
FontSize = 64,
FontFamily = "Baskerville",
Text = "<p>Test</p>"
};

await InvokeOnMainThreadAsync(() =>
{
var handler = CreateHandler<LabelHandler>(label);
label.TextType = TextType.Html;
AssertEquivalentFont(handler, label.ToFont());
});
}

[Fact]
public async Task FontStuffAfterTextTypeIsCorrect()
{
// Note: this is specifically a Controls-level rule that's inherited from Forms
// There's no reason other SDKs need to force font properties when dealing
// with HTML text (since HTML can do that on its own)

var label = new Label
{
TextType = TextType.Html,
Text = "<p>Test</p>"
};

await InvokeOnMainThreadAsync(() =>
{
var handler = CreateHandler<LabelHandler>(label);
label.FontFamily = "Baskerville";
label.FontSize = 64;
AssertEquivalentFont(handler, label.ToFont());
});
}

Color TextColor(LabelHandler handler)
{
#if __IOS__
Expand Down
8 changes: 4 additions & 4 deletions src/Core/src/Handlers/Label/LabelHandler.iOS.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,13 @@ public static void MapLineHeight(ILabelHandler handler, ILabel label)
public static void MapFormatting(ILabelHandler handler, ILabel label)
{
// Update all of the attributed text formatting properties
handler.PlatformView?.UpdateLineHeight(label);
handler.PlatformView?.UpdateTextDecorations(label);
handler.PlatformView?.UpdateCharacterSpacing(label);
handler.UpdateValue(nameof(ILabel.LineHeight));
handler.UpdateValue(nameof(ILabel.TextDecorations));
handler.UpdateValue(nameof(ILabel.CharacterSpacing));

// Setting any of those may have removed text alignment settings,
// so we need to make sure those are applied, too
handler.PlatformView?.UpdateHorizontalTextAlignment(label);
handler.UpdateValue(nameof(ILabel.HorizontalTextAlignment));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't just call the method, trigger the property so that the mappers can re-run.

}
}
}