From 87aa0cfce29b8f45f15ebd513a11ed2df1c52104 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 7 Aug 2015 14:34:27 -0700 Subject: [PATCH 1/2] [Menus] Dismiss menu when clicked Addresses WTD-1506 (context menu does not disappear after action is chosen); listen for click events on the menu itself and dismiss the menu when these occur. --- .../src/actions/ContextMenuAction.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/platform/representation/src/actions/ContextMenuAction.js b/platform/representation/src/actions/ContextMenuAction.js index 390531053e..22e610bc8d 100644 --- a/platform/representation/src/actions/ContextMenuAction.js +++ b/platform/representation/src/actions/ContextMenuAction.js @@ -31,7 +31,8 @@ define( var MENU_TEMPLATE = "" + "", dismissExistingMenu; @@ -48,7 +49,7 @@ define( * should be performed */ function ContextMenuAction($compile, $document, $window, $rootScope, actionContext) { - + function perform() { var winDim = [$window.innerWidth, $window.innerHeight], eventCoors = [actionContext.event.pageX, actionContext.event.pageY], @@ -62,7 +63,7 @@ define( // Remove the context menu function dismiss() { menu.remove(); - body.off("click", dismiss); + body.off("mousedown", dismiss); dismissExistingMenu = undefined; } @@ -86,18 +87,19 @@ define( "go-up": goUp, "context-menu-holder": true }; + scope.dismiss = dismiss; // Create the context menu menu = $compile(MENU_TEMPLATE)(scope); // Add the menu to the body body.append(menu); - + // Stop propagation so that clicks on the menu do not close the menu menu.on('mousedown', function (event) { event.stopPropagation(); }); - + // Dismiss the menu when body is clicked elsewhere // ('mousedown' because 'click' breaks left-click context menus) body.on('mousedown', dismiss); @@ -105,7 +107,7 @@ define( // Don't launch browser's context menu actionContext.event.preventDefault(); } - + return { perform: perform }; @@ -113,4 +115,4 @@ define( return ContextMenuAction; } -); \ No newline at end of file +); From 2d5ec97dc381d9aeae9d78c89c2cda4d3f567cb6 Mon Sep 17 00:00:00 2001 From: Victor Woeltjen Date: Fri, 7 Aug 2015 14:39:59 -0700 Subject: [PATCH 2/2] [Menu] Listen to element directly Add listener to menu element directly instead of using ng-click to aid in testing (and for consistency with related listeners.) WTD-1506. --- .../src/actions/ContextMenuAction.js | 3 +- .../test/actions/ContextMenuActionSpec.js | 31 ++++++++++++++++--- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/platform/representation/src/actions/ContextMenuAction.js b/platform/representation/src/actions/ContextMenuAction.js index 22e610bc8d..67b48536c3 100644 --- a/platform/representation/src/actions/ContextMenuAction.js +++ b/platform/representation/src/actions/ContextMenuAction.js @@ -31,7 +31,6 @@ define( var MENU_TEMPLATE = "" + "", @@ -87,7 +86,6 @@ define( "go-up": goUp, "context-menu-holder": true }; - scope.dismiss = dismiss; // Create the context menu menu = $compile(MENU_TEMPLATE)(scope); @@ -103,6 +101,7 @@ define( // Dismiss the menu when body is clicked elsewhere // ('mousedown' because 'click' breaks left-click context menus) body.on('mousedown', dismiss); + menu.on('click', dismiss); // Don't launch browser's context menu actionContext.event.preventDefault(); diff --git a/platform/representation/test/actions/ContextMenuActionSpec.js b/platform/representation/test/actions/ContextMenuActionSpec.js index 73b877ddc3..03298162b4 100644 --- a/platform/representation/test/actions/ContextMenuActionSpec.js +++ b/platform/representation/test/actions/ContextMenuActionSpec.js @@ -69,7 +69,7 @@ define( mockCompiledTemplate.andReturn(mockMenu); mockDocument.find.andReturn(mockBody); mockRootScope.$new.andReturn(mockScope); - + mockActionContext = {key: 'menu', domainObject: mockDomainObject, event: mockEvent}; action = new ContextMenuAction( @@ -118,9 +118,9 @@ define( it("removes a menu when body is clicked", function () { // Show the menu action.perform(); - + // Verify precondition - expect(mockBody.off).not.toHaveBeenCalled(); + expect(mockBody.remove).not.toHaveBeenCalled(); // Find and fire body's mousedown listener mockBody.on.calls.forEach(function (call) { @@ -133,8 +133,29 @@ define( expect(mockMenu.remove).toHaveBeenCalled(); // Listener should have been detached from body - expect(mockBody.off).toHaveBeenCalled(); + expect(mockBody.off).toHaveBeenCalledWith( + 'mousedown', + jasmine.any(Function) + ); + }); + + it("removes a menu when it is clicked", function () { + // Show the menu + action.perform(); + + // Verify precondition + expect(mockMenu.remove).not.toHaveBeenCalled(); + + // Find and fire body's mousedown listener + mockMenu.on.calls.forEach(function (call) { + if (call.args[0] === 'click') { + call.args[1](); + } + }); + + // Menu should have been removed + expect(mockMenu.remove).toHaveBeenCalled(); }); }); } -); \ No newline at end of file +);