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

Refactor text trimming and implement prefix trimming. #7322

Merged
merged 8 commits into from
Mar 15, 2022

Conversation

MarchingCube
Copy link
Collaborator

@MarchingCube MarchingCube commented Jan 7, 2022

What does the pull request do?

Current text trimming setup was fairly limiting and didn't allow users to customize behavior (for example using custom ellipsis symbols) or parametrize trimming.

I'm also adding new kind of trimming that could be found in programs like Adobe After Effects (trimming that keeps a fixed prefix and has ellipsis in the middle). Such trimming is useful in cases where prefix and suffix is more important than content in the middle.

This trimming mode is configurable so new changed API is immediately useful here.

prefix-trimming.mp4

I've implemented XAML compiler intrinsic handling for both text trimming and decorations. Previously if one used TextDecorations="Underline" it would create a completely new collection of decorations.

What is the current behavior?

One cannot easily configure trimming behavior and only trailing word/character trimming is available.

What is the updated/expected behavior with this PR?

Users can create custom trimming configuration and use it with TextBlock. One can enable new PrefixEllipsis to see new trimming in action.

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

Using an enum is not enough to configure trimming fully so instead it is a class. I've added static properties that have matching names with old enum members, this is comparable to TextDecorations.

Checklist

Breaking changes

TextTrimming is no longer an enum. This breaks binary compatibility. XAML files should compile as is, most of usages in code will work too. Only cases when user expected TextTrimming to be an enum won't work.

Obsoletions / Deprecations

Fixed issues

@robloo
Copy link
Contributor

robloo commented Jan 7, 2022

I was worried when I saw the TextTrimming enum being dropped -- there is a lot of existing code that uses this both in XAML and C#. This is a smartly designed change though and it looks easy to integrate into existing apps. The new PrefixEllipsis is very welcome and has been missing, well, forever in XAML. It's a great idea!

Using "This is the longest name" as an example it seems the following is possible:

Word: "This is the longest..."
Character: "This is the longest na..."
PrefixEllipsis[4]: "This...longest name"

What about the case where only the beginning should be trimmed with an ellipse? This is useful for file paths and other types of text where most significant info is at the end. In WPF there was no way to do this: "... the longest name". I'm assuming it can be done now with PrefixEllipsis and the length set to zero? Just want to be sure that case is fully supported.

Then I wonder if actually PrefixEllipsis with length set to zero should be the default instead of an arbitrary length of 8 characters. This would make it symmetrical by default with character trimming at the end.

Finally, can the PrefixEllipsis length be set in XAML? If so, what is the syntax? The docs really should be updated for that.

Edit: The name "PrefixEllipsis" where did it come from? It doesn't actually match with the other terms. It might be better to call this "PrefixCharacterEllipsis" and leave the door open for a future "PrefixWordEllipsis" as well.

@timunie
Copy link
Contributor

timunie commented Jan 8, 2022

Hi,

Imo this is an interesting idea. I have two questions/suggestions if you don't mind:

  1. What if the important content is at the beginning, but one wants also to keep the last lets say 4 chars? Would that be possible?
  2. A readonly bool IsTrimmed may be a good addition. So the developers could provide a tooltip if trimmed, and only if trimmed.

Happy coding
Tim

@Gillibald
Copy link
Contributor

Hi,

Imo this is an interesting idea. I have two questions/suggestions if you don't mind:

  1. What if the important content is at the beginning, but one wants also to keep the last lets say 4 chars? Would that be possible?
  2. A readonly bool IsTrimmed may be a good addition. So the developers could provide a tooltip if trimmed, and only if trimmed.

Happy coding Tim

Trimming happens on TextLine level and for collapsed lines this is set https://github.com/AvaloniaUI/Avalonia/blob/master/src/Avalonia.Visuals/Media/TextFormatting/TextLine.cs#L56

@Gillibald
Copy link
Contributor

Maybe it might make sense to introduce a position property on TextTrimming so we are able to chose on which side the text is trimmed. PrefixLength will become OmittedLength or something.

@timunie
Copy link
Contributor

timunie commented Jan 8, 2022

Hi,

Imo this is an interesting idea. I have two questions/suggestions if you don't mind:

  1. What if the important content is at the beginning, but one wants also to keep the last lets say 4 chars? Would that be possible?
  2. A readonly bool IsTrimmed may be a good addition. So the developers could provide a tooltip if trimmed, and only if trimmed.

