Print Preview: Refactoring layout mode related logic to LayoutSettings class.

chrome/browser/resources/print_preview/print_preview.js

Index: chrome/browser/resources/print_preview/print_preview.js
diff --git a/chrome/browser/resources/print_preview/print_preview.js b/chrome/browser/resources/print_preview/print_preview.js
index e26fcd89b427de43a04d3abaf974c8a4807209a2..0ee82e28e010fe989d2768a6344804349343726a 100644
--- a/chrome/browser/resources/print_preview/print_preview.js
+++ b/chrome/browser/resources/print_preview/print_preview.js
@@ -45,6 +45,9 @@ var pageSettings;
 // Object holding all the copies related settings.
 var copiesSettings;
 
+// Object holding all the layout related settings.
+var layoutSettings;
+
 // True if the user has click 'Advanced...' in order to open the system print
 // dialog.
 var showingSystemDialog = false;
@@ -80,8 +83,10 @@ function onLoad() {
 
   pageSettings = print_preview.PageSettings.getInstance();
   copiesSettings = print_preview.CopiesSettings.getInstance();
+  layoutSettings = print_preview.LayoutSettings.getInstance();
   pageSettings.addEventListeners();
   copiesSettings.addEventListeners();
+  layoutSettings.addEventListeners();
 
   showLoadingAnimation();
   chrome.send('getDefaultPrinter');
@@ -92,8 +97,6 @@ function onLoad() {
  */
 function addEventListeners() {
   // Controls that require preview rendering.
-  $('landscape').onclick = onLayoutModeToggle;
-  $('portrait').onclick = onLayoutModeToggle;
   $('printer-list').onchange = updateControlsWithSelectedPrinterCapabilities;
 
   // Controls that do not require preview rendering.
@@ -108,8 +111,6 @@ function removeEventListeners() {
   clearTimeout(pageSettings.timerId_);
 
   // Controls that require preview rendering
-  $('landscape').onclick = null;
-  $('portrait').onclick = null;
   $('printer-list').onchange = null;
 
   // Controls that don't require preview rendering.
@@ -245,7 +246,7 @@ function updateWithPrinterCapabilities(settingInfo) {
   var disableColorOption = settingInfo.disableColorOption;
   var disableCopiesOption = settingInfo.disableCopiesOption;
   var setColorAsDefault = settingInfo.setColorAsDefault;
-  var disableLandscapeOption = settingInfo.disableLandscapeOption;
+  var disableLayoutOption = settingInfo.disableLandscapeOption;

[Evan Stade, 2011/07/13 23:42:40]

I don't see much point in stashing this into a local var

[dpapad, 2011/07/14 00:45:15]

Done.

   var setDuplexAsDefault = settingInfo.setDuplexAsDefault;
   var color = $('color');
   var bw = $('bw');
@@ -259,11 +260,8 @@ function updateWithPrinterCapabilities(settingInfo) {
     $('hr-before-copies').classList.add('invisible');
   }
 
-  if (disableLandscapeOption) {
-    fadeOutElement($('landscape-option'));
-  } else {
-    fadeInElement($('landscape-option'));
-  }
+  disableLayoutOption ? fadeOutElement(layoutSettings.layoutOption) :

[Evan Stade, 2011/07/13 23:42:40]

layoutSettings.enableLayoutOption(!disableLayoutOption);

fade in/out logic should be hidden

[dpapad, 2011/07/14 00:45:15]

Done.

+      fadeInElement(layoutSettings.layoutOption);
 
   disableColorOption ? fadeOutElement(colorOptions) :
       fadeInElement(colorOptions);
@@ -303,15 +301,6 @@ function finishedCloudPrinting() {
 }
 
 /**
- * Checks whether the preview layout setting is set to 'landscape' or not.
- *
- * @return {boolean} true if layout is 'landscape'.
- */
-function isLandscape() {
-  return $('landscape').checked;
-}
-
-/**
  * Checks whether the preview color setting is set to 'color' or not.
  *
  * @return {boolean} true if color is 'color'.
@@ -355,7 +344,7 @@ function getSettingsJSON() {
                   'duplex': getDuplexMode(),
                   'copies': copiesSettings.numberOfCopies,
                   'collate': isCollated(),
-                  'landscape': isLandscape(),
+                  'landscape': layoutSettings.isLandscape(),
                   'color': isColor(),
                   'printToPDF': printToPDF};
   var printerList = $('printer-list');
@@ -768,7 +757,7 @@ function onPDFLoad() {
   hideLoadingAnimation();
 
   if (!previewModifiable)
-    fadeOutElement($('landscape-option'));
+    fadeOutElement(layoutSettings.layoutOption);

[Evan Stade, 2011/07/13 23:42:40]

attach PDFLoaded handler in layoutSettings to do this.

[dpapad, 2011/07/14 00:45:15]

Done.

   cr.dispatchSimpleEvent(document, 'PDFLoaded');
 }
 
@@ -954,19 +943,6 @@ function updatePrintSummary() {
 }
 
 /**
- * When the user switches printing orientation mode the page field selection is
- * reset to "all pages selected". After the change the number of pages will be
- * different and currently selected page numbers might no longer be valid.
- * Even if they are still valid the content of these pages will be different.
- */
-function onLayoutModeToggle() {
-  // If the chosen layout is same as before, nothing needs to be done.
-  if (printSettings.isLandscape == isLandscape())
-    return;
-  setDefaultValuesAndRegeneratePreview();
-}
-
-/**
  * Sets the default values and sends a request to regenerate preview data.
  */
 function setDefaultValuesAndRegeneratePreview() {
@@ -987,6 +963,6 @@ function PrintSettings() {
  */
 PrintSettings.prototype.save = function() {
   this.deviceName = getSelectedPrinterName();
-  this.isLandscape = isLandscape();
+  this.isLandscape = layoutSettings.isLandscape();
 }