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.
Problem
Content has a margin on the right proper, but right floated sidebar container will not appear on the right, upsetting the whole page structure.
Comment | File | Size | Author |
---|---|---|---|
#19 | menu-less-than-851px.jpg | 69.45 KB | mjohnq3 |
#19 | menu-less-than-560px.jpg | 53.17 KB | mjohnq3 |
#6 | fix-broken-layout-rtl-1979392-5.patch | 2.84 KB | thamas |
#5 | Screenshot_4_26_13_8_29_AM.png | 67.52 KB | Gábor Hojtsy |
#5 | Screenshot_4_26_13_8_33_AM.png | 46.19 KB | Gábor Hojtsy |
Comments
Comment #1
YesCT CreditAttribution: YesCT commentedComment #2
thamasIn progress, patch coming soon…
Comment #3
thamasHere is a basic patch which should fix the issue.
Comment #4
thamasThe patch in #3 is not right cause it places first sidebar to the left, however in RTL the first should be on the right (I think). So here is a new one which works as it should. (It is against the default state, not the patched version, so patch in commet #3 should be simply ignored. I hope I do it right…)
Comment #5
Gábor HojtsyThanks! This looks good on the desktop (where the 25% margins are affective). Also cross-checking the CSS, you applied the same rules reversed, so good :)
It does not seem to affect the responsive state in
@media all and (min-width: 560px) and (max-width: 850px)
where the sidebars are 50%. Just size your browser down to see:Also I don't think we removed the requirement to add /* LTR */ comments on properties in the original sheet which need modification in the RTL sheet so we don't forget them again like we did this time :D
In short, looks good on desktop, needs work on the smaller version and to add /* LTR */ comments to properties in the original sheet which are overriden in RTL.
Comment #6
thamasOK, here is the new patch to fix the issues you mention. I also made sidebar width to 100% when there is only one sidebar and screen width is below 850px. (I found a line in the code which suggested this, but it did not work:
.one-sidebar #sidebar {width: 100%;}
) If this is not the expected behaviour I can change this.Comment #7
thamasComment #8
Gábor HojtsyLooks amazing in RTL, it adapts properly to whatever screen size I put it at. Also, looks good in terms of the CSS changes themselves now reproducing the responsive sections in RTL that were sadly missed in the responsive conversion.
Comment #9
Gábor HojtsyPut on D8MI sprint for commit tracking.
Comment #10
webchickAwesome work!!
This looks great, apart from some minor spacing issues I noticed
/*LTR*/ -> /* LTR */
but I just fixed those on commit.Committed and pushed to 8.x. Thanks!
Comment #11
Gábor HojtsyWoot, thanks @thamas for jumping on this.
Comment #12
thamasWooo! My first core patch!
I'm happy and proud to be able to help! :)
Comment #13
YesCT CreditAttribution: YesCT commented@thamas congrats!
What made you jump in?
Comment #14
thamas@YesCT thanks!
I always want to help, but my resources (time, knowledge) are limited. I found the issue by reading Gábor's tweet about it. And I saw that I am able to fix it and found the time to do it. :)
Comment #15
YesCT CreditAttribution: YesCT commentedtweeting++
:)
For when you do have time... we are starting to tag frontend like issues to make them easier to find:
http://drupal.org/project/issues/search/drupal?status%5B%5D=Open&issue_t...
Comment #16
thamasThanks for the info! I saw the new tag in #1 but I have forgotten it since than… :)
Comment #17
webchickGreat work, thamas! Welcome to the team. :D
Comment #18
thamasThanks, Angie! :)
Comment #19
mjohnq3 CreditAttribution: mjohnq3 commentedSorry, but this change completely borked the Menu tabs (when they change to buttons). At less than 851px they should be three across, each approx. one-third the width of the screen. At less than 560px they should each be the full screen width, and stacked vertically. See the attached screenshots.
I'm trying to see if I can figure out what happened.
Comment #20
mjohnq3 CreditAttribution: mjohnq3 commentedComment #21
thamas@mjohnq3 The problem with menu tabs is not related to the changes here. The bug was there before the patches (I just tested that state). And I feel we will find other RTL issues as well. (We should check all the stylesheets for possible RTL problems: floats, different margins…)
So I think we should open a new issue for the menu tabs in RTL just to keep things more clear.
Comment #22
Gábor HojtsyLet's open a new issue and link it from here indeed! @mjohnq3: are you up for that?
Comment #23
mjohnq3 CreditAttribution: mjohnq3 commented@thamas - Yes, I can now see that this change didn't affect the menu problem.
@Gabor Hojtsy - I'll take a look as soon as I can. I had a hard drive crash and also was without internet access for
severalmany hours.Comment #24
tsi CreditAttribution: tsi commentedReference - new issue for the menu fix: #1994192: Bartik's menu tabs in RTL
Comment #25
Gábor Hojtsy@thamas: maybe you can help review/test #1994192: Bartik's menu tabs in RTL? :)
Comment #26
thamas@Gábor Hojtsy I'll do my best… :)