Print Preview Componentization: Destinations Dialog Fixes

- Make dialog fixed height
- Remove show all button
- Improve highlighting speed
- Do not scroll search box
- Fix problem with footer disappearing

Bug: 773928
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I4d19b7fa4b2e8246eb34e8c60d10382688e6a747
Reviewed-on: https://chromium-review.googlesource.com/898347
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534232}

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

@@ -30,13 +30,21 @@
         }
         --cr-icon-ripple-size: 0;
         --cr-icon-size: 14px;
-
+        --cr-dialog-body: {
+          box-sizing: border-box;
+          padding-top: 0;
+        }
         box-shadow: 0 4px 23px 5px rgba(0, 0, 0, 0.2),
           0 2px 6px rgba(0, 0, 0, 0.15);
-        max-height: calc(100% - 40px);
+        height: calc(100% - 40px);
         width: 640px;
       }
 
+      :host #searchBox {
+        font-size: calc(13/15 * 1em);
+        margin-top: 14px;
+      }
+
       :host #dialog .cloudprint-promo {
         align-items: center;
         background-color: #f5f5f5;
@@ -61,13 +69,13 @@
     </style>
     <dialog is="cr-dialog" id="dialog" on-cancel="onCloseOrCancel_"
         on-close="onCloseOrCancel_">
-      <div slot="title">
+      <div slot="title" id="titleBox">
         <span>$i18n{destinationSearchTitle}</span>
-      </div>
-      <div slot="body" scrollable>
         <print-preview-search-box id="searchBox"
             label="$i18n{searchBoxPlaceholder}" search-query="{{searchQuery_}}">
         </print-preview-search-box>
+      </div>
+      <div slot="body" scrollable id="lists">
         <print-preview-destination-list
             destinations="[[recentDestinationList_]]"
             search-query="[[searchQuery_]]"
@@ -81,7 +89,7 @@
             on-destination-selected="onDestinationSelected_">
         </print-preview-destination-list>
       </div>
-      <div slot="button-container">
+      <div slot="button-container" id="buttons">
         <button class="cancel-button" on-tap="onCancelButtonTap_">
           $i18n{cancel}
         </button>

chrome/browser/resources/print_preview/new/destination_dialog.js

@@ -62,6 +62,7 @@ Polymer({
         destinationStore,
         print_preview.DestinationStore.EventType.DESTINATION_SEARCH_DONE,
         this.updateDestinations_.bind(this));
+    this.tracker_.add(window, 'resize', this.updateHeight_.bind(this));
   },
 
   /** @private */
@@ -105,8 +106,6 @@ Polymer({
   onCloseOrCancel_: function() {
     if (this.searchQuery_)
       this.$.searchBox.setValue('');
-    this.shadowRoot.querySelectorAll('print-preview-destination-list')
-        .forEach(list => list.reset());
   },
 
   /** @private */
@@ -124,9 +123,20 @@ Polymer({
     this.$.dialog.close();
   },
 
+  /** @private */
+  updateHeight_: function() {
+    const heightNum = this.$.dialog.getBoundingClientRect().height -
+        this.$$('.cloudprint-promo').offsetHeight -
+        this.$.buttons.offsetHeight - this.$.titleBox.offsetHeight;
+
+    this.$.lists.style.height = `${heightNum}px`;
+    this.$.lists.style.minHeight = `${heightNum}px`;
+  },
+
   show: function() {
     this.loadingDestinations_ =
         this.destinationStore.isPrintDestinationSearchInProgress;
     this.$.dialog.showModal();
+    this.updateHeight_();
   },
 });

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

@@ -25,17 +25,13 @@
         padding-bottom: 8px;
       }
 
