From 025b71d068bf530581693af67a31a56dca24beb1 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 1 Jul 2014 21:32:04 +0200 Subject: [PATCH] Use fileActionsReady to re-render sharing icons Whenever file actions are modified, either by registering new actions or when appending a new page of entries, the sharing app is now notified so it can correctly refresh the sharing icon status. Additionally, the core's loadIcons() method is also used to load the existing shares and also refresh the sharing icons afterwards. --- apps/files/js/fileactions.js | 5 +- apps/files/js/filelist.js | 35 +++++---- apps/files/tests/js/filelistSpec.js | 13 +++- apps/files_sharing/js/share.js | 90 ++++++++++++++---------- apps/files_sharing/tests/js/shareSpec.js | 8 +++ core/js/share.js | 11 ++- 6 files changed, 106 insertions(+), 56 deletions(-) diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js index fc7c9ccacef..e06d2912274 100644 --- a/apps/files/js/fileactions.js +++ b/apps/files/js/fileactions.js @@ -181,11 +181,12 @@ return; } this.currentFile = parent; + var $tr = parent.closest('tr'); var self = this; var actions = this.getActions(this.getCurrentMimeType(), this.getCurrentType(), this.getCurrentPermissions()); var file = this.getCurrentFile(); var nameLinks; - if (parent.closest('tr').data('renaming')) { + if ($tr.data('renaming')) { return; } @@ -278,7 +279,7 @@ } if (triggerEvent){ - fileList.$fileList.trigger(jQuery.Event("fileActionsReady", {fileList: fileList})); + fileList.$fileList.trigger(jQuery.Event("fileActionsReady", {fileList: fileList, $files: $tr})); } }, getCurrentFile: function () { diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 07e4e523e03..c7fccc5dd66 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -473,6 +473,7 @@ /** * Appends the next page of files into the table * @param animate true to animate the new elements + * @return array of DOM elements of the newly added files */ _nextPage: function(animate) { var index = this.$fileList.children().length, @@ -483,7 +484,7 @@ isAllSelected = this.isAllSelected(); if (index >= this.files.length) { - return; + return false; } while (count > 0 && index < this.files.length) { @@ -496,12 +497,17 @@ } if (animate) { tr.addClass('appear transparent'); - newTrs.push(tr); } + newTrs.push(tr); index++; count--; } + // trigger event for newly added rows + if (newTrs.length > 0) { + this.$fileList.trigger($.Event('fileActionsReady', {fileList: this, $files: newTrs})); + } + if (animate) { // defer, for animation window.setTimeout(function() { @@ -510,6 +516,7 @@ } }, 0); } + return newTrs; }, /** @@ -518,10 +525,16 @@ */ _onFileActionsUpdated: function() { var self = this; - this.$fileList.find('tr td.filename').each(function() { - self.fileActions.display($(this), false, self); + var $files = this.$fileList.find('tr'); + if (!$files.length) { + return; + } + + $files.each(function() { + self.fileActions.display($(this).find('td.filename'), false, self); }); - this.$fileList.trigger($.Event('fileActionsReady', {fileList: this})); + this.$fileList.trigger($.Event('fileActionsReady', {fileList: this, $files: $files})); + }, /** @@ -533,7 +546,6 @@ // detach to make adding multiple rows faster this.files = filesArray; - this.$fileList.detach(); this.$fileList.empty(); // clear "Select all" checkbox @@ -542,10 +554,7 @@ this.isEmpty = this.files.length === 0; this._nextPage(); - this.$el.find('thead').after(this.$fileList); - this.updateEmptyContent(); - this.$fileList.trigger($.Event('fileActionsReady', {fileList: this})); this.fileSummary.calculate(filesArray); @@ -1283,16 +1292,16 @@ // reinsert row self.files.splice(tr.index(), 1); tr.remove(); - self.add(fileInfo, {updateSummary: false, silent: true}); - self.$fileList.trigger($.Event('fileActionsReady', {fileList: self})); + tr = self.add(fileInfo, {updateSummary: false, silent: true}); + self.$fileList.trigger($.Event('fileActionsReady', {fileList: self, $files: $(tr)})); } }); } else { // add back the old file info when cancelled self.files.splice(tr.index(), 1); tr.remove(); - self.add(oldFileInfo, {updateSummary: false, silent: true}); - self.$fileList.trigger($.Event('fileActionsReady', {fileList: self})); + tr = self.add(oldFileInfo, {updateSummary: false, silent: true}); + self.$fileList.trigger($.Event('fileActionsReady', {fileList: self, $files: $(tr)})); } } catch (error) { input.attr('title', error); diff --git a/apps/files/tests/js/filelistSpec.js b/apps/files/tests/js/filelistSpec.js index 7a6c390f325..713cd5468d8 100644 --- a/apps/files/tests/js/filelistSpec.js +++ b/apps/files/tests/js/filelistSpec.js @@ -797,13 +797,24 @@ describe('OCA.Files.FileList tests', function() { fileList.$fileList.on('fileActionsReady', handler); fileList.setFiles(testFiles); expect(handler.calledOnce).toEqual(true); + expect(handler.getCall(0).args[0].$files.length).toEqual(testFiles.length); }); it('triggers "fileActionsReady" event after single add', function() { var handler = sinon.stub(); + var $tr; fileList.setFiles(testFiles); fileList.$fileList.on('fileActionsReady', handler); - fileList.add({name: 'test.txt'}); + $tr = fileList.add({name: 'test.txt'}); + expect(handler.calledOnce).toEqual(true); + expect(handler.getCall(0).args[0].$files.is($tr)).toEqual(true); + }); + it('triggers "fileActionsReady" event after next page load with the newly appended files', function() { + var handler = sinon.stub(); + fileList.setFiles(generateFiles(0, 64)); + fileList.$fileList.on('fileActionsReady', handler); + fileList._nextPage(); expect(handler.calledOnce).toEqual(true); + expect(handler.getCall(0).args[0].$files.length).toEqual(fileList.pageSize); }); it('does not trigger "fileActionsReady" event after single add with silent argument', function() { var handler = sinon.stub(); diff --git a/apps/files_sharing/js/share.js b/apps/files_sharing/js/share.js index 2efed5310bc..e46be4ada46 100644 --- a/apps/files_sharing/js/share.js +++ b/apps/files_sharing/js/share.js @@ -36,54 +36,34 @@ } return tr; }; - - var oldRenderRow = OCA.Files.FileList.prototype._renderRow; - OCA.Files.FileList.prototype._renderRow = function(fileData) { - var $tr = oldRenderRow.apply(this, arguments); - // if the statuses are loaded already, use them for the icon - // (needed when scrolling to the next page) - var shareStatus = OC.Share.statuses[fileData.id]; - if (fileData.shareOwner || fileData.recipientsDisplayName || shareStatus) { - var permissions = $tr.data('permissions'); - var hasLink = !!(shareStatus && shareStatus.link); - if (permissions & OC.PERMISSION_SHARE) { - OC.Share.markFileAsShared($tr, true, hasLink); - } else { - // if no share action exists because the admin disabled sharing for this user - // we create a share notification action to inform the user about files - // shared with him otherwise we just update the existing share action. - // TODO: make this work like/with OC.Share.markFileAsShared() - var shareNotification = '' + - ' '; - $tr.find('.fileactions').append(function() { - var shareBy = t('files_sharing', 'Shared by {owner}', {owner: escapeHTML(fileData.shareOwner)}); - var $result = $(shareNotification + ' ' + shareBy + ''); - $result.on('click', function() { - return false; - }); - return $result; - }); - } - } - return $tr; - }; } // use delegate to catch the case with multiple file lists - $('#content').delegate('#fileList', 'fileActionsReady',function(ev){ + $('#content').delegate('#fileList', 'fileActionsReady', function(ev){ var fileList = ev.fileList; + var $files = ev.$files; + + function updateIcons($files) { + if (!$files) { + // if none specified, update all + $files = fileList.$fileList.find('tr'); + } + _.each($files, function(file) { + OCA.Sharing.Util.updateFileActionIcon($(file)); + }); + } + if (!OCA.Sharing.sharesLoaded){ - OC.Share.loadIcons('file', fileList); + OC.Share.loadIcons('file', fileList, function() { + // since we don't know which files are affected, just refresh them all + updateIcons(); + }); // assume that we got all shares, so switching directories // will not invalidate that list OCA.Sharing.sharesLoaded = true; } else{ - // this will update the icons for all the currently visible elements - // additionally added elements when scrolling down will be - // updated in the _renderRow override - OC.Share.updateIcons('file', fileList); + updateIcons($files); } }); @@ -140,6 +120,40 @@ }); }, + /** + * Update the file action share icon for the given file + * + * @param $tr file element of the file to update + */ + updateFileActionIcon: function($tr) { + // if the statuses are loaded already, use them for the icon + // (needed when scrolling to the next page) + var shareStatus = OC.Share.statuses[$tr.data('id')]; + if (shareStatus || $tr.attr('data-share-recipients') || $tr.attr('data-share-owner')) { + var permissions = $tr.data('permissions'); + var hasLink = !!(shareStatus && shareStatus.link); + OC.Share.markFileAsShared($tr, true, hasLink); + if ((permissions & OC.PERMISSION_SHARE) === 0) { + // if no share action exists because the admin disabled sharing for this user + // we create a share notification action to inform the user about files + // shared with him otherwise we just update the existing share action. + // TODO: make this work like/with OC.Share.markFileAsShared() + $tr.find('.fileactions .action-share-notification').remove(); + var shareNotification = '' + + ' '; + $tr.find('.fileactions').append(function() { + var shareBy = t('files_sharing', 'Shared by {owner}', {owner: escapeHTML($tr.attr('data-share-owner'))}); + var $result = $(shareNotification + ' ' + shareBy + ''); + $result.on('click', function() { + return false; + }); + return $result; + }); + } + } + }, + /** * Formats a recipients array to be displayed. * The first four recipients will be shown and the diff --git a/apps/files_sharing/tests/js/shareSpec.js b/apps/files_sharing/tests/js/shareSpec.js index 3d4fc754821..600859b4e7d 100644 --- a/apps/files_sharing/tests/js/shareSpec.js +++ b/apps/files_sharing/tests/js/shareSpec.js @@ -80,6 +80,14 @@ describe('OCA.Sharing.Util tests', function() { // TODO: test data-permissions, data-share-owner, etc }); describe('Share action icon', function() { + beforeEach(function() { + OC.Share.statuses = {1: {link: false, path: '/subdir'}}; + OCA.Sharing.sharesLoaded = true; + }); + afterEach(function() { + OC.Share.statuses = {}; + OCA.Sharing.sharesLoaded = false; + }); it('do not shows share text when not shared', function() { var $action, $tr; OC.Share.statuses = {}; diff --git a/core/js/share.js b/core/js/share.js index 5763664c5de..62271a43ceb 100644 --- a/core/js/share.js +++ b/core/js/share.js @@ -29,10 +29,13 @@ OC.Share={ * files "Share" icon to "Shared" according to their share status and * share type. * + * If a callback is specified, the update step is skipped. + * * @param itemType item type * @param fileList file list instance, defaults to OCA.Files.App.fileList + * @param callback function to call after the shares were loaded */ - loadIcons:function(itemType, fileList) { + loadIcons:function(itemType, fileList, callback) { // Load all share icons $.get( OC.filePath('core', 'ajax', 'share.php'), @@ -45,7 +48,11 @@ OC.Share={ $.each(result.data, function(item, data) { OC.Share.statuses[item] = data; }); - OC.Share.updateIcons(itemType, fileList); + if (_.isFunction(callback)) { + callback(OC.Share.statuses); + } else { + OC.Share.updateIcons(itemType, fileList); + } } } );