Print Preview: Move non-title elements out of dialog <slot='title'>

Remove some noise for screen readers in dialogs by moving non-title
elements out of the [slot='title']. Anything in this slot is read
out when the dialog loads. Move these elements to the body instead.

This change requires updating some CSS styling, in order to keep the
lists the only piece of the dialog that scrolls and to keep the
appearance of the dialogs the same.

Bug: 987324
Change-Id: I0ae6f7b338b7df988c12289f7eca2e876bd99a92
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2006216
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732964}

chrome/browser/resources/print_preview/ui/advanced_settings_dialog.html

@@ -5,34 +5,55 @@
     max-width:  calc(100vw - 2 * var(--print-preview-dialog-margin));
   }
 
+  #dialog::part(wrapper) {
+    height: calc(100vh - 2 * var(--print-preview-dialog-margin));
+  }
+
+  #dialog::part(body-container) {
+    flex: 1;
+  }
+
   print-preview-search-box {
     margin-bottom: 8px;
-    margin-top: 24px;
+    margin-top: 16px;
+  }
+
+  cr-dialog [slot=body] {
+    display: flex;
+    flex-direction: column;
+    height: 100%;
   }
 
-  [slot='body'].searching {
+  #itemList {
+    flex: 1;
+    overflow-x: hidden;
+    overflow-y: overlay;
+  }
+
+  #itemList.searching {
     padding-bottom: 20px;
     padding-top: 20px;
   }
 </style>
 <cr-dialog id="dialog" on-close="onCloseOrCancel_">
   <div slot="title">
-    <div>[[i18n('advancedSettingsDialogTitle', destination.displayName)]]
-    </div>
+    [[i18n('advancedSettingsDialogTitle', destination.displayName)]]
+  </div>
+  <div slot="body">
     <print-preview-search-box id="searchBox"
         hidden$="[[!hasMultipleItems_(
             destination.capabilities.printer.vendor_capability)]]"
         label="$i18n{advancedSettingsSearchBoxPlaceholder}"
         search-query="{{searchQuery_}}" autofocus>
     </print-preview-search-box>
-  </div>
-  <div slot="body" class$="[[isSearching_(searchQuery_)]]">
-    <template is="dom-repeat"
-        items="[[destination.capabilities.printer.vendor_capability]]">
-      <print-preview-advanced-settings-item capability="[[item]]"
-          settings="[[settings]]">
-      </print-preview-advanced-settings-item>
-    </template>
+    <div id="itemList" class$="[[isSearching_(searchQuery_)]]">
+      <template is="dom-repeat"
+          items="[[destination.capabilities.printer.vendor_capability]]">
+        <print-preview-advanced-settings-item capability="[[item]]"
+            settings="[[settings]]">
+        </print-preview-advanced-settings-item>
+      </template>
+    </div>
     <div class="no-settings-match-hint"
         hidden$="[[!shouldShowHint_(hasMatching_)]]">
       $i18n{noAdvancedSettingsMatchSearchHint}

chrome/browser/resources/print_preview/ui/destination_dialog.html

@@ -16,25 +16,26 @@
 
   print-preview-search-box {
     margin-bottom: 16px;
-    margin-top: 14px;
+    margin-top: 6px;
   }
 
   .user-info {
-    font-size: calc(13 / 15 * 1em);
     line-height: calc(20 / 15 * 1em);
     margin-bottom: 14px;
-    margin-top: 8px;
   }
 
-  .user-info .account-select-label {
-    padding-inline-end: 18px;
+  .user-info .md-select {
+    width: auto;
   }
 
-  .user-info .account-select {
-    width: auto
+  #accountSelectLabel {
+    color: var(--cr-primary-text-color);
+    padding-inline-end: 18px;
   }
 
   cr-dialog [slot=body] {
+    display: flex;
+    flex-direction: column;
     height: 100%;
   }
 
@@ -113,18 +114,14 @@
   }
 </style>
 <cr-dialog id="dialog" on-close="onCloseOrCancel_">
-  <div slot="title" id="header">
-    <div>$i18n{destinationSearchTitle}</div>
+  <div slot="title" id="header">$i18n{destinationSearchTitle}</div>
+  <div slot="body">
     <div class="user-info" hidden$="[[!activeUser]]" hidden>
-      <label class="account-select-label" id="accountSelectLabel">
-        $i18n{accountSelectTitle}
-      </label>
-      <select class="md-select account-select"
+      <label id="accountSelectLabel">$i18n{accountSelectTitle}</label>
+      <select class="md-select"
           aria-labelledby="accountSelectLabel" on-change="onUserChange_">
         <template is="dom-repeat" items="[[users]]">
-          <option value="[[item]]">
-            [[item]]
-          </option>
+          <option value="[[item]]">[[item]]</option>
         </template>
         <option value="">$i18n{addAccountTitle}</option>
       </select>
@@ -133,8 +130,6 @@
         label="$i18n{searchBoxPlaceholder}" search-query="{{searchQuery_}}"
         autofocus>
     </print-preview-search-box>
-  </div>
-  <div slot="body">
     <print-preview-destination-list id="printList"
         destinations="[[destinations_]]"
         loading-destinations="[[loadingDestinations_]]"

chrome/browser/resources/print_preview/ui/destination_list.html

@@ -8,7 +8,7 @@
   }
 
   #list {
-    max-height: 100%;
+    min-height: var(--destination-item-height);
   }
 
   .throbber-container {
@@ -24,7 +24,6 @@
   }
 
   .no-destinations-message {
-    color: #999;
     padding-bottom: 8px;
     padding-inline-start: 18px;
     padding-top: 8px;
@@ -52,7 +51,8 @@
 <div class="no-destinations-message" hidden$="[[hasDestinations_]]">
   $i18n{noDestinationsMessage}
 </div>
-<iron-list id="list" items="[[matchingDestinations_]]">
+<iron-list id="list" items="[[matchingDestinations_]]"
+    hidden$="[[hideList_]]">
   <template>
     <print-preview-destination-list-item class="list-item"
         search-query="[[searchQuery]]" destination="[[item]]"

chrome/browser/resources/print_preview/ui/destination_list.js

@@ -54,6 +54,12 @@ Polymer({
       type: Boolean,
       value: false,
     },
+
+    /** @private */
+    hideList_: {
+      type: Boolean,
+      value: false,
+    },
   },
 
   observers: [
@@ -100,14 +106,19 @@ Polymer({
 
     const maxDisplayedItems = this.offsetHeight / DESTINATION_ITEM_HEIGHT;
     const isListFullHeight = maxDisplayedItems <= count;
-    const listHeight =
-        isListFullHeight ? this.offsetHeight : count * DESTINATION_ITEM_HEIGHT;
 
     // Update the throbber and "No destinations" message.
     this.hasDestinations_ = count > 0 || this.loadingDestinations;
     this.throbberHidden_ =
         !this.loadingDestinations || isListFullHeight || !this.hasDestinations_;
 
+    this.hideList_ = count === 0;
+    if (this.hideList_) {
+      return;
+    }
+
+    const listHeight =
+        isListFullHeight ? this.offsetHeight : count * DESTINATION_ITEM_HEIGHT;
     this.$.list.style.height = listHeight > DESTINATION_ITEM_HEIGHT ?
         `${listHeight}px` :
         `${DESTINATION_ITEM_HEIGHT}px`;

chrome/browser/resources/print_preview/ui/print_preview_search_box.html

 <style include="print-preview-shared cr-input-style">
   :host {
     display: flex;
-    font-size: calc(13/15 * 1em);
     --cr-input-error-display: none;
   }
 

chrome/browser/resources/print_preview/ui/print_preview_shared_css.html

@@ -64,7 +64,7 @@
     }
 
     #dialog div[slot='title'] {
-      padding-bottom: 0;
+      padding-bottom: 8px;
     }
 
     #dialog div[slot='button-container'] {

chrome/test/data/webui/print_preview/destination_list_test.js

@@ -62,15 +62,18 @@ suite(destination_list_test.suiteName, function() {
     const items =
         list.shadowRoot.querySelectorAll('print-preview-destination-list-item');
     const noMatchHint = list.$$('.no-destinations-message');
+    const ironList = list.$.list;
 
     // Query is initialized to null. All items are shown and the hint is
     // hidden.
+    assertFalse(ironList.hidden);
     items.forEach(item => assertFalse(item.hidden));
     assertTrue(noMatchHint.hidden);
 
     // Searching for "e" should show "One", "Three", and "Five".
     list.searchQuery = /(e)/ig;
     flush();
+    assertFalse(ironList.hidden);
     assertEquals(undefined, Array.from(items).find(item => {
       return !item.hidden &&
           (item.destination.displayName == 'Two' ||
@@ -81,6 +84,7 @@ suite(destination_list_test.suiteName, function() {
     // Searching for "ABC" should show "One" and "Three".
     list.searchQuery = /(ABC)/ig;
     flush();
+    assertFalse(ironList.hidden);
     assertEquals(undefined, Array.from(items).find(item => {
       return !item.hidden && item.destination.displayName != 'One' &&
           item.destination.displayName != 'Three';
@@ -90,6 +94,7 @@ suite(destination_list_test.suiteName, function() {
     // Searching for "F" should show "Four" and "Five"
     list.searchQuery = /(F)/ig;
     flush();
+    assertFalse(ironList.hidden);
     assertEquals(undefined, Array.from(items).find(item => {
       return !item.hidden && item.destination.displayName != 'Four' &&
           item.destination.displayName != 'Five';
@@ -100,12 +105,13 @@ suite(destination_list_test.suiteName, function() {
     // match" hint.
     list.searchQuery = /(UVW)/ig;
     flush();
-    items.forEach(item => assertTrue(item.hidden));
+    assertTrue(ironList.hidden);
     assertFalse(noMatchHint.hidden);
 
     // Searching for 123 should show destinations "Three", "Four", and "Five".
     list.searchQuery = /(123)/ig;
     flush();
+    assertFalse(ironList.hidden);
     assertEquals(undefined, Array.from(items).find(item => {
       return !item.hidden &&
           (item.destination.displayName == 'One' ||
@@ -116,6 +122,7 @@ suite(destination_list_test.suiteName, function() {
     // Clearing the query restores the original state.
     list.searchQuery = null;
     flush();
+    assertFalse(ironList.hidden);
     items.forEach(item => assertFalse(item.hidden));
     assertTrue(noMatchHint.hidden);
   });