[MAC] Disable 'Import' button when visible checkboxes are unchecked.

chrome/browser/resources/options/import_data_overlay.js

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 cr.define('options', function() {
   var Page = cr.ui.pageManager.Page;
   var PageManager = cr.ui.pageManager.PageManager;
 
   /**
    * ImportDataOverlay class
@@ skipping 57 matching lines @@
       // Form controls are disabled until the profile list has been loaded.
       self.setAllControlsEnabled_(false);
     },
 
     /**
      * Sets the enabled and checked state of the commit button.
      * @private
      */
     validateCommitButton_: function() {
       var somethingToImport =
-          $('import-history').checked || $('import-favorites').checked ||
-          $('import-passwords').checked || $('import-search').checked ||
-          $('import-autofill-form-data').checked;
+          ($('import-history-with-label').style.display != 'none' &&

[Dan Beam, 2014/10/07 17:05:15]

i think asking the checkbox itself if it's > 0 width is better, imo

http://jsfiddle.net/b8j7ad82/

[Gaja, 2014/10/08 04:50:07]

I tried using '$(id).offsetWidth > 0', but during the launch of overlay,
$(id).offsetWidth > 0' value will be 'false' for all checkboxes regardless they
will be shown or not, and therefore the 'Import' button will be disabled even
though some checkboxes are checked. After any changes to drop-down options or
checkboxes, the button is being updated and works as intended.

[Gaja, 2014/10/08 05:02:48]

So, falling back to previous check, i.e $(id).style.display != 'none'. Does it
cause any other problem? Please let me know your opinion.

[Dan Beam, 2014/10/08 21:36:23]

well, if the markup ever changes or these checkboxes are hidden in any other way
it'd break this code.  what about:

  var el = $('import-' + type);
  return el.checked && getComputedStyle(el).display != 'none';

(note: the .checked condition comes first as it's faster)

[Gaja, 2014/10/10 05:19:30]

Done. Adding '-with-label' to 'el' and then passing to getComputedStyle, since
'display' property is being updated to its parent div element
<import-***-with-label>. (in updateCheckboxes_: function)

+           $('import-history').checked) ||
+          ($('import-favorites-with-label').style.display != 'none' &&
+           $('import-favorites').checked) ||
+          ($('import-passwords-with-label').style.display != 'none' &&
+           $('import-passwords').checked) ||
+          ($('import-search-with-label').style.display != 'none' &&
+           $('import-search').checked) ||
+          ($('import-autofill-form-data-with-label').style.display != 'none' &&
+           $('import-autofill-form-data').checked);

[Dan Beam, 2014/10/07 17:05:15]

helper function?

  function importable(type) {
    var id = 'import-' + type;
    return $(id).offsetWidth > 0 && $(id).checked;
  }

  var somethingToImport = importable('history') && importable('favorites') &&
  ...
                          

[Gaja, 2014/10/08 04:50:07]

Done. Thanks for the snippet.

       $('import-data-commit').disabled = !somethingToImport;
       $('import-choose-file').disabled = !$('import-favorites').checked;
     },
 
     /**
      * Sets the enabled state of all the checkboxes and the commit button.
      * @private
      */
     setAllControlsEnabled_: function(enabled) {
       var checkboxes =
@@ skipping 179 matching lines @@
     ImportDataOverlay.getInstance().validateCommitButton_();
 
     PageManager.showPageByName('importData');
   };
 
   // Export
   return {
     ImportDataOverlay: ImportDataOverlay
   };
 });