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

Right to Left Layout #2244

Closed
wants to merge 1 commit into from
Closed

Right to Left Layout #2244

wants to merge 1 commit into from

Conversation

nournia
Copy link

@nournia nournia commented Feb 9, 2014

I've added rtlLayout option for using CodeMirror as a right to left text editor. Provided example page shows its usage which is in Persian language (just like Arabic). I created this example to explain how it should work and at the moment it has some issues.

Rendered text and its styling is correct. But in this mode bidi algorithm's behavior must be changed. For example, in the following image, when I put my cursor in the beginning of the line, it stands after # character:

cursor in beginning of the line

Also behavior of multi-cursor algorithm, must be changed. Following image shows that when I move cursor to beginning character of word, it shows two cursor. When you are typing Left to Right, this behavior is right because editor doesn't know your next character is RTL or LTR, but in rtlLayout it is nonsense. In this state if you type RTL or LTR characters, CodeMirror appends them under gray cursor which is correct behavior and the black cursor is useless:

multi-cursor

Finally, mouse click and text selection is completely non-predictable. The correct behavior is obvious: put cursor just under mouse position.

There is similar pull request #1103 which targets same problem but it seems that its changes are removed now. As I said, I've created this simple example for making clear the expected behavior of editor in Right to Left mode, so questions are welcome.

add 'rltLayout' option that is a boolean field.
add demo page for this option
@marijnh
Copy link
Member

marijnh commented Feb 10, 2014

This is a great start! To fix the bidi algorithm issue, I think the only thing you need to do is to add a second parameter to the bidiOrdering function, which indicates the default direction, and then replace references to outerType with this parameter, and ensure that the call to bidiOrdering in getOrder passes the right value ("L" or "R") based on the value of the rtlLayout option.

You should also add an option declaration (under section OPTION DEFAULTS) that declares a handler for when this option changes -- similar to lineWrapping, most caches should be cleared when this option gets a new value (the order property on the line objects should also be cleared).

@marijnh
Copy link
Member

marijnh commented Feb 10, 2014

Also (you're probably aware of this) -- when there is a horizontal scrollbar, this is very broken. And due to issue #1757 , wrapped rtl lines are still very flaky (not just cursor motion, but also selection drawing).

@benbro
Copy link
Contributor

benbro commented Feb 24, 2014

Please consider adding RTL support on a line level and not just document level.
It's common in text editors to define the text direction and alignment per line.

@marijnh
Copy link
Member

marijnh commented Nov 11, 2014

There has been work on this happening in the direction branch. Testing appreciated.

@nournia
Copy link
Author

nournia commented Nov 25, 2014

Great work! It doesn't have anyone of reported bugs in this PR.

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.

None yet

3 participants