Happy coding Tim

Trimming happens on TextLine level and for collapsed lines this is set https://github.com/AvaloniaUI/Avalonia/blob/master/src/Avalonia.Visuals/Media/TextFormatting/TextLine.cs#L56

Thank you for your detailed reply

So I could check if this property is true for any line, if I want to know if any trimming happened?

public​ ​abstract​ ​bool​ ​HasOverflowed​ { ​get; }

@Gillibald
Copy link
Contributor

Hi,
Imo this is an interesting idea. I have two questions/suggestions if you don't mind:

  1. What if the important content is at the beginning, but one wants also to keep the last lets say 4 chars? Would that be possible?
  2. A readonly bool IsTrimmed may be a good addition. So the developers could provide a tooltip if trimmed, and only if trimmed.

Happy coding Tim

Trimming happens on TextLine level and for collapsed lines this is set https://github.com/AvaloniaUI/Avalonia/blob/master/src/Avalonia.Visuals/Media/TextFormatting/TextLine.cs#L56

Thank you for your detailed reply

So I could check if this property is true for any line, if I want to know if any trimming happened?

public​ ​abstract​ ​bool​ ​HasOverflowed​ { ​get; }

You can look for lines that have HasCollapsed set to true

@timunie
Copy link
Contributor

timunie commented Jan 8, 2022

Thank you @Gillibald. Now I got it. 👍

@robloo
Copy link
Contributor

robloo commented Jan 8, 2022

I realize we probably need a separate spec discussion for this. Fundamentally, there are a few different generalized concepts:

  • BreakMode : By 'Word' or 'Character'
  • EllipsisLocation : Start (new), End
    • Note that Middle is not required as the PrefixLength/SuffixLength imply this instead
  • PrefixLength/SuffixLength or Length (new)
    • PrefixLength/SuffixLength can be the same property as the start/end information is provided separately (not sure a good name though)
    • Specifying a length will position the ellipsis in the middle of the text

This all could and probably should be calculated using a single generalized method which would support:

Break Location Example
Word End "first second ..." Length=0
Word Start "... second third" Length=0
Word Start "first... third" Length=1
Character End "first second th..." Length=0
Character End "first seco...hird" Length=4
Character Start "...st second third" Length=0
Character Start "firs...cond third" Length=4
  • Character break mode supports a configurable PrefixLength as specified above but should be extended to support a SuffixLength as well. This would allow ellipsis in the middle that are fully configurable.
  • Prefix/SuffixLength can also apply to Word break mode. Fallback logic is as follows:
    • If the length is too long to display, reduce the length to the next integer that fits
    • If even length=1 cannot fit the entire word, switch to using standard character break mode with no length
  • Using Prefix/SuffixLength with Word break mode is a bit more complicated to implement and isn't needed in the initial implementation. However, it should be included in the spec so it can be added later.

Usage in code would involve the following fields:

  1. None (default)
  2. Clip Optional from UWP and CSS spec that trims text by clipping at the pixel level instead of removing characters or words
  3. CharacterEllipsis represents BreakMode=Character, EllipsisLocation=End, Length=0
  4. WordEllipsis represents BreakMode=Word, EllipsisLocation=End, Length=0
  5. PrefixCharacterEllipsis represents BreakMode=Character, EllipsisLocation=Start, Length=0
  6. PrefixWordEllipsis represents BreakMode=Word, EllipsisLocation=Start, Length=0

Open Questions:

  1. How to make PrefixLength and SuffixLength configurable in XAML. Some special syntax like the nth-child selector may be needed.
  2. Rules for right-to-left and symbolic languages (without a distinction between words/characters). Although I can't think of any special cases needed.
  3. Defaults for PrefixLength and SuffixLength, strong suggestion is zero for both

@Gillibald
Copy link
Contributor

Under CSS there is a spec for this without a prefix length: https://developer.mozilla.org/en-US/docs/Web/CSS/text-overflow

It is possible to trim text at both sides and also define a custom symbol individually. So we could adapt this syntax but need to add the Prefix/SuffixLength

@robloo
Copy link
Contributor

robloo commented Jan 8, 2022

CSS has some good ideas like making the overflow indicator a configurable string. That could also be implemented relatively easily in the underlying functionality here.

