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

[i18n] RTL support #690

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

[i18n] RTL support #690

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b65a373
[rtl] Add `dir=rtl` for right to left languages
sepans Apr 13, 2020
7360072
[rtl] Use `flex` instead of `float` for 'Hello' and search input
sepans Apr 13, 2020
23d0c46
[rtl] rtl css style overrides
sepans Apr 13, 2020
6da4690
[rtl] Move `sidebar` to layout for all pages and use `flex` for conta…
sepans Apr 13, 2020
b5fd450
[rtl] PR comments
sepans May 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions src/assets/css/main.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1168,7 +1168,6 @@ img.gallery-img {
#showcase-page .showcase-intro h1 {
font: italic 900 14.5vw "Montserrat", sans-serif;
color: #ED225D;
text-align: left;
Copy link
Member Author

Choose a reason for hiding this comment

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

👆 was not doing anything except breaking rtl 😄

text-transform: uppercase;
}

Expand Down Expand Up @@ -2019,7 +2018,7 @@ footer {
#i18n-btn {
position: absolute;
top: 2.5em; /* temp promo, 4.0em */
right: 1em;
margin: 0 1em;
}
#i18n-btn a {
font-family: "Montserrat", sans-serif;
Expand Down Expand Up @@ -2167,7 +2166,6 @@ footer {
.column-span {
margin: 0 1em 0 1em;
padding: 0;
float: left;
}
#menu.top_menu,
#menu {
Expand Down Expand Up @@ -2472,3 +2470,21 @@ iframe {
display: inline;
}
}

/* ======= Right-to-left modifications ======== */

html.rtl #lockup {
Copy link
Member

Choose a reason for hiding this comment

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

Can the selector be selecting the lang attribute on the html tag instead of using a class on the tag?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes that is possible. The downside is that for each new rtl language a lang item needs to be added (and possibly someone spending some time to figure it out and/or documenting it). To me, rtl class seems to be a small reasonable abstraction to cover them all.

Copy link
Member

Choose a reason for hiding this comment

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

I would like to try out just using the lang attribute for now, perhaps somewhere down the line we can look into automating the build to handle some of the details that needs to be added manually for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, what language should I put there? I can arbitrarily add either lang(fa) or lang(he), or all of them but in the above comment you suggested removing them all from the Gruntfile.js

Copy link
Member

Choose a reason for hiding this comment

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

lang(fa) for now would be fine, do add a line comment about what the function of it is just above.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry about the delay. I removed elements from rtlLangs array as you suggested, but as for adding :lang(fa) I think it is exactly opposite of what I did for rtlLangs: adding code specific to a language that is not supported yet.
And also it gets ugly as we eventually add multiple rtl languages and chain css for all the language.

left: auto;
right: 1.25em;
}

html.rtl #home-page #asterisk-design-element {
right: auto;
left: 20%;
}

@media screen and (max-width: 719px) {
html.rtl .sidebar-menu-icon {
float: left
}
}