MD Settings: adjust dialog style and scroll border visual

BUG=707631
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2795763003
Cr-Commit-Position: refs/heads/master@{#463373}

chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_dialog.html

@@ -18,11 +18,6 @@
         visibility: hidden;
       }
 
-      #dialog .button-container {
-        margin-top: 0;
-        padding-top: 10px;
-      }
-
       #dialog .footer {
         color: var(--paper-grey-600);
       }
@@ -94,11 +89,6 @@
             max-height: 280px; 
           };
         }
-
-        #dialog .body {
-          margin-bottom: 0;
-          padding-bottom: 12px;
-        }
       }
     </style>
 

chrome/browser/resources/settings/clear_browsing_data_dialog/history_deletion_dialog.html

@@ -4,11 +4,7 @@
 
 <dom-module id="settings-history-deletion-dialog">
   <template>
-    <style include="settings-shared">
-      #dialog .body {
-        margin-bottom: 50px;
-      }
-    </style>
+    <style include="settings-shared"></style>
 
     <dialog is="cr-dialog" id="dialog" close-text="$i18n{close}">
       <div class="title">$i18n{historyDeletionDialogTitle}</div>

chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.html

@@ -41,8 +41,10 @@
         width: 15px;
       }
 
-      #manuallyAddPrinterButton {
-        -webkit-margin-end: 210px;
+      .dialog-buttons {
+        display: flex;
+        justify-content: space-between;
+        width: 100%;
       }
     </style>
     <add-printer-dialog>
@@ -60,17 +62,23 @@
         </div>
       </div>
       <div class="dialog-buttons">
-        <paper-button id="manuallyAddPrinterButton"
-            on-tap="switchToManualAddDialog_">
-          $i18n{manuallyAddPrinterButtonText}
-        </paper-button>
-        <paper-button class="cancel-button" on-tap="onCancelTap_">
-          $i18n{cancelButtonText}
-        </paper-button>
-        <paper-button class="action-button" id="addPrinterButton"
-            disabled="[[!selectedPrinter]]" on-tap="switchToConfiguringDialog_">
-          $i18n{addPrinterButtonText}
-        </paper-button>
+        <div>  <!-- Left group -->
+          <paper-button id="manuallyAddPrinterButton" class="secondary-button"
+              on-tap="switchToManualAddDialog_">
+            $i18n{manuallyAddPrinterButtonText}
+          </paper-button>
+        </div>
+        <div>  <!-- Right group -->
+          <paper-button class="cancel-button secondary-button"
+              on-tap="onCancelTap_">
+            $i18n{cancelButtonText}
+          </paper-button>
+          <paper-button class="action-button" id="addPrinterButton"
+              disabled="[[!selectedPrinter]]"
+              on-tap="switchToConfiguringDialog_">
+            $i18n{addPrinterButtonText}
+          </paper-button>
+        </div>
       </div>
     </add-printer-dialog>
   </template>
@@ -201,7 +209,8 @@
         </div>
       </div>
       <div class="dialog-buttons">
-        <paper-button class="cancel-button" on-tap="onCancelTap_">
+        <paper-button class="cancel-button secondary-button"
+            on-tap="onCancelTap_">
           $i18n{cancelButtonText}
         </paper-button>
         <paper-button class="action-button" on-tap="switchToManufacturerDialog_"
@@ -251,8 +260,10 @@
         padding: 10px 20px;
       }
 
-      #manuallyAddPrinterButton {
-        -webkit-margin-end: 210px;
+      .dialog-buttons {
+        display: flex;
+        justify-content: space-between;
+        width: 100%;
       }
 
       #browseButton {
@@ -303,17 +314,22 @@
         </div>
       </div>
       <div class="dialog-buttons">
-        <paper-button id="manuallyAddPrinterButton"
-            on-tap="switchToManualAddDialog_">
-          $i18n{manuallyAddPrinterButtonText}
-        </paper-button>
-        <paper-button class="cancel-button" on-tap="onCancelTap_">
-          $i18n{cancelButtonText}
-        </paper-button>
-        <paper-button class="action-button" id="addPrinterButton"
-            on-tap="switchToConfiguringDialog_">
-          $i18n{addPrinterButtonText}
-        </paper-button>
+        <div>  <!-- Left group -->
+          <paper-button id="manuallyAddPrinterButton" class="secondary-button"
+              on-tap="switchToManualAddDialog_">
+            $i18n{manuallyAddPrinterButtonText}
+          </paper-button>
+        </div>
+        <div>  <!-- Right group -->
+          <paper-button class="cancel-button secondary-button"
+              on-tap="onCancelTap_">
+            $i18n{cancelButtonText}
+          </paper-button>
+          <paper-button class="action-button" id="addPrinterButton"
+              on-tap="switchToConfiguringDialog_">
+            $i18n{addPrinterButtonText}
+          </paper-button>
+        </div>
       </div>
     </add-printer-dialog>
   </template>
