Thanks for the review guys! @imalabya nice work on the improvements!
@gábor:
1. An all around 0 margin is not surprisingly not different in RTL either and therefore should not be marked as LTR and not have an RTL counterpart with the same style :)
The addition of the margin: 0; in RTL was because the base theme (Classy) was adding margins to tabs specifically for RTL and hence the need override that in Umami.
core/themes/classy/css/components/tabs.css:18
[dir="rtl"] .tabs > li {
margin-left: 0.3em;
margin-right: 0;
}
2. How did 14px became 1rem in the RTL version?
That was changed to keep the Search Result title and result item boxes aligned on the right. See below:
3. Why is RTL getting a full border radius while LTR only has selective?
As you can see form the LTR hover/focus state of the search button, all corners have a radius. I believe that is inherited from core/profiles/demo_umami/themes/umami/css/base.css:65. The reason that I'm applying the radius again is because for RTL we also have to switch the corners that are square and rounder; meaning a more specific styling that overrides the default one in base.css. Then on hover, the RTL specific one needs to be overridden to apply border radius for all corners. :D
Hm, if this was normally in the center, why does it go to the right then?
The RTL rule is actually overriding the 0 part (equivalent to left background-position: left;) in the LTR. The center is for the vertical position which is shared by both.
Why was this not applied to LTR? Does it harm something there? What's the benefit in RTL?
For the most part I worked on the RTL version of things, totally ignoring LTR. In this case, in display: inline-block; is needed to fix the "swapping" issue on pages that have a third-level breadcrumb. See below: