From 676bf1c7fd1cdb3398126bfcc15ac0bb7628e3ab Mon Sep 17 00:00:00 2001
From: jdescottes <julian.descottes@gmail.com>
Date: Sun, 9 Feb 2014 21:49:08 +0100
Subject: [PATCH] Fix : Settings drawer close usability bug

+ Settings drawer could not be closed when clicking above or below its
container. This has been fixed by changin the logic used for determining
if the click was inside/outside of the Settings drawer.

+ Added DOM utility to compensate for the limitations of JQuery
contains...
---
 js/controller/settings/SettingsController.js | 52 +++++++++++---------
 js/utils/Dom.js                              | 29 +++++++++++
 piskel-script-list.js                        |  1 +
 3 files changed, 59 insertions(+), 23 deletions(-)
 create mode 100644 js/utils/Dom.js

diff --git a/js/controller/settings/SettingsController.js b/js/controller/settings/SettingsController.js
index 1f0fe70d..bc1f0318 100644
--- a/js/controller/settings/SettingsController.js
+++ b/js/controller/settings/SettingsController.js
@@ -1,5 +1,5 @@
 (function () {
-  var ns = $.namespace("pskl.controller.settings");
+  var ns = $.namespace('pskl.controller.settings');
 
   var settings = {
     'user' : {
@@ -29,9 +29,9 @@
 
   ns.SettingsController = function (piskelController) {
     this.piskelController = piskelController;
-    this.drawerContainer = document.getElementById("drawer-container");
+    this.drawerContainer = document.getElementById('drawer-container');
     this.settingsContainer = $('[data-pskl-controller=settings]');
-    this.expanded = false;
+    this.isExpanded = false;
     this.currentSetting = null;
   };
 
@@ -39,27 +39,33 @@
    * @public
    */
   ns.SettingsController.prototype.init = function() {
-    // Expand drawer when clicking 'Settings' tab.
-    $('[data-setting]').click(function(evt) {
-      var el = evt.originalEvent.currentTarget;
-      var setting = el.getAttribute("data-setting");
-      if (this.currentSetting != setting) {
-        this.loadSetting(setting);
-      } else {
-        this.closeDrawer();
-      }
-    }.bind(this));
-
-    $('body').click(function (evt) {
-      var isInSettingsContainer = $.contains(this.settingsContainer.get(0), evt.target);
-      if (this.expanded && !isInSettingsContainer) {
-        this.closeDrawer();
-      }
-    }.bind(this));
-
+    $('[data-setting]').click(this.onSettingIconClick.bind(this));
+    $('body').click(this.onBodyClick.bind(this));
     $.subscribe(Events.CLOSE_SETTINGS_DRAWER, this.closeDrawer.bind(this));
   };
 
+  ns.SettingsController.prototype.onSettingIconClick = function (evt) {
+    var el = evt.originalEvent.currentTarget;
+    var setting = el.getAttribute('data-setting');
+    if (this.currentSetting != setting) {
+      this.loadSetting(setting);
+    } else {
+      this.closeDrawer();
+    }
+  };
+
+  ns.SettingsController.prototype.onBodyClick = function (evt) {
+    var target = evt.target;
+
+    var isInDrawerContainer = pskl.utils.Dom.isParent(target, this.drawerContainer);
+    var isInSettingsIcon = target.getAttribute('data-setting');
+    var isInSettingsContainer = isInDrawerContainer || isInSettingsIcon;
+
+    if (this.isExpanded && !isInSettingsContainer) {
+      this.closeDrawer();
+    }
+  };
+
   ns.SettingsController.prototype.loadSetting = function (setting) {
     this.drawerContainer.innerHTML = pskl.utils.Template.get(settings[setting].template);
     (new settings[setting].controller(this.piskelController)).init();
@@ -69,7 +75,7 @@
     $('.' + SEL_SETTING_CLS).removeClass(SEL_SETTING_CLS);
     $('[data-setting='+setting+']').addClass(SEL_SETTING_CLS);
 
-    this.expanded = true;
+    this.isExpanded = true;
     this.currentSetting = setting;
   };
 
@@ -77,7 +83,7 @@
     this.settingsContainer.removeClass(EXP_DRAWER_CLS);
     $('.' + SEL_SETTING_CLS).removeClass(SEL_SETTING_CLS);
 
-    this.expanded = false;
+    this.isExpanded = false;
     this.currentSetting = null;
   };
 
diff --git a/js/utils/Dom.js b/js/utils/Dom.js
new file mode 100644
index 00000000..56818cea
--- /dev/null
+++ b/js/utils/Dom.js
@@ -0,0 +1,29 @@
+(function () {
+  var ns = $.namespace('pskl.utils');
+
+  ns.Dom = {
+    /**
+     * Check if a given HTML element is nested inside another
+     * @param  {HTMLElement}  node  Element to test
+     * @param  {HTMLElement}  parent Potential Ancestor for node
+     * @param  {Boolean}      excludeParent set to true if the parent should be excluded from potential matches
+     * @return {Boolean}      true if parent was found amongst the parentNode chain of node
+     */
+    isParent : function (node, parent, excludeParent) {
+      if (node && parent) {
+
+        if (excludeParent) {
+          node = node.parentNode;
+        }
+
+        while (node) {
+          if (node === parent) {
+            return true;
+          }
+          node = node.parentNode;
+        }
+      }
+      return false;
+    }
+  };
+})();
\ No newline at end of file
diff --git a/piskel-script-list.js b/piskel-script-list.js
index 468b7d77..27286eaa 100644
--- a/piskel-script-list.js
+++ b/piskel-script-list.js
@@ -18,6 +18,7 @@ exports.scripts = [
   "js/utils/core.js",
   "js/utils/UserAgent.js",
   "js/utils/CanvasUtils.js",
+  "js/utils/Dom.js",
   "js/utils/Math.js",
   "js/utils/FileUtils.js",
   "js/utils/FrameUtils.js",