Icons in toolbar are slightly misaligned

Not sure if this is by design, assigning to jesse

This patch still applies. I would be almost certain that this is not by design and the patch resolves a bug. The equal spacing between the top and right of the icon looks a lot better than what is currently there.

Setting to RTBC unless jessebeach wants to chime in and say otherwise.

Looks like this is more accurately "needs review." I've left Jesse a tell on IRC as well.

I like the end result of the changes. I just have a few questions about the code.

1. +++ b/core/modules/toolbar/css/toolbar.icons.css
   @@ -283,19 +283,21 @@
    [dir="rtl"] .toolbar .toolbar-icon.toolbar-handle:before {
   -  left: auto;
   -  right: 0;
   +  left: 0;
    }
    [dir="rtl"].no-svg .toolbar .toolbar-icon.toolbar-handle:before {
   -  left: auto;
   -  right: 0;
   +  left: 0;
   

   I don't believe this left: 0 is necessary. Can you verify that?

2. +++ b/core/modules/toolbar/css/toolbar.icons.css
   @@ -283,19 +283,21 @@
   +}
   +[dir="rtl"] .toolbar .toolbar-icon.toolbar-handle {
   +  padding-right: 0;
   +}
   +[dir="rtl"].no-svg .toolbar .toolbar-icon.toolbar-handle {
   +  padding-right: 0;
   

   What padding is this removing? Do you have a theme other than Bartik applied while you're developing? Just trying to determine why you would be zeroing out padding here.

This has been stressing out the OCD portion of my brain for ages. So, I arrived at the same end with the attached patch.

I'm not sure what the additional CSS in the previous patch was for. The attached patch was manually tested under all combinations of LTR, RLT & no-svg.

Looks good on Chrome and Firefox on OSX both LTR and RLT.

RTBC++

+++ b/core/modules/toolbar/css/toolbar.icons.css
@@ -269,19 +269,15 @@
 [dir="rtl"] .toolbar .toolbar-icon.toolbar-handle:before {
   left: auto;
-  right: 0;
 }
 [dir="rtl"].no-svg .toolbar .toolbar-icon.toolbar-handle:before {
   left: auto;
-  right: 0;
 }

Sorry to be a nudge about this :)

If we remove the left:0 property from the LTR declarations, then we don't need to reset it in the RTL declarations. So, it seems like the RTL declarations here aren't even needed. And any time we can remove CSS, let's do it.

Good catch! Should have seen that, thanks for the review. Another patch is attached. Can confirm things are working LTR and RTL.

Great, thank you for sticking with this issue Sam152.

The updated patch realigns the icons nicely. I did some cross browser testing to verify no visual regressions.

Committed 08c18e1 and pushed to 8.x. Thanks!