ChromeOS: Update Welcome screen material design OOBE.

chrome/browser/resources/chromeos/login/oobe_buttons.html

Index: chrome/browser/resources/chromeos/login/oobe_buttons.html
diff --git a/chrome/browser/resources/chromeos/login/oobe_buttons.html b/chrome/browser/resources/chromeos/login/oobe_buttons.html
index 2a62637ab4a25ffdfa6ef9042863b09da90b3b96..011900c3eca49be532b86a03ab8cb8e8529c0587 100644
--- a/chrome/browser/resources/chromeos/login/oobe_buttons.html
+++ b/chrome/browser/resources/chromeos/login/oobe_buttons.html
@@ -5,55 +5,51 @@
 <link rel="import" href="chrome://resources/polymer/v1_0/iron-icons/iron-icons.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/paper-button/paper-button.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/paper-fab/paper-fab.html">
-<link rel="import" href="chrome://resources/polymer/v1_0/paper-icon-button/paper-icon-button.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/paper-styles/color.html">
 
 <!--
-  Material design button that shows an icon.
+  Material design square button for text-labelled buttons.
+  By default, text is blue, background is white.
+  |inverse| makes text white on a blue background.
 
   Example:
-    <oobe-icon-button icon="close"></oobe-icon-button>
+    <oobe-icon-button i18n-content="oobeOKButtonText" inverse>
+    </oobe-icon-button>
+    <oobe-icon-button i18n-content="oobeCancelButtonText"></oobe-icon-button>
 
   Attributes:
-    'icon' - a name of icon from material design set to show on button.
     'disabled' - button is disabled when the attribute is set.
-    'aria-label' - accessibility label.
+    'inverse' - makes text white and background blue
 -->
-<dom-module id="oobe-icon-button">
-  <link rel="stylesheet" href="oobe_icon_button.css">
+<dom-module id="oobe-text-button">

[michaelpg, 2016/10/09 21:02:51]

why not just use <paper-button> with a class that you provide styling for? I
don't see this wrapper adding anything, and the cost is clear (complexity, extra
DOM nodes, copying code for focus() and onClick_())

[Alexander Alekseev, 2016/10/11 00:01:53]

The basic idea was to group elements by type and have only one implementation. I
think it is a very polymer-like way to implement UI, isn't it? ;)

I could probably rename it to 'oobe-navigational-button' as it is used as "Next"
button only (but I am going to add some "back" instances as well).

[michaelpg, 2016/10/14 16:45:21]

Not when it comes to wrapping elements for the sake of wrapping; there's nothing
private or special about this implementation.

I'm not sure what you mean by grouping elements and having only one
implementation. oobe-text-button and oobe-welcome-secondary-button both have
implementations that amount to just wrapping the buttons (and why doesn't
oobe-welcome-secondary-button have focus() or onClick overrides?)

[Alexander Alekseev, 2016/10/15 00:23:16]

I believe that it is much better to make design structure clearly visible in
implementation.
If designer calls these two (actually three) buttons as "secondary buttons", we
should have them in implementation as "secondary buttons". HTML templates allow
us to have them as separate objects, which is very nice.

It also simplifies and localizes styles for this element.
I believe this affects mostly accessibility functions and we didn't test it yet.
I think it is better to land changes in reasonably sized changes.

As soon as this is landed, we can update dependent screens and then fix issues
with different screen PPI, keyboard navigation, etc.

[michaelpg, 2016/10/16 22:49:24]

OK. I don't agree when it comes to elements this simple -- this is what CSS
classes are designed for -- but won't press it further (and it's not a huge deal
either way).
SGTM, as long as we understand we can't ship this UI as default until it's
sufficiently accessible.

+  <link rel="stylesheet" href="oobe_text_button.css">
   <template>
-    <div on-click="onClick_" on-tap="onClick_">
-      <paper-icon-button id="iconButton" icon="[[icon]]" disabled="[[disabled]]"
-          aria-label="[[ariaLabel]]">
-      </paper-icon-button>
-    </div>
+    <paper-button id="textButton" on-tap="onClick_" disabled="[[disabled]]"
+        inverse$="[[inverse]]">
+      <content></content>
+    </paper-button>
   </template>
 </dom-module>
 
 <!--
-  Material design square button for text-labelled buttons.
-  By default, text is blue, background is white.
-  |inverse| makes text white on a blue background.
+  Material design button that shows an icon and displays text.
 
   Example:
-    <oobe-icon-button label="OK" inverse></oobe-icon-button>
-    <oobe-icon-button label="CANCEL"></oobe-icon-button>
+    <oobe-welcome-secondary-button icon="close"
+        i18n-content="oobeCloseButtonText">
+    </oobe-icon-button>
 
   Attributes:
-    'label' - text on a button
-    'disabled' - button is disabled when the attribute is set.
-    'inverse' - makes text white and background blue
+    'icon' - a name of icon from material design set to show on button.
+    'aria-label' - accessibility label.
 -->
-<dom-module id="oobe-text-button">
-  <link rel="stylesheet" href="oobe_text_button.css">
+<dom-module id="oobe-welcome-secondary-button">

[michaelpg, 2016/10/09 21:02:51]

Same question. You can have an <iron-icon> in a <paper-button> and style it with
CSS.

+  <link rel="stylesheet" href="oobe_welcome_secondary_button.css">
   <template>
-    <div on-click="onClick_" on-tap="onClick_">
-      <paper-button id="textButton" disabled="[[disabled]]"
-          inverse$="[[inverse]]">
-        [[label]]
-        <content></content>
-      </paper-button>
-    </div>
+    <paper-button id="container" class="layout horizontal center"
+        aria-label="[[ariaLabel]]">
+      <iron-icon icon="[[icon]]" class="oobe-icon"></iron-icon>
+      <content></content>
+    </paper-button>
   </template>
 </dom-module>
-