Focus style for Add supervised user button

chrome/browser/resources/chromeos/login/header_bar.css

 /* Copyright (c) 2012 The Chromium Authors. All rights reserved.
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file. */
 
 #login-header-bar {
   -webkit-padding-start: 15px;
   bottom: 0;
   left: 0;
   padding-bottom: 6px;
   position: absolute;
@@ skipping 48 matching lines @@
 }
 
 .add-supervised-user-menu {
   display: none;
 }
 
 #more-settings-header-bar-item.active button.add-supervised-user-menu {
   background-color: white;
   border: 1px solid lightgray;
   border-radius: 2px;
-  bottom: 15px;
+  bottom: 10px;
   color: black !important;

[Evan Stade, 2017/01/04 23:43:17]

side note: we try hard to avoid !important

[Greg Levin, 2017/01/05 18:25:54]

Acknowledged (no idea why this was marked important)

   display: block;
   font-size: 13px;
   height: auto;
   left: 15px;
   margin: 0 0 -10px -10px;

[Greg Levin, 2017/01/04 21:31:25]

I'm still learning the subtleties of css.  This appears to have the same effect
as

  bottom: 0
  left: 5px;
  margin: 0 0 0 0;

What's the merit of the way that it's currently written?

[Evan Stade, 2017/01/04 23:43:17]

do you not want bottom:5px? Why is this button moving down by 5px?
CSS is a house of cards because of cascading rules. Did you check what this
looks like in RTL? Looking at code history can also help shed light, but the
change where this is added[1] isn't that helpful. Since this mysterious pattern
was applied without much commentary (either in code or in rietveld), I'd just
assume it was an oversight.

If your change seems to do the right thing (and do you even need the margin
rule?) and you're feeling lucky, take the chance.

[1] https://codereview.chromium.org/1013613002

[Greg Levin, 2017/01/05 18:25:54]

Requested in issue comment:
https://bugs.chromium.org/p/chromium/issues/detail?id=642330#c5
Also see comments 6 & 7.  I don't know why this was originally offset 5px up and
right, but no one seems to object to it being directly over the [...] button
instead.
All these offsets and margins seem to be for moving the box +5px up and right. 
Without that, they seem unnecessary (except "bottom: 0", without which the box
sinks half-way off the screen; no idea why).  Also works fine RTL.

   min-height: 34px;
   position: absolute;
   text-align: center;
   width: 180px;
 }
 
+#more-settings-header-bar-item.active button.add-supervised-user-menu:focus {
+  border: 2px solid var(--google-blue-500);
+  border-radius: 2px;
+}
+
 html[dir=rtl] #more-settings-header-bar-item.active
                 button.add-supervised-user-menu {
   right: 0;

[Evan Stade, 2017/01/04 23:43:17]

I believe you could just move this rule into the non-rtl block because if both
left and right are specified, the one that wins out depends on text direction
(indeed that's why this works, as we aren't resetting left in this block).

[Greg Levin, 2017/01/05 18:25:54]

Turns out this rule was only needed because "left: 15px" was putting the RTL box
in the wrong place.  Without that line, the RTL version of the box is in the
right place by default.

 }
 
 html[dir=rtl] .header-bar-item {
   background-position: right center;
 }
 
 #login-header-bar #shutdown-button-text,
 #login-header-bar #restart-button-text,
 #login-header-bar #add-user-button-text,
 #login-header-bar #guest-user-button-text,
@@ skipping 63 matching lines @@
   background-image: -webkit-image-set(
       url(images/1x/signout.svg) 1x,
       url(images/2x/signout.svg) 2x );
 }
 
 .button-restricted {
   border: 1px solid transparent;
   color: white !important;
   opacity: 0.4 !important;
 }