Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
I'm working with panels on a arabic site. On rtl direction, rows must be reversed as shown
on the attached image (exemple.png).
The attached patch is fixing this on the flexible layout.
Comment | File | Size | Author |
---|---|---|---|
#10 | flexible_rtl_1377562_10.patch | 1.48 KB | tarekdj |
#5 | flexible_rtl_1377562_5.patch | 1.5 KB | Letharion |
flexible.inc_.patch | 1.35 KB | tarekdj | |
exemple.png | 16.67 KB | tarekdj |
Comments
Comment #1
Letharion CreditAttribution: Letharion commentedDoes it really make sense to hard-code this behaviour? While I have little experience with RTL websites, I find it hard to believe that it's a universal rule to reverse order of all content.
Comment #2
tarekdj CreditAttribution: tarekdj commentedhi Letharion,
I don't agree with you because all rtl laguages are supposed to be displayed from right to left.
An other thing that confirm it : the bartik core theme is automatically reversing on any rtl language
you can test this by adding a custom language with rtl direction.
thanks for your reply
Comment #3
merlinofchaos CreditAttribution: merlinofchaos commentedThe patch fails several code style tests:
Indentation is wrong
Missing braces on an if/else clause
(very very minor) missing a . at the end of the comment.
You may need to read about the Drupal coding style guidelines if you intend to continue contributing patches. The patch seems perfectly reasonable to me, if it can be improved to match code style, so I think the community would be well-served if you did this.
Comment #4
merlinofchaos CreditAttribution: merlinofchaos commentedAlso, retitling for clarity. The original title does not reflect what this patch accurately does, and created a misperception that skewed Letharion's review.
Comment #5
Letharion CreditAttribution: Letharion commentedIf you two say so :)
My in-experience in the RTL area probably made me confused.
Comment #6
merlinofchaos CreditAttribution: merlinofchaos commented+ if ($language->direction == 1) {
Is there a constant or something we can use so we don't compare this to the number 1? It looks like it should be LANGUAGE_RTL -- and that's much better than comparing against a number.
Comment #7
Letharion CreditAttribution: Letharion commentedCompare with RTL constant instead of 1.
Should code that uses the $language global at some point be converted to use the page manager language context in the future?
Comment #8
merlinofchaos CreditAttribution: merlinofchaos commentedI don't think there is a way to feed the layout a language context meaningfully. The global language is probably just fine.
It occurs to me that this probably should be optional (on by default for new layouts, off for existing layouts) in the canvas settings. It is entirely possible that there are people who have already set up their sites for RTL languages that would suddenly have their layouts flip on them. That would be unfortunate for them!
Comment #9
Letharion CreditAttribution: Letharion commented@merlin The language functionality I refer to is this one #1301908: Language support for Page Manager.
Comment #10
tarekdj CreditAttribution: tarekdj commentedChanged 1 by LANGUAGE_RTL.
Comment #11
tarekdj CreditAttribution: tarekdj commentedforgot to change the status in my last comment :-)
Comment #12
Letharion CreditAttribution: Letharion commented#10 does not adress the concerns in #8, and as far as I can tell looks no different from #5, so I set this back to needs work.
Comment #13
tarekdj CreditAttribution: tarekdj commentedI think, in that case, we can add another flexible layout with rtl support. Then the user has the choice to use with or within reverse. This will not affect the existant layouts.