-
Notifications
You must be signed in to change notification settings - Fork 11.4k
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
Right to left writting is now possible #946
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for this contribution! really important step towards internationalization. please see some feedback and questions below. once addressed, I'd be happy to merge.
_layouts/default.html
Outdated
@@ -16,7 +16,8 @@ | |||
{%- include header.html %} | |||
|
|||
<!-- Content --> | |||
<div class="container mt-5"> | |||
<div class="container mt-5" {% if site.rtl_langs contains page.lang %}dir="rtl" align="right"{% endif %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's also worth adding "rtl"
class to this div, so that we can conditionally change CSS of some inner elements (eg, the border location of the blockquote).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy that you find it useful :) . I have to clarify one important fact: I'm by no means dominant on HTML, SCSS, and I'm afraid, even after going through Jekyll's documentation several times, I haven't fully comprehend its philosophy. So please take my response with a grain of salt, i.e., modify it as you feel it's better.
Coming back to your comment, I actually don't know how to add it to div
here. If you provide me with some hints, I would be happy to work on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, Thanks for this great idea.
It is a common practice (+) to add dir=rtl
in a separate div. So, maybe it is better to add it like this:
<div {% if site.rtl_langs contains page.lang %} dir="rtl" endif %>
Stuff
</div>
Also, I see that you have added align=right
. Why do you think that is necessary? I haven't seen it anywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi,
I was not aware of this practice, but it seems to work on my local machine.
align=right
is to decouple the text of each post from the body of the page which have some use cases. E.g., one might want to have the header and navigation in, say English, but the posts in a rtl script. An example post is here.
```scss | ||
blockquote { | ||
background: var(--global-bg-color); | ||
border-left: 2px solid var(--global-theme-color); // change to border-right: | ||
margin: 1.5em 10px; | ||
padding: 0.5em 10px; | ||
font-size: 1.2rem; | ||
} | ||
|
||
... | ||
|
||
|
||
.post-content{ | ||
blockquote { | ||
border-left: 5px solid var(--global-theme-color); // change to border-right: ... | ||
padding: 8px; | ||
} | ||
} | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of providing a suggestion on how to update CSS here, it's better the add rtl-conditional style overrides directly to _scss/_base.scss
, like this (I haven't tested the code below, so please adjust as necessary):
rtl blockquote {
border-left: none;
border-right: 2px solid var(--global-theme-color);
}
.post-content {
rtl blockquote {
border-left: none;
border-right: 2px solid var(--global-theme-color);
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll check this locally, and push a new commit if it worked on my side.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please review my new commit (lines 71-74 and 971-977).
unicode-bidi: bidi-override !important; | ||
direction: unset !important; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, I'm not as familiar with multi-directional language support. what does this do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBH, I don't know much rather than bidi
handles bi-directional scripting in the correct way. Often right-to-left scripts are not equal to a right-aligned version of a left-to-right script. Text becomes more shattered if there are numbers or special characters involved as well. Yet, with smart conditioning, one can prevent the misplacement of phrases. I guess, what bidi does is take care of all those cases.
Why these particular key-values are necessary to fix this, I don't know. I had this problem and (almost) followed the accepted answer. In my case, it worked.
The most recent push, fixes the location of the vertical line of blockquote with
Note: It seems to me that the lateral border highlighting is repeated in |
@arashgmn, thanks again for the implementation of |
Happy to help. And thanks for the awesome template! |
Also, we have a great font for writing |
In the same blog I posted earlier, I use The tricky part is to ensure |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Any update on this? Should I modify anything? |
I am not the best one to verify this, since I have no idea what are the rules when using left-to-right writing. @alshedivat @pourmand1376 |
Hi,
In addition, I added one new post to demonstrate that the sidebar indeed works an Cheers, Arash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please review my new commits and my recent reviews. And as before, thanks for the awesome template!
```scss | ||
blockquote { | ||
background: var(--global-bg-color); | ||
border-left: 2px solid var(--global-theme-color); // change to border-right: | ||
margin: 1.5em 10px; | ||
padding: 0.5em 10px; | ||
font-size: 1.2rem; | ||
} | ||
|
||
... | ||
|
||
|
||
.post-content{ | ||
blockquote { | ||
border-left: 5px solid var(--global-theme-color); // change to border-right: ... | ||
padding: 8px; | ||
} | ||
} | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please review my new commit (lines 71-74 and 971-977).
This branch helps the template's internationalization by providing right-to-left (rtl) scripting.
The
default
layout and all its derivates now automatically change their direction and alignment if a language with right-to-left scripting is specified in the front matter. The feature detects the following right-to-left languages (can be extended in_config.yml
):An example post is also provided in the Blog.