t/200 Made the InsertColumnCommand work in the opposite direction when Loca… #216
Conversation
…le#contentLanguageDirection is 'rtl'.
|
I figured merge cell commands should be mirrored too. WIP. |
… language direction is RTL.
|
Now it's ready for review. |
There was a problem hiding this comment.
The code itself is pretty straight forward but I have doubts about who should dictate left/right here. I think that model and commands should be agnostic of the visual aspect of the text and work on different semantics - so it's "after" for insert column right in LTR but "before" for insert column right in RTL.
I'm flagging this as R- just for the sake of the tools we're using but we need to discuss this I think.
| // │ direction │ 'rtl' │ column │ column+1 │ | ||
| // └───────────┴───────┴───────────────┴──────────────┘ | ||
| // | ||
| if ( ( isOrderRight && isContentLtr ) || ( !isOrderRight && !isContentLtr ) ) { |
There was a problem hiding this comment.
This gets complicated with RTL. And I wonder if we shouldn't put emphasis on the UI for dictating "left"/"right" and have "before"/"after" in the model?
This way the command will accept "before" / "after" params from the UI and will execute them in the model order (as in arrays). The UI would switch command param depending on the content to match the visual aspect of it.
There was a problem hiding this comment.
In other words, the model manipulation should be simpler. It should focus on data direction as in after/before. The visual aspect of the editor, at least in here, could be done on the UI side of things.
This makes sense for me that
|
A better approach in #218. |
Suggested merge commit message (convention)
Fix: Made the
InsertColumnCommandandMergeCellCommandwork in the opposite direction whenLocale#contentLanguageDirectionis'rtl'. Closes ckeditor/ckeditor5#3277.