-
Notifications
You must be signed in to change notification settings - Fork 7.7k
Removed hard-coded LTR dir attributes from form elements #33675
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
Conversation
Directionality is set in the html opening tag only (in base/login/template.ftl) See also related issue keycloak#33642 which is to fix directionality in keycloak.v2/login/template.ftl Closes keycloak#32844 Signed-off-by: Jon Haynes <senyahnoj@gmail.com>
@Eng-Fouad @Mouhand-Kaddo @sharifrezvani as maintainers of the RTL languages, is this how you would expect the UI to work? Or is it normal to force a LTR direction in some of these cases? |
Switching all UI components to RTL is the normal behavior. However, it is just a personal preference to use LTR for text inputs in cases where the text is English and it starts or ends with special characters. For instance, let's assume we have 2 usernames named and here how it would look on LTR text inputs: |
Thanks for that feedback. Then if it is just personal preference I would say let's opt for going full RTL when a RTL language is enabled, as that simplifies things technically and reduces the amount of variants. |
@ahus1 since you have been involved in the other PRs and issues related to this, can I ask you for your take as well? |
I was merely the person who spotted the HTML change. I have no experience with LTR languages at all.
With @Eng-Fouad's comment above, I'd like to hear @senyahnoj's if this changed his mind, if if he wants to continue with the PR as is. Another aspect: This changes both the Login v1 and v2 themes. As this was originally what the v2 themes should do, I'd like to keep the v1 theme stable as this is the "old" theme we want people to migrate from. The reason I pointed towards the old theme was not to change the old theme, but to highlight that it was different in v1 for quite a while than it now in v2. I'm sorry if this caused confusion and extra work here. |
I was contemplating this as well, I feel like it should still be fine to land fixes in V1, but considering this is mostly a matter of opinion I agree the changes should only be applied for V2. This also simplifies any backporting to @senyahnoj could you make the proposed changes? |
OK. In that case I shall close this PR with the changes to both "base" and "keycloak.v2" and reinstate my previous PR which only made the change in the keycloak.v2 template #33652 |
Directionality is set in the html opening tag only (in base/login/template.ftl)
See also related issue #33642 which is to fix directionality in keycloak.v2/login/template.ftl
Closes #32844