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

TextLayout: TextStyleOverrides aren't applied correctly #17922

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

solveEM
Copy link

@solveEM solveEM commented Jan 8, 2025

What does the pull request do?

This PR ensures that all TextStyleOverrides are applied correctly.

What is the current behavior?

Currently, the first TextRun is generated too long.
For example, we have the following text

Hello World\r\n
Hello

... and TextStyleOverrides to highlight the first two "He" at the beginning of each line:

IReadOnlyList<ValueSpan> textStyleOverrides = new List<ValueSpan>()
{
new ValueSpan(0, 2, new GenericTextRunProperties(typeface, backgroundBrush: Brushes.Aqua)),
new ValueSpan(13, 2, new GenericTextRunProperties(typeface, backgroundBrush: Brushes.Aqua)),
};

-> the first TextRun is generated too long:
image

What is the updated/expected behavior with this PR?

The first TextRun has the expected length:
image

How was the solution implemented (if it's not obvious)?

The FormattedTextSource->CreateTextStyleRun method was iterating through all the TextStyleOverrides (TextModifiers) and applying them to the TextRun until the last one was in scope. I changed it to apply the first TextStyleOverride in scope to the TextRun and return it immediately, as it makes no sense to apply more than one override to one TextRun? This makes the hasOverride flag obsolete.

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.3.999-cibuild0054143-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@cla-avalonia
Copy link
Collaborator

cla-avalonia commented Jan 8, 2025

  • All contributors have signed the CLA.

@solveEM
Copy link
Author

solveEM commented Jan 8, 2025

@cla-avalonia agree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants