Fixed file actions menu to close when reclicking trigger

FileActionsMenu is now a backbone view.
The trigger and highlight handling is now done in the
FileActions.showMenu() method using events.

apps/files/js/fileactions.js

@@ -48,13 +48,6 @@
 
 		_fileActionTriggerTemplate: null,
 
-		/**
-		 * File actions menu
-		 *
-		 * @type OCA.Files.FileActionsMenu
-		 */
-		_menu: null,
-
 		/**
 		 * @private
 		 */
@@ -333,8 +326,20 @@
 		 * @param {OCA.Files.FileActionContext} context rendering context
 		 */
 		_showMenu: function(fileName, context) {
-			this._menu = new OCA.Files.FileActionsMenu();
+			var menu;
-			this._menu.showAt(context);
+			var $trigger = context.$file.closest('tr').find('.fileactions .action-menu');
+			$trigger.addClass('open');
+
+			menu = new OCA.Files.FileActionsMenu();
+			menu.$el.on('afterHide', function() {
+				context.$file.removeClass('mouseOver');
+				$trigger.removeClass('open');
+				menu.remove();
+			});
+
+			context.$file.addClass('mouseOver');
+			context.$file.find('td.filename').append(menu.$el);
+			menu.show(context);
 		},
 
 		/**
@@ -378,14 +383,19 @@
 					a: null
 				},
 				function(event) {
+					event.stopPropagation();
+					event.preventDefault();
+
+					if ($actionEl.hasClass('open')) {
+						return;
+					}
+
 					var $file = $(event.target).closest('tr');
 					if ($file.hasClass('busy')) {
 						return;
 					}
 					var currentFile = $file.find('td.filename');
 					var fileName = $file.attr('data-file');
-					event.stopPropagation();
-					event.preventDefault();
 
 					context.fileActions.currentFile = currentFile;
 					// also set on global object for legacy apps

apps/files/js/fileactionsmenu.js

@@ -24,13 +24,9 @@
 	 * @constructs FileActionsMenu
 	 * @memberof OCA.Files
 	 */
-	var FileActionsMenu = function() {
+	var FileActionsMenu = OC.Backbone.View.extend({
-		this.initialize();
+		tagName: 'div',
-	};
+		className: 'fileActionsMenu bubble hidden open menu',
-
-	FileActionsMenu.prototype = {
-		$el: null,
-		_template: null,
 
 		/**
 		 * Current context
@@ -39,19 +35,15 @@
 		 */
 		_context: null,
 
-		/**
+		events: {
-		 * @private
+			'click a.action': '_onClickAction'
-		 */
-		initialize: function() {
-			this.$el = $('<div class="fileActionsMenu bubble hidden open menu"></div>');
-			this._template = Handlebars.compile(TEMPLATE_MENU);
-
-			this.$el.on('click', 'a.action', _.bind(this._onClickAction, this));
-			this.$el.on('afterHide', _.bind(this._onHide, this));
 		},
 
-		destroy: function() {
+		template: function(data) {
-			this.$el.remove();
+			if (!OCA.Files.FileActionsMenu._TEMPLATE) {
+				OCA.Files.FileActionsMenu._TEMPLATE = Handlebars.compile(TEMPLATE_MENU);
+			}
+			return OCA.Files.FileActionsMenu._TEMPLATE(data);
 		},
 
 		/**
@@ -113,8 +105,7 @@
 				return item;
 			});
 
-			this.$el.empty();
+			this.$el.html(this.template({
-			this.$el.append(this._template({
 				items: items
 			}));
 		},
@@ -123,27 +114,17 @@
 		 * Displays the menu under the given element
 		 *
 		 * @param {OCA.Files.FileActionContext} context context
+		 * @param {Object} $trigger trigger element
 		 */
-		showAt: function(context) {
+		show: function(context) {
 			this._context = context;
 
 			this.render();
 			this.$el.removeClass('hidden');
 
-			context.$file.find('td.filename').append(this.$el);
-			context.$file.addClass('mouseOver');
-
 			OC.showMenu(null, this.$el);
-		},
-
-		/**
-		 * Whenever the menu is hidden
-		 */
-		_onHide: function() {
-			this._context.$file.removeClass('mouseOver');
-			this.destroy();
 		}
-	};
+	});
 
 	OCA.Files.FileActionsMenu = FileActionsMenu;
 

apps/files/tests/js/fileactionsSpec.js

@@ -192,10 +192,26 @@ describe('OCA.Files.FileActions tests', function() {
 			context = actionStub.getCall(0).args[1];
 			expect(context.dir).toEqual('/somepath');
 		});
-		it('shows actions menu when clicking the menu trigger', function() {
+		describe('actions menu', function() {
-			expect($tr.find('.menu').length).toEqual(0);
+			it('shows actions menu inside row when clicking the menu trigger', function() {
-			$tr.find('.action-menu').click();
+				expect($tr.find('td.filename .fileActionsMenu').length).toEqual(0);
-			expect($tr.find('.menu').length).toEqual(1);
+				$tr.find('.action-menu').click();
+				expect($tr.find('td.filename .fileActionsMenu').length).toEqual(1);
+			});
+			it('shows highlight on current row', function() {
+				$tr.find('.action-menu').click();
+				expect($tr.hasClass('mouseOver')).toEqual(true);
+			});
+			it('cleans up after hiding', function() {
+				var clock = sinon.useFakeTimers();
+				$tr.find('.action-menu').click();
+				expect($tr.find('.fileActionsMenu').length).toEqual(1);
+				OC.hideMenus();
+				// sliding animation
+				clock.tick(500);
+				expect($tr.hasClass('mouseOver')).toEqual(false);
+				expect($tr.find('.fileActionsMenu').length).toEqual(0);
+			});
 		});
 	});
 	describe('custom rendering', function() {

apps/files/tests/js/fileactionsmenuSpec.js

@@ -87,24 +87,17 @@ describe('OCA.Files.FileActionsMenu tests', function() {
 			dir: fileList.getCurrentDirectory()
 		};
 		menu = new OCA.Files.FileActionsMenu();
-		menu.showAt(menuContext);
+		menu.show(menuContext);
 	});
 	afterEach(function() {
 		fileActions = null;
 		fileList.destroy();
 		fileList = undefined;
-		menu.destroy();
+		menu.remove();
 		$('#dir, #permissions, #filestable').remove();
 	});
 
 	describe('rendering', function() {
-		it('displays menu in the row container', function() {
-			expect(menu.$el.closest('td.filename').length).toEqual(1);
-			expect($tr.find('.fileActionsMenu').length).toEqual(1);
-		});
-		it('highlights the row in the file list', function() {
-			expect($tr.hasClass('mouseOver')).toEqual(true);
-		});
 		it('renders dropdown actions in menu', function() {
 			var $action = menu.$el.find('a[data-action=Testdropdown]');
 			expect($action.length).toEqual(1);
@@ -200,7 +193,7 @@ describe('OCA.Files.FileActionsMenu tests', function() {
 				dir: fileList.getCurrentDirectory()
 			};
 			menu = new OCA.Files.FileActionsMenu();
-			menu.showAt(menuContext);
+			menu.show(menuContext);
 
 			menu.$el.find('.action-download').click();
 
@@ -233,7 +226,7 @@ describe('OCA.Files.FileActionsMenu tests', function() {
 				dir: '/anotherpath/there'
 			};
 			menu = new OCA.Files.FileActionsMenu();
-			menu.showAt(menuContext);
+			menu.show(menuContext);
 
 			menu.$el.find('.action-download').click();
 
@@ -266,7 +259,7 @@ describe('OCA.Files.FileActionsMenu tests', function() {
 				dir: '/somepath/dir'
 			};
 			menu = new OCA.Files.FileActionsMenu();
-			menu.showAt(menuContext);
+			menu.show(menuContext);
 
 			menu.$el.find('.action-delete').click();
 
@@ -276,16 +269,5 @@ describe('OCA.Files.FileActionsMenu tests', function() {
 			deleteStub.restore();
 		});
 	});
-	describe('hiding', function() {
-		beforeEach(function() {
-			menu.$el.trigger(new $.Event('afterHide'));
-		});
-		it('removes highlight on current row', function() {
-			expect($tr.hasClass('mouseOver')).toEqual(false);
-		});
-		it('destroys its DOM element on hide', function() {
-			expect($tr.find('.fileActionsMenu').length).toEqual(0);
-		});
-	});
 });
 

core/js/js.js

@@ -611,9 +611,14 @@ var OC={
 	 */
 	hideMenus: function(complete) {
 		if (OC._currentMenu) {
+			var lastMenu = OC._currentMenu;
 			OC._currentMenu.trigger(new $.Event('beforeHide'));
-			OC._currentMenu.slideUp(OC.menuSpeed, complete);
+			OC._currentMenu.slideUp(OC.menuSpeed, function() {
-			OC._currentMenu.trigger(new $.Event('afterHide'));
+				lastMenu.trigger(new $.Event('afterHide'));
+				if (complete) {
+					complete.apply(this, arguments);
+				}
+			});
 		}
 		OC._currentMenu = null;
 		OC._currentMenuToggle = null;