Print Preview Refresh: Make pages a dropdown

Make pages a dropdown, and only show the custom input if "Custom"
page range is selected

Bug: None
Change-Id: If29a941ca2ff9d75f08bc1e103ac3d0a836e540c
Reviewed-on: https://chromium-review.googlesource.com/1237241
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594076}

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

@@ -1,69 +1,71 @@
 <link rel="import" href="chrome://resources/html/polymer.html">
 
-<link rel="import" href="chrome://resources/cr_elements/cr_radio_button/cr_radio_button.html">
 <link rel="import" href="chrome://resources/cr_elements/cr_input/cr_input.html">
+<link rel="import" href="chrome://resources/html/md_select_css.html">
+<link rel="import" href="chrome://resources/polymer/v1_0/iron-collapse/iron-collapse.html">
 <link rel="import" href="../data/document_info.html">
 <link rel="import" href="../print_preview_utils.html">
 <link rel="import" href="input_behavior.html">
 <link rel="import" href="print_preview_shared_css.html">
+<link rel="import" href="select_behavior.html">
 <link rel="import" href="settings_behavior.html">
 <link rel="import" href="settings_section.html">
 <link rel="import" href="strings.html">
 
 <dom-module id="print-preview-pages-settings">
   <template>
-    <style include="print-preview-shared">
+    <style include="print-preview-shared md-select">
       :host([error-state_='0']) #pageSettingsCustomInput {
         --cr-input-error-display: none;
       }
 
-      :host(:not([error-state_='0'])) #custom-radio-button {
-        /* Margin = -1 * error height = -1 * (16px + 2 lines * line-height) */
-        --cr-radio-button-disc: {
-          margin-top: calc(-16px - 2 * .75rem);
-        };
+      #pageSettingsCustomInput {
+        --cr-input-row-container: {
+          min-height: 38px;
+        }
       }
 
       /* Margin = standard margin (16px) - error field margin (8px) */
-      :host(:not([error-state_='0'])) print-preview-settings-section {
+      :host(:not([error-state_='0'])) #customInputWrapper {
         margin-bottom: 8px;
       }
 
       #pageSettingsCustomInput {
         cursor: default;
-        height: 100%;
+        --cr-form-field-label-height: 100%;
       }
     </style>
-    <print-preview-settings-section
-        class="input-settings-section multirow-controls">
-      <span slot="title">$i18n{pagesLabel}</span>
-      <div slot="controls" id="controls">
-        <div class="radio">
-          <cr-radio-button name="pages" id="all-radio-button"
-              checked$="[[!customSelected_]]"
-              disabled$="[[getDisabled_(disabled)]]"
-              on-click="onAllRadioClick_">
-            <span>$i18n{optionAllPages}</span>
-          </cr-radio-button>
-          <cr-radio-button name="pages" id="custom-radio-button"
-              checked$="[[customSelected_]]"
-              disabled$="[[getDisabled_(disabled)]]"
-              aria-label="$i18n{optionCustomPages}"
-              on-click="onCustomRadioClick_"
-              on-focus="onCustomRadioFocus_"
-              on-blur="onCustomInputBlur_">
-            <cr-input id="pageSettingsCustomInput" type="text"
-                data-timeout-delay="500"
-                disabled$="[[getDisabled_(disabled)]]" spellcheck="false"
-                on-focus="onCustomInputFocus_" on-blur="onCustomInputBlur_"
-                placeholder="$i18n{examplePageRangeText}"
-                error-message="[[getHintMessage_(errorState_,
-                    documentInfo.pageCount)]]">
-            </cr-input>
-          </cr-radio-button>
-        </div>
+    <print-preview-settings-section>
+      <span slot="title" id="pages-label">$i18n{pagesLabel}</span>
+      <div slot="controls">
+        <select class="md-select" aria-labelledby="pages-label"
+            disabled$="[[getDisabled_(disabled)]]"
+            value="{{selectedValue::change}}"
+            on-blur="onSelectBlur_">
+          <option value="[[pagesValueEnum_.ALL]]" selected>
+            $i18n{optionAllPages}
+          </option>
+          <option value="[[pagesValueEnum_.CUSTOM]]">
+            $i18n{optionCustomPages}
+          </option>
+        </select>
       </div>
     </print-preview-settings-section>
+    <iron-collapse opened="[[customSelected_]]">
+      <print-preview-settings-section id="customInputWrapper">
+        <div slot="title"></div>
+        <div slot="controls">
+          <cr-input id="pageSettingsCustomInput" type="text"
+              data-timeout-delay="500"
+              disabled$="[[getDisabled_(disabled)]]" spellcheck="false"
+              on-blur="onCustomInputBlur_"
+              placeholder="$i18n{examplePageRangeText}"
+              error-message="[[getHintMessage_(errorState_,
+                  documentInfo.pageCount)]]">
+          </cr-input>
+        </div>
+      </print-preview-settings-section>
+    </iron-collapse>
   </template>
   <script src="pages_settings.js"></script>
 </dom-module>

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

@@ -12,10 +12,19 @@ const PagesInputErrorState = {
   OUT_OF_BOUNDS: 2,
 };
 
+/** @enum {number} */
+const PagesValue = {
+  ALL: 0,
+  CUSTOM: 1,
+};
+
 Polymer({
   is: 'print-preview-pages-settings',
 
-  behaviors: [SettingsBehavior, print_preview_new.InputBehavior],
+  behaviors: [
+    SettingsBehavior, print_preview_new.InputBehavior,
+    print_preview_new.SelectBehavior
+  ],
 
   properties: {
     /** @type {!print_preview.DocumentInfo} */
@@ -33,14 +42,14 @@ Polymer({
       computed: 'computeAllPagesArray_(documentInfo.pageCount)',
     },
 
+    disabled: Boolean,
+
     /** @private {boolean} */
     customSelected_: {
       type: Boolean,
       value: false,
     },
 
-    disabled: Boolean,
-
     /** @private {number} */
     errorState_: {
       type: Number,
@@ -60,6 +69,15 @@ Polymer({
       type: Array,
       computed: 'computeRangesToPrint_(pagesToPrint_, allPagesArray_)',
     },
+
+    /**
+     * Mirroring the enum so that it can be used from HTML bindings.
+     * @private
+     */
+    pagesValueEnum_: {
+      type: Object,
+      value: PagesValue,
+    },
   },
 
   observers: [
@@ -71,6 +89,11 @@ Polymer({
     'input-change': 'onInputChange_',
   },
 
+  // Initialize in attached() since this doesn't observe settings.pages.
+  attached: function() {
+    this.selectedValue = PagesValue.ALL.toString();
+  },
+
   /** @return {!CrInputElement} The cr-input field element for InputBehavior. */
   getInput: function() {
     return this.$.pageSettingsCustomInput;
@@ -84,6 +107,14 @@ Polymer({
     this.inputString_ = /** @type {string} */ (e.detail);
   },
 
+  onProcessSelectChange: function(value) {
+    this.customSelected_ = value === PagesValue.CUSTOM.toString();
+    if (this.customSelected_) {
+      /** @type {!CrInputElement} */ (this.$.pageSettingsCustomInput)
+          .inputElement.focus();
+    }
+  },
+
   /**
    * @return {boolean} Whether the controls should be disabled.
    * Does not need to depend on |errorState_|, since |errorState_| is always
@@ -274,28 +305,13 @@ Polymer({
   },
 
   /** @private */
-  onAllRadioClick_: function() {
-    this.customSelected_ = false;
-  },
+  onSelectBlur_: function(event) {
+    if (!this.customSelected_ ||
+        event.relatedTarget === this.$.pageSettingsCustomInput) {
+      return;
+    }
 
-  /**
-   * @param {Event} event Contains information about where focus came from.
-   * @private
-   */
-  onCustomRadioFocus_: function(event) {
-    if (event.relatedTarget !== this.$.pageSettingsCustomInput)
-      this.onCustomRadioClick_();
-  },
-
-  /** @private */
-  onCustomRadioClick_: function() {
-    /** @type {!CrInputElement} */ (this.$.pageSettingsCustomInput)
-        .inputElement.focus();
-  },
-
-  /** @private */
-  onCustomInputFocus_: function() {
-    this.customSelected_ = true;
+    this.onCustomInputBlur_(event);
   },
 
   /**
@@ -305,9 +321,11 @@ Polymer({
   onCustomInputBlur_: function(event) {
     this.resetAndUpdate();
 
-    if (this.inputString_.trim() == '' &&
-        event.relatedTarget != this.$$('#custom-radio-button')) {
+    if (this.inputString_ === '' &&
+        event.relatedTarget != this.$.customInputWrapper &&
+        event.relatedTarget != this.$$('.md-select')) {
       this.customSelected_ = false;
+      this.selectedValue = PagesValue.ALL.toString();
     }
   },
 

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

@@ -15,7 +15,6 @@
           margin-top: 3px;
           margin-bottom: 3px;
         };
-        --cr-radio-button-size: 16px;
         --md-select-width: calc(100% - 17px);
         --print-preview-settings-border: 1px solid rgb(232, 234, 237);
         --print-preview-dialog-margin: 34px;
@@ -24,8 +23,7 @@
       }
 
       /* Default state ********************************************************/
-      input[type='checkbox'],
-      input[type='radio'] {
+      input[type='checkbox'] {
         margin-bottom: 0;
         margin-inline-end: 1px;
         margin-inline-start: 0;
@@ -48,13 +46,11 @@
         user-select: none;
       }
 
-      .checkbox cr-checkbox,
-      .radio cr-radio-button {
+      .checkbox cr-checkbox {
         margin-bottom: 3px;
         margin-top: 3px;
         min-height: 32px;
         --cr-checkbox-ripple-size: 38px;
-        --cr-radio-button-ink-size: 38px;
         --cr-checkbox-label-container: {
           overflow: hidden;
           padding-inline-start: 15px;

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

@@ -166,6 +166,39 @@ TEST_F(
           settings_sections_tests.TestNames.DisableMarginsByPagesPerSheet);
     });
 
+PrintPreviewPagesSettingsTest = class extends NewPrintPreviewTest {
+  /** @override */
+  get browsePreload() {
+    return 'chrome://print/new/pages_settings.html';
+  }
+
+  /** @override */
+  get extraLibraries() {
+    return super.extraLibraries.concat([
+      '../settings/test_util.js',
+      'print_preview_test_utils.js',
+      'pages_settings_test.js',
+    ]);
+  }
+
+  /** @override */
+  get suiteName() {
+    return pages_settings_test.suiteName;
+  }
+};
+
+TEST_F('PrintPreviewPagesSettingsTest', 'ValidPageRanges', function() {
+  this.runMochaTest(pages_settings_test.TestNames.ValidPageRanges);
+});
+
+TEST_F('PrintPreviewPagesSettingsTest', 'InvalidPageRanges', function() {
+  this.runMochaTest(pages_settings_test.TestNames.InvalidPageRanges);
+});
+
+TEST_F('PrintPreviewPagesSettingsTest', 'NupChangesPages', function() {
+  this.runMochaTest(pages_settings_test.TestNames.NupChangesPages);
+});
+
 PrintPreviewPolicyTest = class extends NewPrintPreviewTest {
   /** @override */
   get browsePreload() {

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

@@ -68,12 +68,15 @@ cr.define('pages_settings_test', function() {
       Polymer.dom.flush();
 
       const input = pagesSection.$.pageSettingsCustomInput.inputElement;
-      const readyForInput = pagesSection.$$('#custom-radio-button').checked ?
+      const pagesSelect = pagesSection.$$('select');
+      const readyForInput =
+          pagesSelect.value === pagesSection.pagesValueEnum_.CUSTOM.toString() ?
           Promise.resolve() :
-          test_util.eventToPromise('focus', input);
+          test_util.eventToPromise('process-select-change', pagesSection);
 
       // Select custom
-      pagesSection.$$('#custom-radio-button').click();
+      pagesSelect.value = pagesSection.pagesValueEnum_.CUSTOM.toString();
+      pagesSelect.dispatchEvent(new CustomEvent('change'));
       return readyForInput.then(() => {
         // Set input string
         input.value = inputString;

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

@@ -99,36 +99,3 @@ TEST_F(
       this.runMochaTest(
           destination_dialog_interactive_test.TestNames.FocusSearchBox);
     });
-
-PrintPreviewPagesSettingsTest = class extends PrintPreviewInteractiveUITest {
-  /** @override */
-  get browsePreload() {
-    return 'chrome://print/new/pages_settings.html';
-  }
-
-  /** @override */
-  get extraLibraries() {
-    return super.extraLibraries.concat([
-      '../settings/test_util.js',
-      'print_preview_test_utils.js',
-      'pages_settings_test.js',
-    ]);
-  }
-
-  /** @override */
-  get suiteName() {
-    return pages_settings_test.suiteName;
-  }
-};
-
-TEST_F('PrintPreviewPagesSettingsTest', 'ValidPageRanges', function() {
-  this.runMochaTest(pages_settings_test.TestNames.ValidPageRanges);
-});
-
-TEST_F('PrintPreviewPagesSettingsTest', 'InvalidPageRanges', function() {
-  this.runMochaTest(pages_settings_test.TestNames.InvalidPageRanges);
-});
-
-TEST_F('PrintPreviewPagesSettingsTest', 'NupChangesPages', function() {
-  this.runMochaTest(pages_settings_test.TestNames.NupChangesPages);
-});

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

@@ -557,20 +557,21 @@ cr.define('settings_sections_tests', function() {
       assertFalse(pagesElement.hidden);
 
       // Default value is all pages. Print ticket expects this to be empty.
-      const allRadio = pagesElement.$$('#all-radio-button');
-      const customRadio = pagesElement.$$('#custom-radio-button');
+      const pagesSelect = pagesElement.$$('select');
+      const customInputCollapse = pagesElement.$$('iron-collapse');
       const pagesCrInput = pagesElement.$.pageSettingsCustomInput;
       const pagesInput = pagesCrInput.inputElement;
 
       /**
-       * @param {boolean} allChecked Whether the all pages radio button is
-       *     selected.
+       * @param {boolean} allSelected Whether the all pages option is selected.
        * @param {string} inputString The expected string in the pages input.
        * @param {boolean} valid Whether the input string is valid.
        */
-      const validateInputState = function(allChecked, inputString, valid) {
-        assertEquals(allChecked, allRadio.checked);
-        assertEquals(!allChecked, customRadio.checked);
+      const validateInputState = function(allSelected, inputString, valid) {
+        assertEquals(allSelected, !customInputCollapse.opened);
+        assertEquals(
+            allSelected,
+            pagesSelect.value === pagesElement.pagesValueEnum_.ALL.toString());
         assertEquals(inputString, pagesInput.value);
         assertEquals(valid, !pagesCrInput.invalid);
       };
@@ -580,15 +581,14 @@ cr.define('settings_sections_tests', function() {
       assertTrue(page.settings.pages.valid);
 
       // Set selection of pages 1 and 2.
-      customRadio.click();
+      pagesSelect.value = pagesElement.pagesValueEnum_.CUSTOM.toString();
+      pagesSelect.dispatchEvent(new CustomEvent('change'));
 
-      // Manually set |customSelected_| since focus may not work correctly on
-      // MacOS. The PageSettingsTests verify this behavior is correct on all
-      // platforms.
-      pagesElement.set('customSelected_', true);
-
-      triggerInputEvent(pagesInput, '1-2');
-      return test_util.eventToPromise('input-change', pagesElement)
+      return test_util.eventToPromise('process-select-change', pagesElement)
+          .then(function() {
+            triggerInputEvent(pagesInput, '1-2');
+            return test_util.eventToPromise('input-change', pagesElement);
+          })
           .then(function() {
             validateInputState(false, '1-2', true);
             assertEquals(1, page.settings.ranges.value.length);

testing/buildbot/filters/webui_polymer2_browser_tests.filter

@@ -288,6 +288,7 @@ PrintPreviewLinkContainerTest.*
 PrintPreviewModelTest.*
 PrintPreviewNewDestinationSearchTest.*
 PrintPreviewNumberSettingsSectionTest.*
+PrintPreviewPagesSettingsTest.*
 PrintPreviewPolicyTest.*
 PrintPreviewPreviewGenerationTest.*
 PrintPreviewPrintButtonTest.*

testing/buildbot/filters/webui_polymer2_interactive_ui_tests.filter

@@ -15,5 +15,4 @@ MaterialBookmarksFocusTest.All
 MaterialHistoryFocusTest.All
 PrintPreviewDestinationDialogInteractiveTest.FocusSearchBox
 PrintPreviewPrintHeaderInteractiveTest.FocusPrintOnReady
-PrintPreviewPagesSettingsTest.*
 SettingsUIBrowserTest.All