Fix for destroying specific contextmenu, added unit test. Fixes #397

swisnl · Aug 26, 2016 · e738733 · e738733
1 parent 98012af
commit e738733
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 12 deletions.
diff --git a/dist/jquery.contextMenu.js b/dist/jquery.contextMenu.js
@@ -12,7 +12,7 @@
  *   MIT License http://www.opensource.org/licenses/mit-license
  *   GPL v3 http://opensource.org/licenses/GPL-3.0
  *
- * Date: 2016-07-17T19:45:35.567Z
+ * Date: 2016-08-26T11:25:39.329Z
  */
 
 (function (factory) {
@@ -1469,6 +1469,7 @@
 
     // manage contextMenu instances
     $.contextMenu = function (operation, options) {
+
         if (typeof operation !== 'string') {
             options = operation;
             operation = 'create';
@@ -1492,7 +1493,7 @@
             // you never know what they throw at you...
             $context = $(o.context).first();
             o.context = $context.get(0);
-            _hasContext = o.context !== document;
+            _hasContext = !$(o.context).is(document);
         }
 
         switch (operation) {
@@ -1591,7 +1592,9 @@
                     // get proper options
                     var context = o.context;
                     $.each(menus, function (ns, o) {
-                        if (o.context !== context) {
+
+                        // Is this menu equest to the context called from
+                        if (!$(context).is(o.selector)) {
                             return true;
                         }
 

diff --git a/dist/jquery.contextMenu.min.js b/dist/jquery.contextMenu.min.js
diff --git a/dist/jquery.contextMenu.min.js.map b/dist/jquery.contextMenu.min.js.map
diff --git a/src/jquery.contextMenu.js b/src/jquery.contextMenu.js
@@ -1469,6 +1469,7 @@
 
     // manage contextMenu instances
     $.contextMenu = function (operation, options) {
+
         if (typeof operation !== 'string') {
             options = operation;
             operation = 'create';
@@ -1492,7 +1493,7 @@
             // you never know what they throw at you...
             $context = $(o.context).first();
             o.context = $context.get(0);
-            _hasContext = o.context !== document;
+            _hasContext = !$(o.context).is(document);
         }
 
         switch (operation) {
@@ -1591,7 +1592,9 @@
                     // get proper options
                     var context = o.context;
                     $.each(menus, function (ns, o) {
-                        if (o.context !== context) {
+
+                        // Is this menu equest to the context called from
+                        if (!$(context).is(o.selector)) {
                             return true;
                         }
 

diff --git a/test/unit/test-events.js b/test/unit/test-events.js
@@ -6,7 +6,10 @@ var menuRuntime = null;
 function testQUnit(name, itemClickEvent, triggerEvent) {
   QUnit.module(name);
   // before each test
-  function createContextMenu(items) {
+  function createContextMenu(items, classname) {
+    if(typeof(classname) == 'undefined'){
+      classname = 'context-menu';
+    }
     var $fixture = $('#qunit-fixture');
 
     // ensure `#qunit-fixture` exists when testing with karma runner
@@ -15,7 +18,7 @@ function testQUnit(name, itemClickEvent, triggerEvent) {
       $fixture = $('#qunit-fixture');
     }
 
-    $fixture.append("<div class='context-menu'>right click me!</div>");
+    $fixture.append("<div class='" + classname + "'>right click me!</div>");
 
     if(!items){
       items = {
@@ -25,7 +28,7 @@ function testQUnit(name, itemClickEvent, triggerEvent) {
     }
 
     $.contextMenu({
-      selector: '.context-menu',
+      selector: '.' + classname,
       events: {
         show: function(opt) {
           menuRuntime = opt;
@@ -130,6 +133,34 @@ function testQUnit(name, itemClickEvent, triggerEvent) {
   });
 
 
+  QUnit.test('do not open context menu with no visible items', function(assert) {
+    createContextMenu({
+      copy: {name: 'Copy', icon: 'copy'},
+      paste: {name: 'Paste', icon: 'paste'}
+    });
+    createContextMenu({
+      copy: {name: 'Copy', icon: 'copy'},
+      paste: {name: 'Paste', icon: 'paste'}
+    }, 'context-menu-two');
+
+    $(".context-menu").contextMenu();
+    $(".context-menu").contextMenu('hide');
+    $(".context-menu-two").contextMenu();
+    $(".context-menu-two").contextMenu('hide');
+
+    assert.equal(menuOpenCounter, 2, 'contextMenu was opened twice');
+
+    $(".context-menu-two").contextMenu('destroy');
+
+    $(".context-menu").contextMenu();
+    $(".context-menu").contextMenu('hide');
+    $(".context-menu-two").contextMenu();
+    $(".context-menu-two").contextMenu('hide');
+
+    assert.equal(menuOpenCounter, 3, 'destroyed contextMenu was not opened');
+
+    destroyContextMenuAndCleanup();
+  });
 
   QUnit.test('do not open context menu with no visible items', function(assert) {
     createContextMenu({
@@ -191,4 +222,4 @@ function testQUnit(name, itemClickEvent, triggerEvent) {
 };
 
 testQUnit('contextMenu events', '', 'mouseup')
-testQUnit('contextMenu events - click handler', 'click', 'click')
+testQUnit('contextMenu events - click handler', 'click', 'click')