Revert "Print Preview Refresh: Make pages a dropdown"

This reverts commit 525b54bb.

Reason for revert: Major changes to input behavior required, pushing
to later milestone.

Original change's description:
> 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}

TBR=dpapad@chromium.org,rbpotter@chromium.org

Change-Id: Id2bbfedbf7ab5406a7a583c9acaf9fd5eeac930b
No-Presubmit: true
No-Tree-Checks: true
No-Try: false
Bug: 889382, 889383, 889389
Reviewed-on: https://chromium-review.googlesource.com/1246407Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594457}

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

 <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 md-select">
+    <style include="print-preview-shared">
       :host([error-state_='0']) #pageSettingsCustomInput {
         --cr-input-error-display: none;
       }
 
-      #pageSettingsCustomInput {
-        --cr-input-row-container: {
-          min-height: 38px;
-        }
+      :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);
+        };
       }
 
       /* Margin = standard margin (16px) - error field margin (8px) */
-      :host(:not([error-state_='0'])) #customInputWrapper {
+      :host(:not([error-state_='0'])) print-preview-settings-section {
         margin-bottom: 8px;
       }
 
       #pageSettingsCustomInput {
         cursor: default;
-        --cr-form-field-label-height: 100%;
+        height: 100%;
       }
     </style>
-    <print-preview-settings-section>
-      <span slot="title" id="pages-label">$i18n{pagesLabel}</span>
-      <div slot="controls">
-        <select class="md-select" aria-labelledby="pages-label"
+    <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)]]"
-            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">
+              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"
+                data-timeout-delay="500" on-keydown="onKeydown_"
                 disabled$="[[getDisabled_(disabled)]]" spellcheck="false"
-              on-blur="onCustomInputBlur_" on-keydown="onKeydown_"
+                on-focus="onCustomInputFocus_" on-blur="onCustomInputBlur_"
                 placeholder="$i18n{examplePageRangeText}"
                 error-message="[[getHintMessage_(errorState_,
                     documentInfo.pageCount)]]">
             </cr-input>
+          </cr-radio-button>
+        </div>
       </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,19 +12,10 @@ 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,
-    print_preview_new.SelectBehavior
-  ],
+  behaviors: [SettingsBehavior, print_preview_new.InputBehavior],
 
   properties: {
     /** @type {!print_preview.DocumentInfo} */
@@ -42,14 +33,14 @@ Polymer({
       computed: 'computeAllPagesArray_(documentInfo.pageCount)',
     },
 
-    disabled: Boolean,
-
     /** @private {boolean} */
     customSelected_: {
       type: Boolean,
       value: false,
     },
 
+    disabled: Boolean,
+
     /** @private {number} */
     errorState_: {
       type: Number,
@@ -69,15 +60,6 @@ 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: [
@@ -89,11 +71,6 @@ 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;
@@ -107,14 +84,6 @@ 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
@@ -305,21 +274,34 @@ Polymer({
   },
 
   /** @private */
-  onSelectBlur_: function(event) {
-    if (!this.customSelected_ ||
-        event.relatedTarget === this.$.pageSettingsCustomInput) {
-      return;
-    }
+  onAllRadioClick_: function() {
+    this.customSelected_ = false;
+  },
+
+  /**
+   * @param {Event} event Contains information about where focus came from.
+   * @private
+   */
+  onCustomRadioFocus_: function(event) {
+    if (event.relatedTarget !== this.$.pageSettingsCustomInput)
+      this.onCustomRadioClick_();
+  },
 
-    this.onCustomInputBlur_(event);
+  /** @private */
+  onCustomRadioClick_: function() {
+    /** @type {!CrInputElement} */ (this.$.pageSettingsCustomInput)
+        .inputElement.focus();
+  },
+
+  /** @private */
+  onCustomInputFocus_: function() {
+    this.customSelected_ = true;
   },
 
   /** @private */
   resetIfEmpty_: function() {
-    if (this.inputString_ === '') {
+    if (this.inputString_ === '')
       this.customSelected_ = false;
-      this.selectedValue = PagesValue.ALL.toString();
-    }
   },
 
   /**
@@ -339,10 +321,8 @@ Polymer({
   onCustomInputBlur_: function(event) {
     this.resetAndUpdate();
 
-    if (event.relatedTarget != this.$.customInputWrapper &&
-        event.relatedTarget != this.$$('.md-select')) {
+    if (event.relatedTarget != this.$$('#custom-radio-button'))
       this.resetIfEmpty_();
-    }
   },
 
   /**

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

@@ -11,6 +11,7 @@
         --cr-input-row-container: {
           min-height: 38px;
         };
+        --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;
@@ -19,7 +20,8 @@
       }
 
       /* Default state ********************************************************/
-      input[type='checkbox'] {
+      input[type='checkbox'],
+      input[type='radio'] {
         margin-bottom: 0;
         margin-inline-end: 1px;
         margin-inline-start: 0;
@@ -42,11 +44,13 @@
         user-select: none;
       }
 
-      .checkbox cr-checkbox {
+      .checkbox cr-checkbox,
+      .radio cr-radio-button {
         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,39 +166,6 @@ 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,15 +68,12 @@ cr.define('pages_settings_test', function() {
       Polymer.dom.flush();
 
       const input = pagesSection.$.pageSettingsCustomInput.inputElement;
-      const pagesSelect = pagesSection.$$('select');
-      const readyForInput =
-          pagesSelect.value === pagesSection.pagesValueEnum_.CUSTOM.toString() ?
+      const readyForInput = pagesSection.$$('#custom-radio-button').checked ?
           Promise.resolve() :
-          test_util.eventToPromise('process-select-change', pagesSection);
+          test_util.eventToPromise('focus', input);
 
       // Select custom
-      pagesSelect.value = pagesSection.pagesValueEnum_.CUSTOM.toString();
-      pagesSelect.dispatchEvent(new CustomEvent('change'));
+      pagesSection.$$('#custom-radio-button').click();
       return readyForInput.then(() => {
         // Set input string
         input.value = inputString;

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

@@ -99,3 +99,36 @@ 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,21 +557,20 @@ cr.define('settings_sections_tests', function() {
       assertFalse(pagesElement.hidden);
 
       // Default value is all pages. Print ticket expects this to be empty.
-      const pagesSelect = pagesElement.$$('select');
-      const customInputCollapse = pagesElement.$$('iron-collapse');
+      const allRadio = pagesElement.$$('#all-radio-button');
+      const customRadio = pagesElement.$$('#custom-radio-button');
       const pagesCrInput = pagesElement.$.pageSettingsCustomInput;
       const pagesInput = pagesCrInput.inputElement;
 
       /**
-       * @param {boolean} allSelected Whether the all pages option is selected.
+       * @param {boolean} allChecked Whether the all pages radio button is
+       *     selected.
        * @param {string} inputString The expected string in the pages input.
        * @param {boolean} valid Whether the input string is valid.
        */
-      const validateInputState = function(allSelected, inputString, valid) {
-        assertEquals(allSelected, !customInputCollapse.opened);
-        assertEquals(
-            allSelected,
-            pagesSelect.value === pagesElement.pagesValueEnum_.ALL.toString());
+      const validateInputState = function(allChecked, inputString, valid) {
+        assertEquals(allChecked, allRadio.checked);
+        assertEquals(!allChecked, customRadio.checked);
         assertEquals(inputString, pagesInput.value);
         assertEquals(valid, !pagesCrInput.invalid);
       };
@@ -581,14 +580,15 @@ cr.define('settings_sections_tests', function() {
       assertTrue(page.settings.pages.valid);
 
       // Set selection of pages 1 and 2.
-      pagesSelect.value = pagesElement.pagesValueEnum_.CUSTOM.toString();
-      pagesSelect.dispatchEvent(new CustomEvent('change'));
+      customRadio.click();
+
+      // 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);
 
-      return test_util.eventToPromise('process-select-change', pagesElement)
-          .then(function() {
       triggerInputEvent(pagesInput, '1-2');
-            return test_util.eventToPromise('input-change', pagesElement);
-          })
+      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

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

testing/buildbot/filters/webui_polymer2_interactive_ui_tests.filter

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