[SuperEditor] Add Valued Attribution support in SuperEditor (ColorAttribution, SizeAttribution, etc...)

[Sh4dord]

Hello, I write this issue as Feature, tell me if It's supported or not because I'm not able to find this behavior in Demo.

User Behavior
As a User, I use DemoEditor
I would like to change some text color from Selection or just from Cursor.

I use ToggleTextAttributionsRequest.
In some case, the Command cannot be applied du to 'canMerge' of ColorAttribute

Ex:
[color A]All my text is colored into A color[/color A]
If I decide to color any word between beacons: ERROR

Feature Value
When an attribution is associated with value Editor cannot split attribution or manage them to apply the new one if already the same Attribution is applied.

With this feature, editor will be able to use Attribution with value by applying attribution on some TextNode portions an not only paragraph.

Ex 1:
[color A]All my text is colored into A color[/color A]
If I decide to color any word between beacons: [color A]All my text is [/color A][color B]colored[/color B] [color A]into A color[/color A]

Ex 2:
Only the word [color A]color[/color A] is colored in A color
If I decide to color all the sentence with B color: [color B]Only the word color is colored in A color[/color B]

The behavior targeted is the same as a classic editor

This behavior can be the same with SizeAttribution for text size, Hightlight, FontFamily etc...

Additional context
If you need more, code example or else don't hesitate to ask for.

[KevinBrendel]

I also seem to be facing this issue.

I am not that familiar with the codebase, but it appears that e.g. ColorAttribution uses the same canMerge logic as NamedAttribution:

  @override
  bool canMergeWith(Attribution other) {
    return this == other;
  }

This works for NamedAttribution, as checking for the identity is exactly what we want. For ColorAttribution it doesn't work, as different colors still represent the same attribution, but will fail this check.

Checking for the id (could also check for the type instead) seems to work. Would this cause any further issues if we change it to that for the relevant attributions? @matthew-carroll

  @override
  bool canMergeWith(Attribution other) {
    return this.id == other.id;
  }

If the fix is that simple, I can make another pull request.

[KevinBrendel]

Ok, no, this doesn't work. It can look correctly, but it creates e.g. two overlapping ColorAttributions. A merge feature would need to be implemented in toggleAttribution/addAttribution that removes the old attribution of the same type for that range and then adds the new one.