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

Add RTL support + styles #86

Open
nadavkav opened this issue Jun 27, 2016 · 10 comments

Comments

Projects
None yet
5 participants
@nadavkav
Copy link
Contributor

commented Jun 27, 2016

It would be very useful to have the Moodle user's current language available to the IFRAME (or DIV) of the hvp content. and use that parameter (flag?) to enable various RTL features:

  • UI translation (PHP & JS))
  • Editor RTL alignment (+ Directionality RTL & LTR buttons + Initial editor directionality & alignment setup)
  • Content styles
@nadavkav

This comment has been minimized.

Copy link
Contributor Author

commented Jul 2, 2016

Applying the following suggested patch to h5p-php-library
h5p/h5p-php-library#23

Enabled me to upload a new H5P.multichoice content package with patched RTL CSS selectors, which gave initial RTL support.
nadavkav/h5p-multi-choice@053120e

@falcon-git

This comment has been minimized.

Copy link
Member

commented Jul 4, 2016

Reviewing this has been scheduled for next release. (Not the one we're working on now, but the one after this one. probably starts working on it in August)

@nadavkav

This comment has been minimized.

Copy link
Contributor Author

commented Jul 4, 2016

Ok
Just know that I am basing css fixes in ALL h5p content components (those from the library) on the above fix. (that adds an rtl class, I can count on) so please see if I am on the right track with your future road map about implementing RTL/LTR support.

@nadavkav

This comment has been minimized.

Copy link
Contributor Author

commented Oct 9, 2016

Hi @falcon-git , Any news on RTL support?

@fnoks

This comment has been minimized.

Copy link
Member

commented Oct 17, 2016

@nadavkav : This issue is scheduled for the release we are working on, but won't promise you any release date.

I am not any expert in this field, but shouldn't we use the dir attribute instead of adding a class?

@nadavkav

This comment has been minimized.

Copy link
Contributor Author

commented Oct 17, 2016

Thank you for considering it.
Moodle uses both, the dir attribute and a special classes dir-ltr / dir -rtl, to support cases where dir attribute is not enough. actually, dir-rtl is used a lot.
So it is actually best if we use the dir attribute too.
And I will use the special dir-rtl only in cases where we have no other way of fixing the UI in RTL mode.

@nadavkav

This comment has been minimized.

Copy link
Contributor Author

commented Oct 17, 2016

Though it would be very helpful to know what is going to be implemented, as I need to go through ALL the libraries and fix the interactions. (Locally, we fixes only a few we are currently using, but can not send PR as it is based on this fix)

@fnoks

This comment has been minimized.

Copy link
Member

commented Nov 17, 2016

I have closed the related pull request. Just create a new one if you are able to implement the solution we have discussed!

@mkpelletier

This comment has been minimized.

Copy link

commented Oct 5, 2017

Greetings,
Has there been any progress in integrating this? I am having issues working with RTL languages in the most recent H5P. Furthermore, the absence of any style support is a problem. These complex scripts work better with certain fonts, but the interactions editor does not seem to allow styling beyond what is available on the toolbar. Is there a reason we cannot access the underlying html to style our text?

Thanks again for all your effort and a fantastic product!

@nadavkav nadavkav changed the title Add RLT support + styles Add RTL support + styles Oct 5, 2017

@timothyylim

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2017

Hi @mkpelletier, as far as I am aware, there hasn't been any progress integrating this just yet. It was a UX decision to not let authors access the underlying HTML. It reduces complexity and makes it easier to render the view as the possible inputs are limited in scope.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.