Skip to content

[Windows] Fix LineHeight values <1 having no effect by setting LineStackingStrategy to BlockLineHeight #31289

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
Merged
rmarinho merged 10 commits into from
Sep 19, 2025
Merged

[Windows] Fix LineHeight values <1 having no effect by setting LineStackingStrategy to BlockLineHeight #31289

rmarinho merged 10 commits into from
Sep 19, 2025

Conversation

@MartyIX
Copy link
Contributor

@MartyIX MartyIX commented Aug 22, 2025 •
edited
Loading

Description of Change

This PR is cherry-picked #31219 with an added UI test to see the result.

The PR is structured to commits this way:

  • The first commit adds a test
  • The second commit implements a simple way to set the LineStackingStrategy=BlockLineHeight in code (I believe it adds performance overhead)
  • The last commit implements the same change but by setting a default style in XAML to avoid setting LineStackingStrategy needlessly.

Comparison of screenshots on Windows

This is just for comparison reasons:

Currently PR
image image

Issues Fixed

Fixes #24520

@MartyIX MartyIX requested a review from a team as a code owner August 22, 2025 11:48
@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Aug 22, 2025
@MartyIX MartyIX force-pushed the feature/2025-08-22-windows-Change_the_LineStackingStrategy_to_BlockLineHeight_for_Labels branch from 4e88956 to f716eec Compare August 22, 2025 11:54
@PureWeen
Copy link
Member

PureWeen commented Aug 22, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Aug 22, 2025

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen PureWeen added this to the .NET 10.0-rc2 milestone Aug 23, 2025
@PureWeen
Copy link
Member

PureWeen commented Aug 24, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Aug 24, 2025

Azure Pipelines successfully started running 3 pipeline(s).

@MartyIX MartyIX force-pushed the feature/2025-08-22-windows-Change_the_LineStackingStrategy_to_BlockLineHeight_for_Labels branch from a8be0ad to be858ef Compare August 25, 2025 07:49
@jsuarezruiz
Copy link
Contributor

jsuarezruiz commented Aug 26, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Aug 26, 2025

Azure Pipelines successfully started running 3 pipeline(s).

jsuarezruiz
jsuarezruiz suggested changes Aug 28, 2025
View reviewed changes
@github-project-automation github-project-automation bot moved this from Todo to Changes Requested in MAUI SDK Ongoing Aug 28, 2025
@MartyIX MartyIX force-pushed the feature/2025-08-22-windows-Change_the_LineStackingStrategy_to_BlockLineHeight_for_Labels branch from be858ef to e5c9b52 Compare August 28, 2025 07:24
@jfversluis
Copy link
Member

jfversluis commented Aug 28, 2025

/azp run

@jfversluis jfversluis moved this from Changes Requested to Ready To Review in MAUI SDK Ongoing Aug 28, 2025
@azure-pipelines
Copy link

azure-pipelines bot commented Aug 28, 2025

Azure Pipelines successfully started running 3 pipeline(s).

@MartyIX
Copy link
Contributor Author

MartyIX commented Aug 28, 2025

Tests seem to pass.

@MartyIX
Copy link
Contributor Author

MartyIX commented Aug 28, 2025

@bhavanesh2001 Could you please review/take a look?

@PureWeen PureWeen added the p/0 Current heighest priority issues that we are targeting for a release. label Aug 30, 2025
@mattleibow
Copy link
Member

mattleibow commented Sep 1, 2025 •
edited
Loading

How does this work with spans? Looking at the WPF docs: https://learn.microsoft.com/en-us/dotnet/api/system.windows.controls.textblock.linestackingstrategy?view=windowsdesktop-9.0

We do support line heights on the label AND the spans... Not sure how they play together, but this PR may be regardless of how it looks. I do think that a 0.5 light height should apply, even if it looks a bit silly since the user did explicitly set it. You may actually want weird things with weird fonts...

<Page xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
  xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml">
  <StackPanel>
    
    <!-- This TextBlock has a LineStackingStrategy set to "MaxHeight". -->
    <TextBlock LineStackingStrategy="MaxHeight" LineHeight="10" Width="500" TextWrapping="Wrap" 
     Background="Yellow">
      Use the <Span FontSize="30">LineStackingStrategy</Span> property to determine how a line box is
      created for each line. A value of <Span FontSize="20">MaxHeight</Span> specifies that the stack
      height is the smallest value that contains all the inline elements on that line when those
      elements are properly aligned. A value of <Span FontSize="20">BlockLineHeight</Span> specifies
      that the stack height is determined by the block element LineHeight property value.
    </TextBlock>

    <!-- Here is the same TextBlock but the LineStackingStrategy is set to "BlockLineHeight". -->
    <TextBlock LineStackingStrategy="BlockLineHeight" LineHeight="10" Width="500" TextWrapping="Wrap" 
     Background="Blue" Margin="0,40,0,0">
      Use the <Span FontSize="30">LineStackingStrategy</Span> property to determine how a line box is
      created for each line. A value of <Span FontSize="20">MaxHeight</Span> specifies that the stack
      height is the smallest value that contains all the inline elements on that line when those
      elements are properly aligned. A value of <Span FontSize="20">BlockLineHeight</Span> specifies
      that the stack height is determined by the block element LineHeight property value.
    </TextBlock>

  </StackPanel>
</Page>
image

@MartyIX
Copy link
Contributor Author

MartyIX commented Sep 3, 2025

How does this work with spans?

Honestly, I don't really know. I need to take a deeper look.