I will also amend the table above. PrefixLength/SuffixLength can really be the same property. It can also apply to word break mode.

@jmacato
Copy link
Member

jmacato commented Mar 10, 2022

Closing due to inactivity and merge conflicts. @MarchingCube reopen this if it's gonna be worked on again :)

@jmacato jmacato closed this Mar 10, 2022
@Gillibald Gillibald reopened this Mar 10, 2022
@Gillibald
Copy link
Contributor

Will resolve conflicts in a few hours

@robloo
Copy link
Contributor

robloo commented Mar 10, 2022

I don't think this is quite general-purpose enough. Merging as-is may very well be a restriction in the future.

This is brand-new functionality. API and features should be well discussed IMO.

@Gillibald
Copy link
Contributor

Had a quick look at this and it seems master did diverge too much recently. We might need to recreate everything on top of master.

@Gillibald
Copy link
Contributor

@robloo We can add more advanced trimming modes after this PR got merged. This is mainly aiming on being able to customize the text trimming by replacing the TextTrimming enum with a class that is responsible for TextLine collapsing.

@robloo
Copy link
Contributor

robloo commented Mar 11, 2022

@robloo We can add more advanced trimming modes after this PR got merged. This is mainly aiming on being able to customize the text trimming by replacing the TextTrimming enum with a class that is responsible for TextLine collapsing.

Alright, as long as the API isn't locked after this. Some of my comments about naming of the new class members should be considered at least though.

@MarchingCube MarchingCube force-pushed the refactor-text-trimming branch from a17f110 to 05fc177 Compare March 14, 2022 00:06
@MarchingCube
Copy link
Collaborator Author

@Gillibald I have found some time to resolve conflicts. Since text formatting changes are mostly wrapped up we can revisit this PR again.

I've added a TODO to both ellipsis functions since I'm not dealing with flow direction yet. Also I've added TryMeasureCharactersBackwards as a temporary placeholder until we figure API.

If prefix trimming is too controversial to include now we can also look into text formatting API to let users implement it externally. Since I would be fine with moving this implementation to my project until we figure out API surface and how to configure it. In my opinion it should only work with character level trimming since prefix length is given in characters.

Ellipsis symbol is already configurable in my PR (each trimming ctor takes it). Only problem would be XAML syntax although we might be missing a few features in there to make this work (constructor params primarily). Although I would expect users to either create a new static trimming property and reference it via x:Static, might be something for another contributor to pick up later.

@robloo
Copy link
Contributor

robloo commented Mar 14, 2022

In my opinion it should only work with character level trimming since prefix length is given in characters.

  1. This isn't matching/symmetrical with the existing TextTrimming functionality that supports both word and character (CharacterEllipsis, WordEllipsis).
  2. It would be non-obvious that PrefixEllipse applies only for character as it isn't part of the name like the other enum values.
  3. There is an opportunity loss if this PR only supports characters. It also restricts future expansion not to account for both PrefixCharacterEllipsis and PrefixWordEllipsis at least in name.
  4. The prefix Length property can be unitless. It doesn't need to represent only characters or words, it can be a unitless integer for both.
  5. A good algorithm would support everything I outlined anyway as it is symmetrical functionality. You are going to have to implement all of the pieces anyway.

My recommendation is to at least:

  1. Change the name from PrefixEllipse -> PrefixCharacterEllipsis to allow future expansion without breaking changes
  2. Use a unitless Length property that could in the future apply for both character and word trimming
  3. Make Length default to 0 so functionality of PrefixCharacterEllipsis is intuitively the inverse of CharacterEllipsis

@MarchingCube MarchingCube force-pushed the refactor-text-trimming branch from c91a9ae to 47ad3d7 Compare March 14, 2022 23:25
@MarchingCube MarchingCube marked this pull request as ready for review March 14, 2022 23:25
@MarchingCube MarchingCube requested a review from Gillibald March 14, 2022 23:25
@MarchingCube MarchingCube changed the title WIP: Refactor text trimming and implement prefix trimming. Refactor text trimming and implement prefix trimming. Mar 14, 2022
@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 0.10.999-cibuild0019298-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

@jmacato jmacato enabled auto-merge March 15, 2022 11:48
@jmacato jmacato merged commit 3e674da into AvaloniaUI:master Mar 15, 2022
@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 0.10.999-cibuild0019301-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

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.

7 participants