@@ -334,7 +350,8 @@
         <div id="configuringMessage">$i18n{printerConfiguringMessage}</div>
       </div>
       <div class="dialog-buttons">
-        <paper-button class="cancel-button" on-tap="onCancelConfiguringTap_">
+        <paper-button class="cancel-button secondary-button"
+            on-tap="onCancelConfiguringTap_">
           $i18n{cancelButtonText}
         </paper-button>
       </div>

chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.html

@@ -106,14 +106,7 @@
       }
 
       #dialog .button-container {
-        border-top: 1px solid rgba(0, 0, 0, 0.14);
-        margin-top: 0;
-        padding-top: 10px;
-      }
-
-      #dialog .button-container ::content paper-button {
-        font-size: 107.69%;  /* 14px / 13px */
-        font-weight: bold;
+        border-top: 1px solid var(--paper-grey-300);
       }
     </style>
 

chrome/test/data/webui/cr_elements/cr_dialog_test.js

@@ -156,19 +156,35 @@ suite('cr-dialog', function() {
     var bodyContainer = dialog.$$('.body-container');
     assertTrue(!!bodyContainer);
 
+    dialog.showModal();  // Attach the dialog for the first time here.
+
     var observerCount = 0;
 
     // Needs to setup the observer before attaching, since InteractionObserver
     // calls callback before MutationObserver does.
-    var observer = new MutationObserver(function() {
+    var observer = new MutationObserver(function(changes) {
+      // Only care about class mutations.
+      if (changes[0].attributeName != 'class')
+        return;
+
       observerCount++;
-      if (observerCount == 1) {  // Triggered when scrolled to bottom.
-        assertFalse(bodyContainer.classList.contains('bottom-scrollable'));
-        bodyContainer.scrollTop = 0;
-      } else if (observerCount == 2) {  // Triggered when scrolled back to top.
-        assertTrue(bodyContainer.classList.contains('bottom-scrollable'));
-        observer.disconnect();
-        done();
+      switch (observerCount) {
+        case 1:  // Triggered when scrolled to bottom.
+          assertFalse(bodyContainer.classList.contains('bottom-scrollable'));
+          assertTrue(bodyContainer.classList.contains('top-scrollable'));
+          bodyContainer.scrollTop = 0;
+          break;
+        case 2:  // Triggered when scrolled back to top.
+          assertTrue(bodyContainer.classList.contains('bottom-scrollable'));
+          assertFalse(bodyContainer.classList.contains('top-scrollable'));
+          bodyContainer.scrollTop = 2;
+          break;
+        case 3:  // Triggered when finally scrolling to middle.
+          assertTrue(bodyContainer.classList.contains('bottom-scrollable'));
+          assertTrue(bodyContainer.classList.contains('top-scrollable'));
+          observer.disconnect();
+          done();
+          break;
       }
     });
     observer.observe(bodyContainer, {attributes: true});
@@ -176,6 +192,5 @@ suite('cr-dialog', function() {
     // Height is normally set via CSS, but mixin doesn't work with innerHTML.
     bodyContainer.style.height = '1px';
     bodyContainer.scrollTop = 100;
-    dialog.showModal();  // Attach the dialog for the first time here.
   });
 });

ui/webui/resources/cr_elements/cr_dialog/cr_dialog.html

@@ -8,6 +8,7 @@
   <template>
     <style>
       :host {
+        --scroll-border: 1px solid var(--paper-grey-300);
         border: 0;
         border-radius: 2px;
         bottom: 0;
@@ -28,69 +29,35 @@
         top: 0;
       }
 
-      .top-container {
-        align-items: center;
-        border-bottom: 1px solid var(--divider-color);
-        display: flex;
-        min-height: 52px;
-      }
-
-      .title-container {
-        display: flex;
-        flex: 1;
-        outline: none;
-      }
-
-      :host ::content .title {
-        font-size: 123.07%; /* (16px / 13px) * 100 */
-      }
-
-      #close {
-        --paper-icon-button: {
-          height: 40px;
-          width: 40px;
-        };
-        -webkit-margin-end: 6px;
-        /* <paper-icon-button> overrides --iron-icon-{height,width}, so this
-         * padding essentially reduces 40x40 to 20x20. */
-        padding: 10px;
-      }
-
-      .body-container {
-        display: flex;
-        flex-direction: column;
-        overflow: auto;
-        @apply(--cr-dialog-body-container);
-      }
-
       :host([show-scroll-borders]) .body-container {
         /* Prevent layout moving when border does appear. */
         border-bottom: 1px solid transparent;
+        border-top: 1px solid transparent;
       }
 
       :host([show-scroll-borders]) .body-container.bottom-scrollable {
-        border-bottom: 1px solid var(--paper-grey-300);
+        border-bottom: var(--scroll-border);
+      }
+
+      :host([show-scroll-borders]) .body-container.top-scrollable {
+        border-top: var(--scroll-border);
       }
 
       :host ::content .body {
-        padding-bottom: 12px;
-        padding-top: 12px;
+        padding: 12px 16px;
       }
 
-      :host ::content .body,
       :host ::content .title {
-        -webkit-padding-end: 24px;
-        -webkit-padding-start: 24px;
+        font-size: 115.38%;  /* (15px / 13px) * 100 */
+        line-height: 1;
+        padding: 16px 16px;
         flex: 1;
       }
 
       :host ::content .button-container {
-        -webkit-padding-end: 16px;
-        -webkit-padding-start: 16px;
+        padding: 16px 16px;
         display: flex;
         justify-content: flex-end;
-        margin-bottom: 12px;
-        margin-top: 12px;
       }
 
       :host ::content .button-container .cancel-button {
@@ -108,6 +75,39 @@
       :host ::content .border-top-divider {
         border-top: 1px solid var(--divider-color);
       }
+
+      .body-container {
+        display: flex;
+        flex-direction: column;
+        overflow: auto;
+        @apply(--cr-dialog-body-container);
+      }
+
+      .top-container {
+        align-items: center;
+        display: flex;
+        min-height: 47px;
+      }
+
+      .title-container {
+        display: flex;
+        flex: 1;
+        outline: none;
+      }
+
+      #close {
+        --paper-icon-button: {
+          height: 36px;
+          width: 36px;
+        };
+
+        --layout-inline: {
+          display: flex;
+        };
+
+        -webkit-margin-end: 4px;
+        padding: 10px;  /* Makes the actual icon 16x16. */
+      }
     </style>
     <div class="top-container">
       <div class="title-container" tabindex="-1">
@@ -118,6 +118,7 @@
       </paper-icon-button>
     </div>
     <div class="body-container">
+      <span id="bodyTopMarker"></span>
       <content select=".body"></content>
       <span id="bodyBottomMarker"></span>
     </div>

ui/webui/resources/cr_elements/cr_dialog/cr_dialog.js

@@ -66,10 +66,21 @@ Polymer({
     if (this.showScrollBorders) {
       var bodyContainer = this.$$('.body-container');
 
+      var bottomMarker = this.$.bodyBottomMarker;
+      var topMarker = this.$.bodyTopMarker;
+
       var callback = function(entries) {
-        assert(entries.length == 1);
-        bodyContainer.classList.toggle(
-            'bottom-scrollable', entries[0].intersectionRatio == 0);
+        assert(entries.length <= 2);
+        for (var i = 0; i < entries.length; i++) {
+          var target = entries[i].target;
+          assert(target == bottomMarker || target == topMarker);
+
+          var classToToggle =
+              target == bottomMarker ? 'bottom-scrollable' : 'top-scrollable';
+
+          bodyContainer.classList.toggle(
+              classToToggle, entries[i].intersectionRatio == 0);
+        }
       };
 
       this.intersectionObserver_ = new IntersectionObserver(
@@ -78,7 +89,8 @@ Polymer({
             root: bodyContainer,
             threshold: 0,
           }));
-      this.intersectionObserver_.observe(this.$.bodyBottomMarker);
+      this.intersectionObserver_.observe(bottomMarker);
+      this.intersectionObserver_.observe(topMarker);
     }
   },