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.

Sign up for GitHub

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

Jump to bottom

feat: HTML lang attribute #53

Merged
merged 3 commits into from
Oct 4, 2023
Merged

feat: HTML lang attribute #53

merged 3 commits into from
Oct 4, 2023

Conversation

paulwer
Copy link
Contributor

@paulwer paulwer commented Sep 22, 2023 •
edited
Loading

As within file:
https://github.com/keycloak/keycloak/blob/main/themes/src/main/resources/theme/base/login/template.ftl

Keycloak sets the html default language tag to the current language, which can be usefull for screen readers or additional plugins.

keycloak/keycloak@4036324#diff-8f5ce48097e1dfc8ce6ebfc5b0c4f1a387d40cfe4abe8bde76da550e09574256

@lukin lukin changed the title Add html language tag - according to keycloak default Add HTML lang attribute Sep 25, 2023
@lukin
Copy link
Owner

lukin commented Sep 25, 2023

Hey @paulwer 👋

Thank you for all of your contributions. I'll merge this soon after updating the tests and will look at the rest of the pull requests afterwards.

@paulwer paulwer changed the title Add HTML lang attribute feat: HTML lang attribute Sep 26, 2023
@paulwer paulwer mentioned this pull request Sep 27, 2023
Closed
@paulwer
Copy link
Contributor Author

paulwer commented Sep 28, 2023

@lukin i guess the attribute should also be added within the test-files? should I add another commit including this?

f.ex. in https://github.com/lukin/keywind/blob/master/html/login/error.html
like:

<html lang="en">
  <head>
         [ ... ]
  </head>
  <body class=" ... ">
        [ ... ]
  </body>
</html>

@maxsivkov
Copy link

Hello

Should we also think about RTL languages (Arabic, Hebrew, Persian...)?
I'm not sure all browsers will respect lang attribute in the meaning of text direction

Currently I want to extend template.ftl and introduce dir attribute to the html tag

@paulwer
Copy link
Contributor Author

paulwer commented Oct 3, 2023

@maxsivkov I suggest you to create an pr or issue for this topic seperatly <3

@lukin
Copy link
Owner

lukin commented Oct 4, 2023

I apologize for my delayed response. From today, I will start a review of all these exciting changes. 🙂

I haven't had the chance to create the tests documentation yet, but here's a summary. When you run the mvn test command, it verifies the FreeMarker templates and automatically updates the HTML pages, which currently serve as snapshots. This enables a quick review of any modifications made on all the relevant pages without starting the Keycloak server. In the future, I intend to include these tests in the CI pipeline.

@lukin lukin merged commit 9beadbe into lukin:master Oct 4, 2023
@paulwer paulwer deleted the patch-2 branch October 5, 2023 05:05
@paulwer
Copy link
Contributor Author

paulwer commented Oct 5, 2023

very nice. thank you <3

lukin added a commit that referenced this pull request Oct 24, 2023
@paulwer @lukin
feat: add html lang attribute (#53)
a7d7a66 
Co-authored-by: Anthony Lukin <anthony@lukin.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@paulwer @lukin @maxsivkov