Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

T/ckeditor5/1151: Adjusted InsertTableView for better compatibility with right–to–left (RTL) languages #196

Merged
merged 4 commits into from
Aug 12, 2019

Conversation

oleq
Copy link
Member

@oleq oleq commented Jul 11, 2019

Suggested merge commit message (convention)

Other: Adjusted InsertTableView for better compatibility with right–to–left (RTL) languages. See ckeditor/ckeditor5#1151.


Additional information

A piece of ckeditor/ckeditor5#1881.

@coveralls
Copy link

coveralls commented Jul 25, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 92d8f87 on t/ckeditor5/1151 into 4569d21 on master.

@oleq oleq marked this pull request as ready for review August 5, 2019 14:18
@Reinmar
Copy link
Member

Reinmar commented Aug 6, 2019

I don't understand this change. Can you explain why spaces were bad?

@Reinmar Reinmar self-requested a review August 6, 2019 18:18
@Reinmar Reinmar self-assigned this Aug 6, 2019
@Reinmar
Copy link
Member

Reinmar commented Aug 7, 2019

There's also ckeditor/ckeditor5#1151 (comment) which perhaps we can squeeze into this PR (if not, let's make it a followup for it27).

@oleq
Copy link
Member Author

oleq commented Aug 8, 2019

When you have a table let's say 2 x 5 (LTR) it is 2 rows and 5 columns. But when you enable RTL UI things go backwards and the label is displayed x 5 2, which makes no sense. Maybe I should've used nbsp instead. I made this change at the beginning and didn't think it through.

There's also ckeditor/ckeditor5#1151 (comment) which perhaps we can squeeze into this PR (if not, let's make it a followup for it27).

Follow–up.

…ableView for better compatibility with RTL languages.
@oleq
Copy link
Member Author

oleq commented Aug 8, 2019

Sorted out the label problem. Used the multiplication sign '×' instead of 'x' letter and it looks great now.
image

image

@Reinmar
Copy link
Member

Reinmar commented Aug 12, 2019

Interesting problem :D I didn't think about this at all.

@Reinmar Reinmar merged commit 524586b into master Aug 12, 2019
@Reinmar Reinmar deleted the t/ckeditor5/1151 branch August 12, 2019 10:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants