Print Preview: Simplify other_options_settings javascript code.

chrome/browser/resources/print_preview/settings/other_options_settings.js

Index: chrome/browser/resources/print_preview/settings/other_options_settings.js
diff --git a/chrome/browser/resources/print_preview/settings/other_options_settings.js b/chrome/browser/resources/print_preview/settings/other_options_settings.js
index c5e1563e29a8b76d05d9eea66f8ca0d5ea0fd5c7..e6762646f15c70a879dd6bffe1a96720941fa8c0 100644
--- a/chrome/browser/resources/print_preview/settings/other_options_settings.js
+++ b/chrome/browser/resources/print_preview/settings/other_options_settings.js
@@ -6,127 +6,149 @@ cr.define('print_preview', function() {
   'use strict';
 
   /**
-   * UI component that renders checkboxes for various print options.
-   * @param {!print_preview.ticket_items.Duplex} duplex Duplex ticket item.
-   * @param {!print_preview.ticket_items.FitToPage} fitToPage Fit-to-page ticket
-   *     item.
-   * @param {!print_preview.ticket_items.CssBackground} cssBackground CSS
-   *     background ticket item.
-   * @param {!print_preview.ticket_items.SelectionOnly} selectionOnly Selection
-   *     only ticket item.
-   * @param {!print_preview.ticket_items.HeaderFooter} headerFooter Header
-   *     footer ticket item.
+   * Represents a single option in the Other Options settings section.
+   * @param {!print_preview.ticket_items.TicketItem} The ticket item for this
+   *     element, used to read and write.
+   * @param {boolean} Whether this option is collapsible or not.
+   * @param {string} The CSS id selector string for the container element for

[dpapad, 2016/12/15 20:32:59]

s/selector string//
(See related following comment too).

Also all @param annotations here are missing the name of the parameter. The
valid syntax is

@param {!print_preview.ticket_items.TicketItem} ticketItem The ticket item ....

[rbpotter, 2016/12/15 23:28:17]

Done.

+   *     this option, used to access the container and checkbox HTML elements.
    * @constructor
-   * @extends {print_preview.SettingsSection}
    */
-  function OtherOptionsSettings(
-      duplex, fitToPage, cssBackground, selectionOnly, headerFooter) {
-    print_preview.SettingsSection.call(this);
-
+  function CheckboxTicketItemElement(ticketItem, collapsible, cssId) {
     /**
-     * Duplex ticket item, used to read/write the duplex selection.
-     * @type {!print_preview.ticket_items.Duplex}
-     * @private
+     * Ticket item for this element, used to read/write.
+     * @private {!print_preview.ticket_items.TicketItem}
      */
-    this.duplexTicketItem_ = duplex;
+    this.ticketItem_ = ticketItem;
 
     /**
-     * Fit-to-page ticket item, used to read/write the fit-to-page selection.
-     * @type {!print_preview.ticket_items.FitToPage}
-     * @private
+     * Whether this element is collapsible.
+     * @private {boolean}
      */
-    this.fitToPageTicketItem_ = fitToPage;
+    this.collapsible_ = collapsible;
 
     /**
-     * Enable CSS backgrounds ticket item, used to read/write.
-     * @type {!print_preview.ticket_items.CssBackground}
-     * @private
+     * The CSS id string for the container element.
+     * @private {string}
      */
-    this.cssBackgroundTicketItem_ = cssBackground;
+    this.cssId_ = cssId;
 
     /**
-     * Print selection only ticket item, used to read/write.
-     * @type {!print_preview.ticket_items.SelectionOnly}
-     * @private
+     * The HTML container element. Populated when decorate() is called.
+     * @private {HTMLElement}
      */
-    this.selectionOnlyTicketItem_ = selectionOnly;
+    this.container_ = null;
 
     /**
-     * Header-footer ticket item, used to read/write.
-     * @type {!print_preview.ticket_items.HeaderFooter}
-     * @private
+     * The HTML checkbox input element. The checkbox child element of
+     * container_. Populated when decorate() is called.
+     * @private {HTMLElement}
      */
-    this.headerFooterTicketItem_ = headerFooter;
+    this.checkbox_ = null;
+  };
 
-     /**
-     * Header footer container element.
-     * @type {HTMLElement}
-     * @private
-     */
-    this.headerFooterContainer_ = null;
+  CheckboxTicketItemElement.prototype = {
 
-    /**
-     * Header footer checkbox.
-     * @type {HTMLInputElement}
-     * @private
-     */
-    this.headerFooterCheckbox_ = null;
+    /** @return {boolean} Whether the element is collapsible */
+    get collapsible() {
+      return this.collapsible_;
+    },
 
     /**
-     * Fit-to-page container element.
-     * @type {HTMLElement}
-     * @private
+     * @return {!print_preview.ticket_items.TicketItem} The ticket item for this
+     *      element.
      */
-    this.fitToPageContainer_ = null;
+    get ticketItem() {
+      return this.ticketItem_;
+    },
 
-    /**
-     * Fit-to-page checkbox.
-     * @type {HTMLInputElement}
-     * @private
-     */
-    this.fitToPageCheckbox_ = null;
+    /** @return {HTMLElement} The checkbox HTML element. */
+    get checkbox() {
+      return this.checkbox_;
+    },
 
-    /**
-     * Duplex container element.
-     * @type {HTMLElement}
-     * @private
-     */
-    this.duplexContainer_ = null;
+    /** Initializes container and checkbox */
+    decorate: function(otherOptionsSettings) {
+        this.container_ = otherOptionsSettings.getElement().querySelector(
+            this.cssId_);

[dpapad, 2016/12/15 20:32:59]

Technically the "#" prefix is not part of the ID. Can you remove it from lines
141-149, and simply add it here when calling querySelector?

this.container_ = otherOptionsSettings.getElement().querySelector('#' +
this.cssId_);

But also,
There is not really a need to pass |otherOptionsSettings| as a parameter, is it?
You could simply get the container elements by ID as follows

document.getElementById(this.cssId_);  // Note that for this to work cssId_
should not include the '#' prefix. So how about the following?

this.container_ = document.getElementById(this.cssId_);
this.checkbox_ = this.container_.querySelector('.checkbox');

[rbpotter, 2016/12/15 23:28:17]

Done.

+        this.checkbox_ = this.container_.querySelector('.checkbox');
+    },
 
-    /**
-     * Duplex checkbox.
-     * @type {HTMLInputElement}
-     * @private
-     */
-    this.duplexCheckbox_ = null;
+    /** Resets container and checkbox. */
+    exitDocument: function() {
+      this.container_ = null;
+      this.checkbox_ = null;
+    },
 
-    /**
-     * Print CSS backgrounds container element.
-     * @type {HTMLElement}
-     * @private
-     */
-    this.cssBackgroundContainer_ = null;
+    /** Called when the checkbox is clicked. Updates the ticket item value. */
+    onCheckboxClick: function() {
+      this.ticketItem_.updateValue(this.checkbox_.checked);
+    },
 
     /**
-     * Print CSS backgrounds checkbox.
-     * @type {HTMLInputElement}
-     * @private
+     * Called when the ticket item changes. Updates the UI state.
+     * @param {!print_preview.SettingsSection.OtherOptionsSettings} The
+     *        settings section that this element is part of.
      */
-    this.cssBackgroundCheckbox_ = null;
+    onTicketItemChange: function(otherOptionsSettings) {
+      this.checkbox_.checked = this.ticketItem_.getValue();
+      otherOptionsSettings.updateUiStateInternal();
+    },
 
     /**
-     * Print selection only container element.
-     * @type {HTMLElement}
-     * @private
+     * @param {boolean} Whether the settings section has content collapsed.
+     * @return {boolean} Whether this element should be visible.
      */
-    this.selectionOnlyContainer_ = null;
+    isVisible: function(collapseContent) {
+      return this.ticketItem_.isCapabilityAvailable() &&
+             (!this.collapsible_ || !collapseContent);
+    },
 
     /**
-     * Print selection only checkbox.
-     * @type {HTMLInputElement}
-     * @private
+     * Sets the visibility of the element.
+     * @param {boolean} Whether the settings section has content collapsed.
      */
-    this.selectionOnlyCheckbox_ = null;
+    setVisibility: function(collapseContent) {
+      setIsVisible(this.container_, this.isVisible(collapseContent));
+    },
+
+  };
+
+  /**
+   * UI component that renders checkboxes for various print options.
+   * @param {!print_preview.ticket_items.Duplex} duplex Duplex ticket item.
+   * @param {!print_preview.ticket_items.FitToPage} fitToPage Fit-to-page ticket
+   *     item.
+   * @param {!print_preview.ticket_items.CssBackground} cssBackground CSS
+   *     background ticket item.
+   * @param {!print_preview.ticket_items.SelectionOnly} selectionOnly Selection
+   *     only ticket item.
+   * @param {!print_preview.ticket_items.HeaderFooter} headerFooter Header
+   *     footer ticket item.
+   * @constructor
+   * @extends {print_preview.SettingsSection}
+   */
+  function OtherOptionsSettings(
+      duplex, fitToPage, cssBackground, selectionOnly, headerFooter) {
+    print_preview.SettingsSection.call(this);
+
+    /*
+     * @private {!Array<!CheckboxTicketItemElement>} checkbox ticket item
+     *      elements representing the different options in the section.
+     *      Selection only must always be the last element in the array.
+     */
+    this.elements_ = [
+        new CheckboxTicketItemElement(headerFooter, true,
+                                      '#header-footer-container'),
+        new CheckboxTicketItemElement(fitToPage, false,
+                                      '#fit-to-page-container'),
+        new CheckboxTicketItemElement(duplex, false, '#duplex-container'),
+        new CheckboxTicketItemElement(cssBackground, true,
+                                      '#css-background-container'),
+        new CheckboxTicketItemElement(selectionOnly, true,
+                                      '#selection-only-container')
+    ];
+
   };
 
   OtherOptionsSettings.prototype = {
@@ -134,241 +156,71 @@ cr.define('print_preview', function() {
 
     /** @override */
     isAvailable: function() {
-      return this.headerFooterTicketItem_.isCapabilityAvailable() ||
-             this.fitToPageTicketItem_.isCapabilityAvailable() ||
-             this.duplexTicketItem_.isCapabilityAvailable() ||
-             this.cssBackgroundTicketItem_.isCapabilityAvailable() ||
-             this.selectionOnlyTicketItem_.isCapabilityAvailable();
+      return this.elements_.some(function(element) {
+        return element.ticketItem.isCapabilityAvailable();
+      });
     },
 
     /** @override */
     hasCollapsibleContent: function() {
-      return this.headerFooterTicketItem_.isCapabilityAvailable() ||
-             this.cssBackgroundTicketItem_.isCapabilityAvailable() ||
-             this.selectionOnlyTicketItem_.isCapabilityAvailable();
+      return this.elements_.some(function(element) {
+        return element.collapsible;
+      });
     },
 
     /** @override */
     set isEnabled(isEnabled) {
-      this.headerFooterCheckbox_.disabled = !isEnabled;
-      this.fitToPageCheckbox_.disabled = !isEnabled;
-      this.duplexCheckbox_.disabled = !isEnabled;
-      this.cssBackgroundCheckbox_.disabled = !isEnabled;
+      /* Skip selection only, which must always be the last element, as this

[dpapad, 2016/12/15 20:32:59]

Nit:
Skip |ticket_items.SelectionOnly|, which must always be .....

[rbpotter, 2016/12/15 23:28:17]

Done.

+       * checkbox is enabled based on whether the user has selected something
+       * in the page, which is different logic from the other elements. */
+      for (var i = 0; i < this.elements_.length - 1; i++)
+        this.elements_[i].checkbox.disabled = !isEnabled;
     },
 
     /** @override */
     enterDocument: function() {
       print_preview.SettingsSection.prototype.enterDocument.call(this);
-      this.tracker.add(
-          this.headerFooterCheckbox_,
-          'click',
-          this.onHeaderFooterCheckboxClick_.bind(this));
-      this.tracker.add(
-          this.fitToPageCheckbox_,
-          'click',
-          this.onFitToPageCheckboxClick_.bind(this));
-      this.tracker.add(
-          this.duplexCheckbox_,
-          'click',
-          this.onDuplexCheckboxClick_.bind(this));
-      this.tracker.add(
-          this.cssBackgroundCheckbox_,
-          'click',
-          this.onCssBackgroundCheckboxClick_.bind(this));
-      this.tracker.add(
-          this.selectionOnlyCheckbox_,
-          'click',
-          this.onSelectionOnlyCheckboxClick_.bind(this));
-      this.tracker.add(
-          this.duplexTicketItem_,
-          print_preview.ticket_items.TicketItem.EventType.CHANGE,
-          this.onDuplexChange_.bind(this));
-      this.tracker.add(
-          this.fitToPageTicketItem_,
-          print_preview.ticket_items.TicketItem.EventType.CHANGE,
-          this.onFitToPageChange_.bind(this));
-      this.tracker.add(
-          this.cssBackgroundTicketItem_,
-          print_preview.ticket_items.TicketItem.EventType.CHANGE,
-          this.onCssBackgroundChange_.bind(this));
-      this.tracker.add(
-          this.selectionOnlyTicketItem_,
-          print_preview.ticket_items.TicketItem.EventType.CHANGE,
-          this.onSelectionOnlyChange_.bind(this));
-      this.tracker.add(
-          this.headerFooterTicketItem_,
-          print_preview.ticket_items.TicketItem.EventType.CHANGE,
-          this.onHeaderFooterChange_.bind(this));
+      this.elements_.forEach(function(element) {
+        this.tracker.add(
+            element.checkbox,
+            'click',
+            element.onCheckboxClick.bind(element));
+        this.tracker.add(
+            element.ticketItem,
+            print_preview.ticket_items.TicketItem.EventType.CHANGE,
+            element.onTicketItemChange.bind(element, this));
+      }, this);
     },
 
     /** @override */
     exitDocument: function() {
       print_preview.SettingsSection.prototype.exitDocument.call(this);
-      this.headerFooterContainer_ = null;
-      this.headerFooterCheckbox_ = null;
-      this.fitToPageContainer_ = null;
-      this.fitToPageCheckbox_ = null;
-      this.duplexContainer_ = null;
-      this.duplexCheckbox_ = null;
-      this.cssBackgroundContainer_ = null;
-      this.cssBackgroundCheckbox_ = null;
-      this.selectionOnlyContainer_ = null;
-      this.selectionOnlyCheckbox_ = null;
+      for (var i = 0; i < this.elements_.length; i++)
+        this.elements_[i].exitDocument();
     },
 
     /** @override */
     decorateInternal: function() {
-      this.headerFooterContainer_ = this.getElement().querySelector(
-          '.header-footer-container');
-      this.headerFooterCheckbox_ = this.headerFooterContainer_.querySelector(
-          '.header-footer-checkbox');
-      this.fitToPageContainer_ = this.getElement().querySelector(
-          '.fit-to-page-container');
-      this.fitToPageCheckbox_ = this.fitToPageContainer_.querySelector(
-          '.fit-to-page-checkbox');
-      this.duplexContainer_ = this.getElement().querySelector(
-          '.duplex-container');
-      this.duplexCheckbox_ = this.duplexContainer_.querySelector(
-          '.duplex-checkbox');
-      this.cssBackgroundContainer_ = this.getElement().querySelector(
-          '.css-background-container');
-      this.cssBackgroundCheckbox_ = this.cssBackgroundContainer_.querySelector(
-          '.css-background-checkbox');
-      this.selectionOnlyContainer_ = this.getElement().querySelector(
-          '.selection-only-container');
-      this.selectionOnlyCheckbox_ = this.selectionOnlyContainer_.querySelector(
-          '.selection-only-checkbox');
+      for (var i = 0; i < this.elements_.length; i++)
+        this.elements_[i].decorate(this);
     },
 
     /** @override */
     updateUiStateInternal: function() {
       if (this.isAvailable()) {
-        setIsVisible(this.headerFooterContainer_,
-                     this.headerFooterTicketItem_.isCapabilityAvailable() &&
-                     !this.collapseContent);
-        setIsVisible(this.fitToPageContainer_,
-                     this.fitToPageTicketItem_.isCapabilityAvailable());
-        setIsVisible(this.duplexContainer_,
-                     this.duplexTicketItem_.isCapabilityAvailable());
-        setIsVisible(this.cssBackgroundContainer_,
-                     this.cssBackgroundTicketItem_.isCapabilityAvailable() &&
-                     !this.collapseContent);
-        setIsVisible(this.selectionOnlyContainer_,
-                     this.selectionOnlyTicketItem_.isCapabilityAvailable() &&
-                     !this.collapseContent);
+        for (var i = 0; i < this.elements_.length; i++)
+          this.elements_[i].setVisibility(this.collapseContent);
       }
       print_preview.SettingsSection.prototype.updateUiStateInternal.call(this);
     },
 
     /** @override */
     isSectionVisibleInternal: function() {
-      if (this.collapseContent) {
-        return this.fitToPageTicketItem_.isCapabilityAvailable() ||
-               this.duplexTicketItem_.isCapabilityAvailable();
-      }
-
-      return this.isAvailable();
+      return this.elements_.some(function(element) {
+        return element.isVisible(this.collapseContent);
+      }, this);
     },
 
-     /**
-     * Called when the header-footer checkbox is clicked. Updates the print
-     * ticket.
-     * @private
-     */
-    onHeaderFooterCheckboxClick_: function() {
-      this.headerFooterTicketItem_.updateValue(
-          this.headerFooterCheckbox_.checked);
-    },
-
-    /**
-     * Called when the fit-to-page checkbox is clicked. Updates the print
-     * ticket.
-     * @private
-     */
-    onFitToPageCheckboxClick_: function() {
-      this.fitToPageTicketItem_.updateValue(this.fitToPageCheckbox_.checked);
-    },
-
-    /**
-     * Called when the duplex checkbox is clicked. Updates the print ticket.
-     * @private
-     */
-    onDuplexCheckboxClick_: function() {
-      this.duplexTicketItem_.updateValue(this.duplexCheckbox_.checked);
-    },
-
-    /**
-     * Called when the print CSS backgrounds checkbox is clicked. Updates the
-     * print ticket store.
-     * @private
-     */
-    onCssBackgroundCheckboxClick_: function() {
-      this.cssBackgroundTicketItem_.updateValue(
-          this.cssBackgroundCheckbox_.checked);
-    },
-
-    /**
-     * Called when the print selection only is clicked. Updates the
-     * print ticket store.
-     * @private
-     */
-    onSelectionOnlyCheckboxClick_: function() {
-      this.selectionOnlyTicketItem_.updateValue(
-          this.selectionOnlyCheckbox_.checked);
-    },
-
-    /**
-     * Called when the duplex ticket item has changed. Updates the duplex
-     * checkbox.
-     * @private
-     */
-    onDuplexChange_: function() {
-      this.duplexCheckbox_.checked = this.duplexTicketItem_.getValue();
-      this.updateUiStateInternal();
-    },
-
-    /**
-     * Called when the fit-to-page ticket item has changed. Updates the
-     * fit-to-page checkbox.
-     * @private
-     */
-    onFitToPageChange_: function() {
-      this.fitToPageCheckbox_.checked = this.fitToPageTicketItem_.getValue();
-      this.updateUiStateInternal();
-    },
-
-    /**
-     * Called when the CSS background ticket item has changed. Updates the
-     * CSS background checkbox.
-     * @private
-     */
-    onCssBackgroundChange_: function() {
-      this.cssBackgroundCheckbox_.checked =
-          this.cssBackgroundTicketItem_.getValue();
-      this.updateUiStateInternal();
-    },
-
-    /**
-     * Called when the print selection only ticket item has changed. Updates the
-     * CSS background checkbox.
-     * @private
-     */
-    onSelectionOnlyChange_: function() {
-      this.selectionOnlyCheckbox_.checked =
-          this.selectionOnlyTicketItem_.getValue();
-      this.updateUiStateInternal();
-    },
-
-    /**
-     * Called when the header-footer ticket item has changed. Updates the
-     * header-footer checkbox.
-     * @private
-     */
-    onHeaderFooterChange_: function() {
-      this.headerFooterCheckbox_.checked =
-          this.headerFooterTicketItem_.getValue();
-      this.updateUiStateInternal();
-    },
   };
 
   // Export