@mattleibow mattleibow moved this from Ready To Review to In Progress in MAUI SDK Ongoing Sep 11, 2025
jsuarezruiz
jsuarezruiz previously requested changes Sep 11, 2025
View reviewed changes
src/Controls/tests/TestCases.HostApp/Issues/Issue24520.xaml Outdated
<Label BackgroundColor="Green" LineHeight="1.4" FontSize="20" Text="ÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑ"/>
<Label BackgroundColor="Red" LineHeight="1.6" FontSize="20" Text="ÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑ"/>
<Label BackgroundColor="Green" LineHeight="2.0" FontSize="20" Text="ÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑ"/>
<Label BackgroundColor="Red" LineHeight="4.6" FontSize="20" Text="ÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑÑ"/>
Copy link
Contributor

@jsuarezruiz jsuarezruiz Sep 11, 2025

Choose a reason for hiding this comment

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

Could we update the test to cover more cases? For example, using Spans.

Also, what happens with mixed fonts?

<Label>
    <Label.FormattedText>
        <FormattedString>
            <Span Text="Normal " FontFamily="Arial"/>
            <Span Text="Special" FontFamily="CustomFont"/>
        </FormattedString>
    </Label.FormattedText>
</Label>

With BlockLineHeight, if fonts have different block heights, this could cause inconsistent line spacing within the same label.

Copy link
Contributor

@jsuarezruiz jsuarezruiz Sep 11, 2025

Choose a reason for hiding this comment

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

Also, could we include some Labels inside containers with constrained height?

Copy link
Contributor Author

@MartyIX MartyIX Sep 12, 2025

Choose a reason for hiding this comment

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

I tried to improve the test a bit:

image

However, it's not finished. I'm a bit out of time for a few days.

Copy link
Contributor Author

@MartyIX MartyIX Sep 17, 2025

Choose a reason for hiding this comment

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

Improved version:

image

@github-project-automation github-project-automation bot moved this from In Progress to Changes Requested in MAUI SDK Ongoing Sep 11, 2025
@MartyIX MartyIX force-pushed the feature/2025-08-22-windows-Change_the_LineStackingStrategy_to_BlockLineHeight_for_Labels branch from e5c9b52 to cd25072 Compare September 12, 2025 09:33
@PureWeen
Copy link
Member

PureWeen commented Sep 12, 2025 •
edited by jfversluis
Loading

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Sep 12, 2025

Azure Pipelines successfully started running 3 pipeline(s).

PureWeen
PureWeen requested changes Sep 15, 2025
View reviewed changes
Copy link
Member

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

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

src/Controls/tests/TestCases.HostApp/Issues/Issue24520.xaml(107,40): Error XC0009: No property, BindableProperty, or event found for "WordWrap", or mismatching type between value and property.

@MartyIX
Copy link
Contributor Author

MartyIX commented Sep 17, 2025

src/Controls/tests/TestCases.HostApp/Issues/Issue24520.xaml(107,40): Error XC0009: No property, BindableProperty, or event found for "WordWrap", or mismatching type between value and property.

Should be fixed

@MartyIX MartyIX force-pushed the feature/2025-08-22-windows-Change_the_LineStackingStrategy_to_BlockLineHeight_for_Labels branch from 2d49129 to 5c97da7 Compare September 17, 2025 07:03
@azure-pipelines
Copy link

azure-pipelines bot commented Sep 17, 2025

Azure Pipelines successfully started running 3 pipeline(s).

@azure-pipelines
Copy link

azure-pipelines bot commented Sep 17, 2025

Azure Pipelines successfully started running 3 pipeline(s).

MartyIX
MartyIX commented Sep 17, 2025
View reviewed changes
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue24520.cs
@@ -0,0 +1,23 @@
#if TEST_FAILS_ON_ANDROID // https://github.com/dotnet/maui/issues/24504
Copy link
Contributor Author

@MartyIX MartyIX Sep 17, 2025

Choose a reason for hiding this comment

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

See this comment: #24504 (comment)

@PureWeen PureWeen moved this from Changes Requested to Approved in MAUI SDK Ongoing Sep 18, 2025
@rmarinho rmarinho merged commit e594740 into dotnet:net10.0 Sep 19, 2025
149 checks passed
@github-project-automation github-project-automation bot moved this from Approved to Done in MAUI SDK Ongoing Sep 19, 2025
@MartyIX MartyIX deleted the feature/2025-08-22-windows-Change_the_LineStackingStrategy_to_BlockLineHeight_for_Labels branch September 19, 2025 08:40
@github-actions github-actions bot locked and limited conversation to collaborators Oct 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@PureWeen PureWeen PureWeen approved these changes

@StephaneDelcroix StephaneDelcroix Awaiting requested review from StephaneDelcroix StephaneDelcroix was automatically assigned from dotnet/dotnet-maui-reviewers

@jfversluis jfversluis Awaiting requested review from jfversluis jfversluis was automatically assigned from dotnet/dotnet-maui-reviewers

+1 more reviewer

@jsuarezruiz jsuarezruiz jsuarezruiz left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

community ✨ Community Contribution needs-breaking-change-doc-created p/0 Current heighest priority issues that we are targeting for a release. platform/windows t/breaking 💥

Projects

MAUI SDK Ongoing
Status: Done

Milestone

.NET 10.0-rc2

Development

Successfully merging this pull request may close these issues.

6 participants

@MartyIX @PureWeen @jsuarezruiz @jfversluis @mattleibow @rmarinho