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

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

Closed
1 task done
jonasbernardo opened this issue Jun 18, 2024 · 11 comments · Fixed by #2060
Closed
1 task done

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

jonasbernardo opened this issue Jun 18, 2024 · 11 comments · Fixed by #2060
Assignees
Labels
bug Something isn't working

Comments

@jonasbernardo
Copy link

jonasbernardo commented Jun 18, 2024

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

@jonasbernardo jonasbernardo added the bug Something isn't working label Jun 18, 2024
@AtlasAutocode
Copy link
Collaborator

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
Copy link
Collaborator

CatHood0 commented Jul 23, 2024

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
Copy link
Collaborator

CatHood0 commented Jul 23, 2024

You can also see this on

final nodeTextDirection = getDirectionOfNode(line);

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

TextDirection getDirectionOfNode(Node node) {

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
Copy link
Collaborator

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
Copy link
Collaborator

CatHood0 commented Jul 24, 2024

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 CatHood0 self-assigned this Jul 24, 2024
@CatHood0
Copy link
Collaborator

This will be complex

@CatHood0 CatHood0 mentioned this issue Jul 24, 2024
8 tasks
@CatHood0
Copy link
Collaborator

This will be complex

It was more easy that i thinked

@AtlasAutocode
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

CatHood0 commented Jul 24, 2024

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
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants