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 and preferably, bidi support #1637

Closed
ahangarha opened this issue Jun 1, 2023 · 7 comments · Fixed by #1654
Closed

Add RTL and preferably, bidi support #1637

ahangarha opened this issue Jun 1, 2023 · 7 comments · Fixed by #1654
Labels
1. to develop Accepted and waiting to be taken care of enhancement New feature or request
Milestone

Comments

@ahangarha
Copy link

Nextcloud:

  • Nextcloud-Version: 25.0.5
  • Forms-Version: 3.3.0

Is your feature request related to a problem? Please describe.
Currently, there is no support for RTL or bidi (bidirectional text) support. As a result, LTR is the enforced direction for the form.

Describe the solution you'd like

  • For RTL/LTR, there can be a simple toggle option. Than can simply toggle direction of the form container (either as HTML or CSS property) There is no need for custom rtl CSS. Using logical CSS properties for alignment, spacing,... (e.g. text-align: start or padding-inline-start,...) can help to not bring complication to the styling.
  • For bidi, there are more works to do. I am available to discuss the implementation though I might not be able to code.

Describe alternatives you've considered
A custom CSS field for customizing the form would be also good, but this is entirely another feature.

Additional context
I don't think it be needed unless you need my help with bidirectional text support.

Regards

@ahangarha ahangarha added 0. Needs triage Pending approval or rejection. This issue is pending approval. enhancement New feature or request labels Jun 1, 2023
@Chartman123 Chartman123 added 1. to develop Accepted and waiting to be taken care of and removed 0. Needs triage Pending approval or rejection. This issue is pending approval. labels Jun 23, 2023
@susnux
Copy link
Collaborator

susnux commented Jun 24, 2023

This would be a great addition, I think there are just a few places where RTL works.
(I just checked the personal settings page of Nextcloud but the text is still left aligned when using a RTL language).

Edit: I roughly checked our CSS, I think as soon as we add direction: rtl to the body element everything should work.

@ahangarha
Copy link
Author

Mostly yes, but not enough. There are a few cases of using values like padding-left|right that should get changed to logical properties like padding-inline-start|end. That ensures the alignment and spacing works fine based on the direction.

@ahangarha
Copy link
Author

But I would like to suggest using dir="auto" on user-generated elements. This way, there is no need to explicitly mention the direction of the whole form. This way, even in RTL layout, one can add LTR values (e.g, English name) that get displayed in the correct direction. This becomes crucial when the text has mixed content. This way we get them in the current direction.

And I would say, this should happen regardless of the direction used in NextCloud by the person creating the form.

If it sounds complex, let us work on it. I am gathering a group of Iranians to work for a couple of weeks to improve NextCloud and its apps' support for RTL languages (to be more precise, support for bidirectional text support.

@ahangarha ahangarha changed the title Add RTL and preferablty, bidi support Add RTL and preferably, bidi support Jun 25, 2023
@susnux
Copy link
Collaborator

susnux commented Jun 25, 2023

Comment from @ahangarha nextcloud-libraries/nextcloud-l10n#639 (review)

It seems you use the locale used by the person creating the form. This is not ideal. I might use English but I want to create a form in RTL (or the opposite).

There are two proposals:

Add an option to set the direction of the form (this can get its default value from the locale of the creator of the form). This is flexible but not yet not preferred.
Make any user-generated field/content to set the direction automatically using dir="auto". Here, we need to also fix any styles related to left|right to use logical styles (like text-aligh: start or padding-inline-start|end,...

I prefer the last one. It is very flexible and no magic logic is required to be implemented.

@susnux
Copy link
Collaborator

susnux commented Jun 25, 2023

So I tested a bit, sadly I do not speak any RTL language so I highly appreatiate any help by you @ahangarha :)
If I see it correctly we need two things:

  1. Global direction property to set the design of the site itself, e.g. dir='rtl' on the body tag.
  2. Any user defined content = everywhere we use the translation function + where users can enter text need the dir='auto' tag.

Am I correct @ahangarha ?


Because currently if I set my language to Arabic, which is RTL, it looks wrong, the text seems to be left aligned.

current default with body dir="rtl" with `dir="auto" on user content
image image image

Note the wrong paddings and margins in the second image because of the padding-left instead of padding-inline-start and the wrong text alignment for English text.
Also on the third image I only mocked some elements to demonstrate the dir=auto.

@susnux
Copy link
Collaborator

susnux commented Jun 25, 2023

@ahangarha Besides using *-inline and *-block for padding and margin, I think we need to use inset instead of left right top bottom for positioning, am I right?

@ahangarha
Copy link
Author

ahangarha commented Jun 25, 2023

@susnux You have done a pretty good job! The PR looks good.
And you are right about properties used for absolute positioning.

One point is that regardless of the global direction of the page, form should show text in correct direction.

@Chartman123 Chartman123 added this to the 3.4 milestone Oct 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants