From f9acf4627e2c85150f1c0001d4df4ade41cbb458 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 25 Sep 2015 12:23:28 +0200 Subject: [PATCH] Fix sidebar interaction - Clicking a file row or selecting it will open the sidebar. - When sidebar is open, its contents update with the last selection. - Dragging doesn't open the sidebar but does update its contents if it was open already. - Switching folders closes the sidebar. - Close sidebar when highlighted file got deleted/removed from list --- apps/files/js/filelist.js | 53 ++++++++++++++++++----------- apps/files/js/files.js | 21 +++++------- apps/files/tests/js/filelistSpec.js | 42 ++++++++++++++++++++--- 3 files changed, 79 insertions(+), 37 deletions(-) diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index e4a7aadd60..82d510f900 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')); + } }, /** @@ -1305,8 +1309,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); }, @@ -1513,11 +1519,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(); @@ -1527,6 +1534,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 4fdc9eb211..9ab7609cc4 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); + if (!$selectedFiles.length) { + $selectedFiles = $(this); } - else{ - $(this).fadeTo(250, 1); - } - $('#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 c05e7c3721..b3d85cf08f 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() {