CSS has unnecessary duplicated rules with RTL selector.

Hello everyone,
There seem to be an issue with RTL css.
I'm looking at the file admin.toolbar.css in version 3.1.0
The RTL rules are mostly exactly the same rules as without the the RTL selector and they are just causing stronger rules overrides:
For example:

.toolbar-tray-horizontal .menu-item--expanded {
  background-color: #f5f5f2;
}

[dir="rtl"] .toolbar-tray-horizontal .menu-item--expanded {
  background-color: #f5f5f2;
}

The language direction is normally irrelevant to the background color of a menu-item and the direction selector makes it stronger than other custom or contrib css that use the .toolbar-tray-horizontal .menu-item--expanded selector.
So everything might look well in LTR but when switching to RTL we suddenly see these stronger rule overrides.
And to overcome this custom and conrib css also neet to duplicate their code to achieve a stronger selector.

The example above is just a simple example, In Gin these duplicated rules broke the whole hover functionality in RTL (while in LTR is was working well).

Proposed resolution

Use RTL selector only on directional style rules.
From a fast look around it seems like all the selectors with RTL can be removed and we just need to add these two rules to keep the css file essentially the same:

[dir="rtl"] .toolbar-tray-horizontal ul li.menu-item--expanded ul li.menu-item--expanded {
  background-position: center left;
  background-image: url(../misc/icons/0074bd/chevron-left.svg);
}
[dir="rtl"] .toolbar-tray-horizontal ul li.menu-item--expanded .menu-item.hover-intent ul {
  margin: -40px 197px 0 0;
}
CommentFileSizeAuthor
#4 3273375-4.patch3.32 KBtinto

Comments

Yonka created an issue. See original summary.

romainj’s picture

Status: Active » Needs work
tinto’s picture

Version: 3.1.0 » 3.x-dev

I've tested the proposed resolution in Drupal 9.5.x with Admin Toolbar 3.x-dev, but it messed up the position of the submenu items underneath expanded parent items quite a bit.

Further testing revealed that - in addition to the css mentioned above - these lines are needed too:

[dir="rtl"] .toolbar-tray-horizontal li:hover ul li {
  float: none;
}

[dir="rtl"] .toolbar-tray-horizontal li.hover-intent ul li {
  float: none;
}

Other than that, I agree with the OP that a lot of RTL-specific CSS could be considered obsolete, especially if it causes problems for (sub)themers down the line.

tinto’s picture

StatusFileSize
new3.32 KB

Here's a patch.

tinto’s picture

Status: Needs work » Needs review
malphas’s picture

Status: Needs review » Reviewed & tested by the community

I installed the patch and tested it (LTR + RTL) and it works fine, the float:none rules you added are indeed necessary for the toolbar to work properly in RTL.

  • 67a3ff5 committed on 3.x
    Issue #3273375 by tinto, Yonka, romainj, malphas: CSS has unnecessary...
romainj’s picture

Patch commited to the latest 3.x dev branch. THANKS

romainj’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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

romainj’s picture

Status: Closed (fixed) » Fixed

Status: Fixed » Closed (fixed)

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