search

visual-space / visual-editor

Rich text editor for Flutter based on Delta format (Quill fork)
MIT License
290 stars 46 forks source link

Bug/text direction rtl #148

Closed edenbd50 closed 10 months ago

edenbd50 commented 2 years ago

Goal: Support Right-To-Left text alignment for Visual Editor.

Problem:

1) Currently Visual Editor changes the whole block's directionality and does not effect the actual text inside.

For example:

RTL Language with Direction-LTR

<img width="105" alt="image" src="https://user-images.githubusercontent.com/9706212/193379683-b9fcbab9-f545-4f3b-ba7b-dd6c624567b5.png">

RTL Language with Direction-RTL

<img width="108" alt="image" src="https://user-images.githubusercontent.com/9706212/193379669-95354a3c-12e9-4151-8e17-585583b21f1b.png">

2) In Right-To-Left languages the default TextAlignment is reversed meaning Start to End and End to Start.

For example:

Normally its: `TextAlignment.start`, in RTL language it should be `TextAlignment.end`

<img width="497" alt="image" src="https://user-images.githubusercontent.com/9706212/193383805-fd97ebe1-d50e-49ca-bf7a-16a712391dd5.png">

Normally its: `TextAlignment.end`, in RTL language it should be `TextAlignment.start`

 <img width="491" alt="image" src="https://user-images.githubusercontent.com/9706212/193383811-2652be8f-33b2-492e-a189-860d46674ee7.png">

Solution:

RTLPage demo is added to Example pages:

Tests:

Note: In order to make logic testable the signature function of _textLineStylesUtils.getTextAlign was changed from receiving a LineM to receive AttributeM.

image
adrian-moisa commented 2 years ago

I've provided a bunch of feedback here and on discord.

A) Deleting last char on RTL errors out I promised to gate keep the bugs away. So I was doing some smoke testing and found out that if you delete down to the last character on a RTL line, we get an error. It's very hard to say at first sight what's that about. Can you have a look on this one?

B) Shouldn't there be a button to also reverse back to LTR ?

C) If I tap at the end of RTL line (left side) the cursor goes after the first character instead of in front of it Clicking in front of it is really difficult (needs precision)

D) The arrows move the caret in an inverted fashion I am new to RTL so maybe I have the wrong bias. But I think left arrow should move carte to left. It moves to right -> And then it reverts again if it goes trough the latin characters

E) The selection behavior is really odd in mixed mode Image

F) The caret jumps wildly in mixed mode

G) When retrieving latest develop, use rebase. Now it's a merge. And it creates a complex history. I'm keeping it super trimmed down for ease of reading. I can't squash it as it is right now. I'll see if I can force it somehow. The easiest would be if you copy past everything on a new branch such that I can get the squashed version.

Looks like github/chrome has the same implementation... I guess I'm not too familiar with how it should work.

Nice work on setting up the tests ❤️ I'm curios, I see there was some RTL attempt in the legacy code. What happened there? Why id did not work? Was it even attempted? So, let me know, what you think of the feedback and if you wish to tackle any of them in this commit or make new tickets. I'll wait for your answer before merging. By the looks of the places you changed, I think I wont collide with your changes in my work.

Also, if you are so kind, please try to cherypick/squash somehow the changes on top of the latest develop so we can do one single clean commit.

I hope I don't ask too much. Thanks a lot for the patience! Great work!

H) What about bullets? Are they expected to be on the right side or left side? And code block numbers

edenbd50 commented 2 years ago

I've provided a bunch of feedback here and on discord.

A) Deleting last char on RTL errors out I promised to gate keep the bugs away. So I was doing some smoke testing and found out that if you delete down to the last character on a RTL line, we get an error. It's very hard to say at first sight what's that about. Can you have a look on this one?

B) Shouldn't there be a button to also reverse back to LTR ?

C) If I tap at the end of RTL line (left side) the cursor goes after the first character instead of in front of it Clicking in front of it is really difficult (needs precision)

D) The arrows move the caret in an inverted fashion I am new to RTL so maybe I have the wrong bias. But I think left arrow should move carte to left. It moves to right -> And then it reverts again if it goes trough the latin characters

E) The selection behavior is really odd in mixed mode Image

F) The caret jumps wildly in mixed mode

G) When retrieving latest develop, use rebase. Now it's a merge. And it creates a complex history. I'm keeping it super trimmed down for ease of reading. I can't squash it as it is right now. I'll see if I can force it somehow. The easiest would be if you copy past everything on a new branch such that I can get the squashed version.

Looks like github/chrome has the same implementation... I guess I'm not too familiar with how it should work.

Nice work on setting up the tests ❤️ I'm curios, I see there was some RTL attempt in the legacy code. What happened there? Why id did not work? Was it even attempted? So, let me know, what you think of the feedback and if you wish to tackle any of them in this commit or make new tickets. I'll wait for your answer before merging. By the looks of the places you changed, I think I wont collide with your changes in my work.

Also, if you are so kind, please try to cherypick/squash somehow the changes on top of the latest develop so we can do one single clean commit.

  • use the ticket code in the name of the commit. I've started this convention a few commits ago.

I hope I don't ask too much. Thanks a lot for the patience! Great work!

H) What about bullets? Are they expected to be on the right side or left side? And code block numbers

A) Deleting last char on RTL errors out I think I have an Idea why it occurs, the last character of each row from my rtl.json example is “\n” which holds the RTL and Alignment properties. It appears that you can not use direction attribute directly to a Text node, only to the “\n” at the end otherwise it throws an error. Maybe because direction is considered a block type. It should behave as Quote / code block type. How do you think we should approach it?

B) Shouldn't there be a button to also reverse back to LTR ? It depends.. you can create direction ltr button that triggers attribute direction ltr. Since the default of the editor is already ltr It doesn’t matter, the rtl can behave as on / off(=ltr).

C) If I tap at the end of RTL line (left side) the cursor goes after the first character instead of in front of it Didn’t quite get what do you mean by after and in front , care to share an example?

D+E+F) The arrows move the caret in an inverted fashion, The selection behavior is really odd in mixed mode, The caret jumps wildly in mixed mode

G) When retrieving latest develop, use rebase. Yeah absolutely right 👍🏻 H) What about bullets? Are they expected to be on the right side or left side? Absolutely on right side , bulletin, checkbox , number list should all start from right. I didnt try to fix it in this pr since it’s considered out of scope and would have been messy. From the research Ive made, the bulletin is printed as Paint and not part of the sentence therefore direction / text alignment doesn’t apply to it. It uses Offset( 0 , … ) in order to paint it, so we need to attach it to RTL check and change it to the widget’s width minus the left padding of the actual bulletin. I considered it a fixable yet different PR imo ofc.