[TextTrimming] Fixed some text trimming bugs

[dme-compunet]

What does the pull request do?

There are currently some bugs with text trimming.
@Gillibald sorry I'm sending this while you have an open PR on this issues.

What is the current behavior?

What is the updated/expected behavior with this PR?

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

Currently, text trimming collapses the text-runs in their visual order (and in reverse RTL), after this PR, it collapses the text-runs in their logical order (before bidi-reordering).

Checklist

• Added unit tests (if possible)?
• Added XML documentation to any related classes?
• Consider submitting a PR to https://github.com/AvaloniaUI/avalonia-docs with user documentation

Fixed issues

Fixes #14068
Fixes #17569
Fixes #17888

[dme-compunet]

@adirh3 can you check this branch and make sure it fixes the issues?

[dme-compunet]

For comparison, this is what PR #17899 does:

[rabbitism]

At least the chinese trimming doesn't make sense for me after change.

[Gillibald]

Thanks for this. I will have a closer look shortly. I am not so sure we should trim text in logical order.

[Gillibald]

I think you are right that we should trim text in logical order because otherwise text isn't split continuously.

We can't break public API in a minor release so we need to revert the breaking change. Mark the line with a TODO12 comment and add a comment to the breaking change tracking issue.

In general we need unit tests for this PR. My PR wasn't merged because I had no time working on tests yet. I am fine with dropping my PR in favor of my PR.

We need a test that ensures text is trimmed continuously.

We need a test that ensures the ellipsis is drawn at the right position.

We need tests that make sure bidi order isn't altered for trimmed text.

[dme-compunet]

I have now reverted the breaking changes. one advantage I see in passing the textRun parameter instead of textLine is allowing to a custom text-trimming implementation to get the logical text runs in an easy way.

It's not clear to me how this should be implemented so that it will merge into the next major version, so I'll leave it without breaking changes for now.

I will try to add the unit tests soon.

[avaloniaui-bot]

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

[avaloniaui-bot]

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

[avaloniaui-bot]

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

[dme-compunet]

@Gillibald there are also some bugs that need to be fixed in TextLeadingPrefixCharacterEllipsis, is it better to open this in a separate PR, or attach it with this PR?

[Gillibald]

A separate PR would be ideal

[dme-compunet]

@Gillibald Any thoughts about this? maybe it's best to leave it for v12 with the breaking changes?

[Gillibald]

You should at least create a related issue about your findings. Reviewing your PR shortly. Thanks for your contribution.

[avaloniaui-bot]

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