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

t/1151: Implemented the right–to–left (RTL) languages support for the UI and the content #1881

Merged
merged 26 commits into from
Aug 12, 2019

Conversation

oleq
Copy link
Member

@oleq oleq commented Jul 11, 2019

Suggested merge commit message (convention)

Feature: Implemented the right–to–left (RTL) languages support for the UI and the content (see #1151).


Additional information

This is also a constellation branch. Other PRs in the constellation:

@oleq oleq removed the type:sub-pr label Jul 30, 2019
@oleq oleq requested a review from Reinmar August 5, 2019 14:17
@oleq oleq marked this pull request as ready for review August 5, 2019 14:18
Copy link
Member

@Reinmar Reinmar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All fine in this PR.

@Reinmar
Copy link
Member

Reinmar commented Aug 7, 2019

OK, I finished reviewing other PRs. There are some R-es but mostly due to implementation details and API. The whole thing works fine.

Two things that I found but which are not blocking:

  1. The keyboard navigation around widgets is broken.
  2. I miss RTL tests for all features. Where did you actually test them? The best way I found was to override the language in Locale() and build all manual tests.

@Reinmar
Copy link
Member

Reinmar commented Aug 7, 2019

And there's also #1151 (comment) which should be fairly easy to resolve.

@oleq
Copy link
Member Author

oleq commented Aug 8, 2019

  • The keyboard navigation around widgets is broken.

https://github.com/ckeditor/ckeditor5-widget/issues/97

And there's also #1151 (comment) which should be fairly easy to resolve.

https://github.com/ckeditor/ckeditor5-table/issues/200

2. I miss RTL tests for all features. Where did you actually test them? The best way I found was to override the language in Locale() and build all manual tests.

Those features that are affected by UI language/content language and RTL in an obvious way (indent, alignment, editor-inline) got their new rtl.js manual tests.

Other features that have very little (block quote) or nothing (font) to do with RTL can be checked in docs (UI language guide) or in the locale.js manual test (ckeditor5-utils).

I don't see any good reason for creating an RTL test for each feature (as a rule of thumb).

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.

3 participants