singerdmx / flutter-quill

Rich text editor for Flutter
https://pub.dev/packages/flutter_quill
MIT License
2.46k stars 783 forks source link

problems when using languages ​​that use ltr instead of rtl #1928

Closed jonasbernardo closed 4 days ago

jonasbernardo commented 1 month ago

Is there an existing issue for this?

Flutter Quill version

9.4.2

Steps to reproduce

problem with languages ​​that use ltr instead of rtl, DirectionAttribute('ltr'); When I touch the enumerated list or checkbox option, even when using ltr in languages ​​like Hebrew, the checkboxes and enumerated lists do not change, they remain in the position as if using rtl

Expected results

checkbox and the list number go to the beginning of the line on the right side

Actual results

checkbox and list number are at the beginning of the line on the left side

Code sample

default quill

Screenshots or Video

No response

Logs

No response

AtlasAutocode commented 1 month ago

In my exploring the code, I have found that lists are implemented by hard coding the checkbox (or list number) into the left edge of the display irrespective of ltr or rtl. You can see this if you change the text alignment to center or right - the checkbox stays on the left edge with the text floating in the center - which looks weird! Fixing that is not trivial. More complex is the issue you raise of Hebrew with LTR but checkboxes on the right which would need some kind of language lookup. I think the whole concept of lists and alignments needs review. Anyone have any comments or solutions?

CatHood0 commented 4 days ago

I see the direction attribute, and i guess this one works on one way: If you want to force the content be RTL then the Attribute must be: DirectionAttribute('ltr'). But, i just want to content just be on the LTR, then just need to pass null (by default this is the value to force LTR)

You can see this on Attribute file:

// "attributes":{"direction":"rtl"}
static const Attribute<String?> rtl = DirectionAttribute('rtl');

And this is the default attribute:

static const DirectionAttribute direction = DirectionAttribute(null);

I think this behavior is weird

CatHood0 commented 4 days ago

You can also see this on https://github.com/singerdmx/flutter-quill/blob/6ecd377b26649a18d26d6b8fa77e5cf66d68f0d0/lib/src/editor/widgets/text/text_block.dart#L187

This get the Direction from the text using this method from delta_diff:

https://github.com/singerdmx/flutter-quill/blob/6ecd377b26649a18d26d6b8fa77e5cf66d68f0d0/lib/src/delta/delta_diff.dart#L92

This is the whole code from getDirectionOfNode:

TextDirection getDirectionOfNode(Node node) {
  final direction = node.style.attributes[Attribute.direction.key];
  if (direction == Attribute.rtl) {
    return TextDirection.rtl;
  }
  return TextDirection.ltr;
}

But i think this just be applying the Directionality to the text and is missing the leading widget from the Line

AtlasAutocode commented 4 days ago

We need an expert in right-to-left to get involved with a review of the whole concept.

@CatHood0 you can look at EditableTextLine in text_line.dart which contains a widget called 'leading'. Lists have the bullet or number hard coded to appear on the left irrespective of ltr or rtl. - see enum TextLineSlot.

CatHood0 commented 4 days ago

We need an expert in right-to-left to get involved with a review of the whole concept.

I think the same too.

@CatHood0 you can look at EditableTextLine in text_line.dart which contains a widget called 'leading'.

I'll try to fix it.

CatHood0 commented 4 days ago

This will be complex

CatHood0 commented 4 days ago

This will be complex

It was more easy that i thinked

AtlasAutocode commented 3 days ago

Looks good to me - well done.

nodeTextDirection == TextDirection.ltr && isRTL(context)

I think RTL can be set by the programmer in one of the configurations. If they set this, will this affect your logic? Personally, I think it is better to automatically select LTR/RTL based on the language in which case perhaps the configuration setting should be removed?

QuillSimpleToolbarConfigurations has a setting for: this.showDirection = false, This is used in the example code (simple_toolbar.dart) to show a button to toggle the directionality. Since this is better handled by detecting the language, @EchoEllet is there a protocol for removing code that is no longer appropriate?

There is also an attribute for rtl/ltr. Is this now obsolete? Should we remove DirectionAttribute?

EchoEllet commented 3 days ago

By default, the editor will use the same local as the one in the WidgetsApp or MaterialApp.

Which will use the same direction as the local, the local can be overridden, I'm not sure why this fix manually check the language or why currently we have a bug.

CatHood0 commented 3 days ago

I'm not sure why this fix manually check the language

I didn't have idea about Directionality.of(context) how works. I already make a fix to avoid manually checking the language

why currently we have a bug

This is because on raw_editor_state file, we used the Directionality from getDirectionOfNode instead of validate the current system direction and the direction attribute from the node. Then, the EditableTextBlock or EditableTextLine always receive the argument TextDirection.ltr even if the system language is RTL.

CatHood0 commented 3 days ago

I think RTL can be set by the programmer in one of the configurations. If they set this, will this affect your logic?

We will just need to modify the logic getting the direction from the configurations. But, i don't know if we need to do it.