Main menu is broken on smaller screens in RTL and styles in general are not equivalent to the LTR styles.
Working on a patch.

CommentFileSizeAuthor
#11 bartik_menu_tabs_rtl-1994192-11.patch4.14 KBmjohnq3
#7 bartik_menu_tabs_rtl-1994192-7.patch2.59 KBtsi
#7 bartik_menu_tabs_rtl-1994192-7.patch2.59 KBtsi
#6 bartik_menu_tabs_rtl-1994192-6.patch2.6 KBmjohnq3
#6 interdiff.txt672 bytesmjohnq3
#4 bartik_menu_tabs_rtl-1994192-4.patch3.62 KBtsi
#3 bartik_menu_tabs_rtl.jpg43.57 KBmjohnq3
#3 bartik_menu_tabs_rtl-1994192-3.patch2.58 KBmjohnq3
#3 interdiff.txt563 bytesmjohnq3
#1 bartik_menu_tabs_rtl-1994192-1.patch2.98 KBtsi

Comments

tsi’s picture

tsi commented
Status: Active » Needs review
StatusFileSize
new bartik_menu_tabs_rtl-1994192-1.patch2.98 KB
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue tags: +D8MI
mjohnq3’s picture

mjohnq3 commented
StatusFileSize
new interdiff.txt563 bytes
new bartik_menu_tabs_rtl-1994192-3.patch2.58 KB
new bartik_menu_tabs_rtl.jpg43.57 KB

This looks good except for one minor problem - no left padding on the main menu tabs. See attached screenshot and interdiff.txt.

tsi’s picture

tsi commented
StatusFileSize
new bartik_menu_tabs_rtl-1994192-4.patch3.62 KB

The padding there should be (according to the LTR styles) padding: 0.7em 0.8em; and apply only under the min-width: 901px media query, this means we add a line to the RTL styles which looks the same as the LTR equivalent only for cascading reasons (patch attached). not perfect but maybe we have no choice.

Status: Needs review » Needs work

The last submitted patch, bartik_menu_tabs_rtl-1994192-4.patch, failed testing.

mjohnq3’s picture

mjohnq3 commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt672 bytes
new bartik_menu_tabs_rtl-1994192-6.patch2.6 KB

I'll take another crack at this. The affected css is now mobile-first.

tsi’s picture

tsi commented
StatusFileSize
new bartik_menu_tabs_rtl-1994192-7.patch2.59 KB
new bartik_menu_tabs_rtl-1994192-7.patch2.59 KB

There's a tab that snuck in there.
Besides that, seems like #6 does the same as #4 only that it is formatted better (mine got messed up by git) so it is fine by me.
This one removes the tab.

thamas’s picture

thamas
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Reviewed & tested by the community

Tested the last patch, it works well.

dries’s picture

dries commented
Priority: Normal » Minor
Status: Reviewed & tested by the community » Needs work

Committed to 8.x. but I'd like us to clean up the comments. They are not very consistent.

xjm’s picture

xjm
she/her
Primary language English
commented 
+++ b/core/themes/bartik/css/style-rtl.cssundefined
@@ -70,9 +70,11 @@ ul.tips {
 /* --------------- Secondary Menu ------------ */

@@ -249,6 +251,45 @@ ul.tips {
+/* ----------- media queries ------------------------------- */
...
+ /* ------------ Header and Menus -------------------------- */

To clarify @Dries' comment, the new hyphen-header things have more hyphens and the case is not consistent.

mjohnq3’s picture

mjohnq3 commented
Status: Needs work » Needs review
StatusFileSize
new bartik_menu_tabs_rtl-1994192-11.patch4.14 KB

The hyphen-header thingies are now all title case and start at column #24. The ending "/" all end at column #62.

Status: Needs review » Needs work
Issue tags: -frontend, -D8MI

The last submitted patch, bartik_menu_tabs_rtl-1994192-11.patch, failed testing.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs work » Needs review
Issue tags: +frontend, +D8MI

#11: bartik_menu_tabs_rtl-1994192-11.patch queued for re-testing.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Reviewed & tested by the community

Looks fine!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed b3b93ac and pushed to 8.x. Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.