-      :host .title {
+      :host :-webkit-any(.title, .action-link, .total) {
         -webkit-padding-end: 8px;
         -webkit-padding-start: 4px;
         display: inline;
         vertical-align: middle;
       }
 
-      :host .action-link {
-        vertical-align: middle;
-      }
-
       :host .throbber-container {
         -webkit-padding-end: 16px;
         -webkit-padding-start: 8px;
@@ -55,15 +51,6 @@
         padding-top: 8px;
       }
 
-      :host .total {
-        color: #777;
-      }
-
-      :host > footer {
-        -webkit-padding-start: 18px;
-        padding-top: 6px;
-      }
-
       :host .list-item {
         -webkit-padding-end: 2px;
         -webkit-padding-start: 18px;
@@ -88,6 +75,9 @@
     </style>
     <header>
       <h4 class="title">[[title]]</h4>
+      <span class="total" hidden$="[[!showDestinationsTotal_]]">
+        [[i18n('destinationCount', matchingDestinationsCount_)]]
+      </span>
       <a is="action-link" class="action-link" hidden$="[[!hasActionLink]]"
           on-tap="onActionLinkTap_">
         $i18n{manage}
@@ -96,23 +86,15 @@
         <div class="throbber"></div>
       </div>
     </header>
-    <template is="dom-repeat" items="[[destinations]]" notify-dom-change
-        filter="[[computeFilter_(searchQuery, showAll_, destinations)]]">
+    <template is="dom-repeat" items="[[destinations]]">
       <print-preview-destination-list-item class="list-item"
-          on-tap="onDestinationSelected_" destination="[[item]]">
+          search-query="[[searchQuery]]" destination="[[item]]"
+          on-tap="onDestinationSelected_">
       </print-preview-destination-list-item>
     </template>
     <div class="no-destinations-message" hidden$="[[hasDestinations_]]">
       $i18n{noDestinationsMessage}
     </div>
-    <footer hidden$="[[footerHidden_]]">
-      <button class="show-all-button" on-tap="onShowAllTap_">
-        $i18n{showAllButtonText}
-      </button>
-      <span class="total">
-        [[i18n('destinationCount', matchingDestinationsCount_)]]
-      </span>
-    </footer>
   </template>
   <script src="destination_list.js"></script>
 </dom-module>

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

@@ -5,11 +5,6 @@
 (function() {
 'use strict';
 
-// TODO (rbpotter): Adjust the short list size based on the dialog height
-// instead of always using this constant.
-/** @type {number} */
-const SHORT_DESTINATION_LIST_SIZE = 4;
-
 Polymer({
   is: 'print-preview-destination-list',
 
@@ -17,7 +12,10 @@ Polymer({
 
   properties: {
     /** @type {Array<!print_preview.Destination>} */
-    destinations: Array,
+    destinations: {
+      type: Array,
+      observer: 'destinationsChanged_',
+    },
 
     /** @type {boolean} */
     hasActionLink: {
@@ -32,27 +30,18 @@ Polymer({
     },
 
     /** @type {?RegExp} */
-    searchQuery: Object,
+    searchQuery: {
+      type: Object,
+      observer: 'update_',
+    },
 
     /** @type {boolean} */
     title: String,
 
-    /** @private {boolean} */
-    showAll_: {
-      type: Boolean,
-      value: false,
-    },
-
     /** @private {number} */
     matchingDestinationsCount_: {
       type: Number,
-      computed: 'computeMatchingDestinationsCount_(destinations, searchQuery)',
-    },
-
-    /** @type {boolean} */
-    footerHidden_: {
-      type: Boolean,
-      computed: 'computeFooterHidden_(matchingDestinationsCount_, showAll_)',
+      value: 0,
     },
 
     /** @private {boolean} */
@@ -60,65 +49,63 @@ Polymer({
       type: Boolean,
       computed: 'computeHasDestinations_(matchingDestinationsCount_)',
     },
-  },
-
-  listeners: {
-    'dom-change': 'updateListItems_',
-  },
-
-  /** @private {number} */
-  shownCount_: 0,
 
-  /** @private */
-  updateListItems_: function() {
-    this.shadowRoot
-        .querySelectorAll('print-preview-destination-list-item:not([hidden])')
-        .forEach(item => item.update(this.searchQuery));
+    /** @private {boolean} */
+    showDestinationsTotal_: {
+      type: Boolean,
+      computed: 'computeShowDestinationsTotal_(matchingDestinationsCount_)',
+    },
   },
 
   /**
-   * @return {Function}
+   * @param {!Array<!print_preview.Destination>} current
+   * @param {?Array<!print_preview.Destination>} previous
    * @private
    */
-  computeFilter_: function() {
-    this.shownCount_ = 0;
-    return destination => {
-      const isShown =
-          (!this.searchQuery || destination.matches(this.searchQuery)) &&
-          (this.shownCount_ < SHORT_DESTINATION_LIST_SIZE || this.showAll_);
-      if (isShown)
-        this.shownCount_++;
-      return isShown;
-    };
+  destinationsChanged_: function(current, previous) {
+    if (previous == undefined) {
+      this.matchingDestinationsCount_ = this.destinations.length;
+    } else {
+      this.update_();
+    }
   },
 
-  /**
-   * @return {number}
-   * @private
-   */
-  computeMatchingDestinationsCount_: function() {
-    return this.destinations
-        .filter(destination => {
-          return !this.searchQuery || destination.matches(this.searchQuery);
-        })
-        .length;
+  /** @private */
+  update_: function() {
+    if (!this.destinations)
+      return;
+
+    const listItems =
+        this.shadowRoot.querySelectorAll('print-preview-destination-list-item');
+
+    let matchCount = 0;
+    listItems.forEach(item => {
+      item.hidden =
+          !!this.searchQuery && !item.destination.matches(this.searchQuery);
+      if (!item.hidden) {
+        matchCount++;
+        item.update();
+      }
+    });
+
+    this.matchingDestinationsCount_ =
+        !this.searchQuery ? listItems.length : matchCount;
   },
 
   /**
    * @return {boolean}
    * @private
    */
-  computeFooterHidden_: function() {
-    return this.matchingDestinationsCount_ < SHORT_DESTINATION_LIST_SIZE ||
-        this.showAll_;
+  computeHasDestinations_: function() {
+    return !this.destinations || this.matchingDestinationsCount_ > 0;
   },
 
   /**
    * @return {boolean}
    * @private
    */
-  computeHasDestinations_: function() {
-    return this.matchingDestinationsCount_ > 0;
+  computeShowDestinationsTotal_: function() {
+    return this.matchingDestinationsCount_ > 4;
   },
 
   /** @private */
@@ -126,22 +113,15 @@ Polymer({
     print_preview.NativeLayer.getInstance().managePrinters();
   },
 
-  /** @private */
-  onShowAllTap_: function() {
-    this.showAll_ = true;
-  },
-
   /**
    * @param {!Event} e Event containing the destination that was selected.
    * @private
    */
   onDestinationSelected_: function(e) {
     this.fire(
-        'destination-selected', /** @type {!{model: Object}} */ (e).model.item);
-  },
-
-  reset: function() {
-    this.showAll_ = false;
+        'destination-selected',
+        /** @type {PrintPreviewDestinationListItemElement} */
+        (e.target).destination);
   },
 });
 })();

chrome/browser/resources/print_preview/new/destination_list_item.html

@@ -92,7 +92,7 @@
         font-style: italic;
       }
 
-      :host([stale]) :-webkit-any(.icon, .name, .connection-status) {
+      :host([stale_]) :-webkit-any(.icon, .name, .connection-status) {
         opacity: 0.4;
       }
     </style>

chrome/browser/resources/print_preview/new/destination_list_item.js

@@ -9,18 +9,17 @@ Polymer({
     /** @type {!print_preview.Destination} */
     destination: Object,
 
-    /** @private {string} */
-    searchHint_: {
-      type: String,
-      notify: true,
-    },
+    /** @type {?RegExp} */
+    searchQuery: Object,
 
-    /** @type {boolean} */
-    stale: {
+    /** @private */
+    stale_: {
       type: Boolean,
-      notify: true,
       reflectToAttribute: true,
     },
+
+    /** @private {string} */
+    searchHint_: String,
   },
 
   observers: [
@@ -34,40 +33,31 @@ Polymer({
   /** @private */
   onDestinationPropertiesChange_: function() {
     this.title = this.destination.displayName;
-    this.stale = this.destination.isOfflineOrInvalid;
+    this.stale_ = this.destination.isOfflineOrInvalid;
   },
 
-  /** @param {?RegExp} searchQuery The search query to update for. */
-  update: function(searchQuery) {
-    this.updateSearchHint_(searchQuery);
-    this.updateHighlighting_(searchQuery);
+  update: function() {
+    this.updateSearchHint_();
+    this.updateHighlighting_();
   },
 
-  /**
-   * @param {?RegExp} searchQuery The search query to update the hint for.
-   * @private
-   */
-  updateSearchHint_: function(searchQuery) {
-    if (!searchQuery) {
-      this.searchHint_ = '';
-      return;
-    }
-    this.searchHint_ = this.destination.extraPropertiesToMatch
-                           .filter(p => p.match(searchQuery))
-                           .join(' ');
+  /** @private */
+  updateSearchHint_: function() {
+    this.searchHint_ = !this.searchQuery ?
+        '' :
+        this.destination.extraPropertiesToMatch
+            .filter(p => p.match(this.searchQuery))
+            .join(' ');
   },
 
-  /**
-   * @param {?RegExp} searchQuery The search query to update the hint for.
-   * @private
-   */
-  updateHighlighting_: function(searchQuery) {
+  /** @private */
+  updateHighlighting_: function() {
     if (this.highlighted_) {
       cr.search_highlight_utils.findAndRemoveHighlights(this);
       this.highlighted_ = false;
     }
 
-    if (!searchQuery)
+    if (!this.searchQuery)
       return;
 
     this.shadowRoot.querySelectorAll('.searchable').forEach(element => {
@@ -79,7 +69,7 @@ Polymer({
         if (textContent.length == 0)
           return;
 
-        if (searchQuery.test(textContent)) {
+        if (this.searchQuery.test(textContent)) {
           // Don't highlight <select> nodes, yellow rectangles can't be
           // displayed within an <option>.
           // TODO(rbpotter): solve issue below before adding advanced
@@ -88,7 +78,7 @@ Polymer({
           // instead.
           if (node.parentNode.nodeName != 'OPTION') {
             cr.search_highlight_utils.highlight(
-                node, textContent.split(searchQuery));
+                node, textContent.split(this.searchQuery));
             this.highlighted_ = true;
           }
         }