Merge pull request #19365 from owncloud/files-detailsbar-interaction

Fix sidebar interaction

3 files changed, 79 insertions, 37 deletions

diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js
index 1b069530e69..b4cc71e2d3b 100644
--- a/apps/files/js/filelist.js
+++ b/apps/files/js/filelist.js
@@ -239,13 +239,6 @@
 
 			this.updateSearch();
 
-			this.$el.on('click', function(event) {
-				var $target = $(event.target);
-				// click outside file row ?
-				if (!$target.closest('tbody').length && !$target.closest('#app-sidebar').length) {
-					self._updateDetailsView(null);
-				}
-			});
 			this.$fileList.on('click','td.filename>a.name', _.bind(this._onClickFile, this));
 			this.$fileList.on('change', 'td.filename>.selectCheckBox', _.bind(this._onClickFileCheckbox, this));
 			this.$el.on('urlChanged', _.bind(this._onUrlChanged, this));
@@ -278,6 +271,9 @@
 			if (this._newButton) {
 				this._newButton.remove();
 			}
+			if (this._detailsView) {
+				this._detailsView.remove();
+			}
 			// TODO: also unregister other event handlers
 			this.fileActions.off('registerAction', this._onFileActionsUpdated);
 			this.fileActions.off('setDefault', this._onFileActionsUpdated);
@@ -307,7 +303,6 @@
 					permissions: OC.PERMISSION_READ,
 					actionHandler: function(fileName, context) {
 						self._updateDetailsView(fileName);
-						OC.Apps.showAppSidebar(self._detailsView.$el);
 					}
 				});
 			}
@@ -408,9 +403,14 @@
 					this._currentFileModel.off();
 				}
 				this._currentFileModel = null;
+				OC.Apps.hideAppSidebar(this._detailsView.$el);
 				return;
 			}
 
+			if (this._detailsView.$el.hasClass('disappear')) {
+				OC.Apps.showAppSidebar(this._detailsView.$el);
+			}
+
 			var $tr = this.findFileEl(fileName);
 			var model = this.getModelForFile($tr);
 
@@ -453,10 +453,10 @@
 		 * Selected/deselects the given file element and updated
 		 * the internal selection cache.
 		 *
-		 * @param $tr single file row element
-		 * @param state true to select, false to deselect
+		 * @param {Object} $tr single file row element
+		 * @param {bool} state true to select, false to deselect
 		 */
-		_selectFileEl: function($tr, state) {
+		_selectFileEl: function($tr, state, showDetailsView) {
 			var $checkbox = $tr.find('td.filename>.selectCheckBox');
 			var oldData = !!this._selectedFiles[$tr.data('id')];
 			var data;
@@ -475,11 +475,8 @@
 				delete this._selectedFiles[$tr.data('id')];
 				this._selectionSummary.remove(data);
 			}
-			if (this._selectionSummary.getTotal() === 1) {
+			if (this._detailsView && this._selectionSummary.getTotal() === 1 && !this._detailsView.$el.hasClass('disappear')) {
 				this._updateDetailsView(_.values(this._selectedFiles)[0].name);
-			} else {
-				// show nothing when multiple files are selected
-				this._updateDetailsView(null);
 			}
 			this.$el.find('.select-all').prop('checked', this._selectionSummary.getTotal() === this.files.length);
 		},
@@ -489,6 +486,9 @@
 		 */
 		_onClickFile: function(event) {
 			var $tr = $(event.target).closest('tr');
+			if ($tr.hasClass('dragging')) {
+				return;
+			}
 			if (this._allowSelection && (event.ctrlKey || event.shiftKey)) {
 				event.preventDefault();
 				if (event.shiftKey) {
@@ -552,9 +552,13 @@
 		 */
 		_onClickFileCheckbox: function(e) {
 			var $tr = $(e.target).closest('tr');
-			this._selectFileEl($tr, !$tr.hasClass('selected'));
+			var state = !$tr.hasClass('selected');
+			this._selectFileEl($tr, state);
 			this._lastChecked = $tr;
 			this.updateSelectionSummary();
+			if (state) {
+				this._updateDetailsView($tr.attr('data-file'));
+			}
 		},
 
 		/**
@@ -1320,8 +1324,10 @@
 					sortdirection: this._sortDirection
 				}
 			});
-			// close sidebar
-			this._updateDetailsView(null);
+			if (this._detailsView) {
+				// close sidebar
+				this._updateDetailsView(null);
+			}
 			var callBack = this.reloadCallback.bind(this);
 			return this._reloadCall.then(callBack, callBack);
 		},
@@ -1528,11 +1534,12 @@
 		remove: function(name, options){
 			options = options || {};
 			var fileEl = this.findFileEl(name);
+			var fileId = fileEl.data('id');
 			var index = fileEl.index();
 			if (!fileEl.length) {
 				return null;
 			}
-			if (this._selectedFiles[fileEl.data('id')]) {
+			if (this._selectedFiles[fileId]) {
 				// remove from selection first
 				this._selectFileEl(fileEl, false);
 				this.updateSelectionSummary();
@@ -1542,6 +1549,14 @@
 				fileEl.find('td.filename').draggable('destroy');
 			}
 			this.files.splice(index, 1);
+			if (this._currentFileModel && this._currentFileModel.get('id') === fileId) {
+				// Note: in the future we should call destroy() directly on the model
+				// and the model will take care of the deletion.
+				// Here we only trigger the event to notify listeners that
+				// the file was removed.
+				this._currentFileModel.trigger('destroy');
+				this._updateDetailsView(null);
+			}
 			fileEl.remove();
 			// TODO: improve performance on batch update
 			this.isEmpty = !this.files.length;
diff --git a/apps/files/js/files.js b/apps/files/js/files.js
index 4fdc9eb2110..9ab7609cc40 100644
--- a/apps/files/js/files.js
+++ b/apps/files/js/files.js
@@ -356,7 +356,7 @@ var createDragShadow = function(event) {
 	var isDragSelected = $(event.target).parents('tr').find('td input:first').prop('checked');
 	if (!isDragSelected) {
 		//select dragged file
-		FileList._selectFileEl($(event.target).parents('tr:first'), true);
+		FileList._selectFileEl($(event.target).parents('tr:first'), true, false);
 	}
 
 	// do not show drag shadow for too many files
@@ -365,7 +365,7 @@ var createDragShadow = function(event) {
 
 	if (!isDragSelected && selectedFiles.length === 1) {
 		//revert the selection
-		FileList._selectFileEl($(event.target).parents('tr:first'), false);
+		FileList._selectFileEl($(event.target).parents('tr:first'), false, false);
 	}
 
 	// build dragshadow
@@ -413,22 +413,17 @@ var dragOptions={
 	cursor: 'move',
 	start: function(event, ui){
 		var $selectedFiles = $('td.filename input:checkbox:checked');
-		if($selectedFiles.length > 1){
-			$selectedFiles.parents('tr').fadeTo(250, 0.2);
-		}
-		else{
-			$(this).fadeTo(250, 0.2);
+		if (!$selectedFiles.length) {
+			$selectedFiles = $(this);
 		}
+		$selectedFiles.closest('tr').fadeTo(250, 0.2).addClass('dragging');
 	},
 	stop: function(event, ui) {
 		var $selectedFiles = $('td.filename input:checkbox:checked');
-		if($selectedFiles.length > 1){
-			$selectedFiles.parents('tr').fadeTo(250, 1);
-		}
-		else{
-			$(this).fadeTo(250, 1);
+		if (!$selectedFiles.length) {
+			$selectedFiles = $(this);
 		}
-		$('#fileList tr td.filename').addClass('ui-draggable');
+		$selectedFiles.closest('tr').fadeTo(250, 1).removeClass('dragging');
 	}
 };
 // sane browsers support using the distance option
diff --git a/apps/files/tests/js/filelistSpec.js b/apps/files/tests/js/filelistSpec.js
index c05e7c37214..b3d85cf08fa 100644
--- a/apps/files/tests/js/filelistSpec.js
+++ b/apps/files/tests/js/filelistSpec.js
@@ -135,6 +135,9 @@ describe('OCA.Files.FileList tests', function() {
 	});
 	afterEach(function() {
 		testFiles = undefined;
+		if (fileList) {
+			fileList.destroy();
+		}
 		fileList = undefined;
 
 		notificationStub.restore();
@@ -1881,8 +1884,9 @@ describe('OCA.Files.FileList tests', function() {
 	describe('Details sidebar', function() {
 		beforeEach(function() {
 			fileList.setFiles(testFiles);
+			fileList.showDetailsView('Two.jpg');
 		});
-		it('Clicking on a file row will trigger file action if no details view configured', function() {
+		it('triggers file action when clicking on row if no details view configured', function() {
 			fileList._detailsView = null;
 			var updateDetailsViewStub = sinon.stub(fileList, '_updateDetailsView');
 			var actionStub = sinon.stub();
@@ -1904,7 +1908,7 @@ describe('OCA.Files.FileList tests', function() {
 			expect(updateDetailsViewStub.notCalled).toEqual(true);
 			updateDetailsViewStub.restore();
 		});
-		it('Clicking on a file row will trigger details sidebar', function() {
+		it('highlights current file when clicked and updates sidebar', function() {
 			fileList.fileActions.setDefault('text/plain', 'Test');
 			var $tr = fileList.findFileEl('One.txt');
 			$tr.find('td.filename>a.name').click();
@@ -1912,14 +1916,34 @@ describe('OCA.Files.FileList tests', function() {
 
 			expect(fileList._detailsView.getFileInfo().id).toEqual(1);
 		});
-		it('Clicking outside to deselect a file row will trigger details sidebar', function() {
+		it('keeps the last highlighted file when clicking outside', function() {
 			var $tr = fileList.findFileEl('One.txt');
 			$tr.find('td.filename>a.name').click();
 
 			fileList.$el.find('tfoot').click();
 
-			expect($tr.hasClass('highlighted')).toEqual(false);
-			expect(fileList._detailsView.getFileInfo()).toEqual(null);
+			expect($tr.hasClass('highlighted')).toEqual(true);
+			expect(fileList._detailsView.getFileInfo().id).toEqual(1);
+		});
+		it('keeps the last highlighted file when unselecting file using checkbox', function() {
+			var $tr = fileList.findFileEl('One.txt');
+			$tr.find('input:checkbox').click();
+			expect($tr.hasClass('highlighted')).toEqual(true);
+			$tr.find('input:checkbox').click();
+
+			expect($tr.hasClass('highlighted')).toEqual(true);
+			expect(fileList._detailsView.getFileInfo().id).toEqual(1);
+		});
+		it('closes sidebar whenever the currently highlighted file was removed from the list', function() {
+			var $tr = fileList.findFileEl('One.txt');
+			$tr.find('td.filename>a.name').click();
+			expect($tr.hasClass('highlighted')).toEqual(true);
+
+			expect(fileList._detailsView.getFileInfo().id).toEqual(1);
+
+			expect($('#app-sidebar').hasClass('disappear')).toEqual(false);
+			fileList.remove('One.txt');
+			expect($('#app-sidebar').hasClass('disappear')).toEqual(true);
 		});
 		it('returns the currently selected model instance when calling getModelForFile', function() {
 			var $tr = fileList.findFileEl('One.txt');
@@ -1935,6 +1959,14 @@ describe('OCA.Files.FileList tests', function() {
 			var model3 = fileList.getModelForFile($tr);
 			expect(model3).toEqual(model1);
 		});
+		it('closes the sidebar when switching folders', function() {
+			var $tr = fileList.findFileEl('One.txt');
+			$tr.find('td.filename>a.name').click();
+
+			expect($('#app-sidebar').hasClass('disappear')).toEqual(false);
+			fileList.changeDirectory('/another');
+			expect($('#app-sidebar').hasClass('disappear')).toEqual(true);
+		});
 	});
 	describe('File actions', function() {
 		it('Clicking on a file name will trigger default action', function() {