From 9c9158e6b78c69835cb7280810819fb2cd6f84f7 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 28 Oct 2015 17:43:36 +0100 Subject: [PATCH] Fix icon update to be more consistent Makes the details bar show the same icon as in the list. --- apps/files/js/filelist.js | 22 +++++++-- apps/files/js/mainfileinfodetailview.js | 4 +- apps/files/tests/js/filelistSpec.js | 47 +++++++++++++++++-- .../tests/js/mainfileinfodetailviewSpec.js | 14 ++++++ apps/files_sharing/js/share.js | 7 +++ core/js/mimetype.js | 2 + core/js/share.js | 17 +++++-- core/js/tests/specs/shareSpec.js | 7 +++ 8 files changed, 109 insertions(+), 11 deletions(-) diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index c84d6c3c47..99804b1b9b 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -767,7 +767,7 @@ */ elementToFile: function($el){ $el = $($el); - return { + var data = { id: parseInt($el.attr('data-id'), 10), name: $el.attr('data-file'), mimetype: $el.attr('data-mime'), @@ -777,6 +777,15 @@ etag: $el.attr('data-etag'), permissions: parseInt($el.attr('data-permissions'), 10) }; + var icon = $el.attr('data-icon'); + if (icon) { + data.icon = icon; + } + var mountType = $el.attr('data-mounttype'); + if (mountType) { + data.mountType = mountType; + } + return data; }, /** @@ -899,11 +908,12 @@ mtime = parseInt(fileData.mtime, 10), mime = fileData.mimetype, path = fileData.path, + dataIcon = null, linkUrl; options = options || {}; if (isNaN(mtime)) { - mtime = new Date().getTime() + mtime = new Date().getTime(); } if (type === 'dir') { @@ -911,6 +921,7 @@ if (fileData.mountType && fileData.mountType.indexOf('external') === 0) { icon = OC.MimeType.getIconUrl('dir-external'); + dataIcon = icon; } } @@ -926,6 +937,11 @@ "data-permissions": fileData.permissions || this.getDirectoryPermissions() }); + if (dataIcon) { + // icon override + tr.attr('data-icon', dataIcon); + } + if (fileData.mountType) { tr.attr('data-mounttype', fileData.mountType); } @@ -1177,7 +1193,7 @@ // display actions this.fileActions.display(filenameTd, !options.silent, this); - if (fileData.isPreviewAvailable) { + if (fileData.isPreviewAvailable && mime !== 'httpd/unix-directory') { var iconDiv = filenameTd.find('.thumbnail'); // lazy load / newly inserted td ? // the typeof check ensures that the default value of animate is true diff --git a/apps/files/js/mainfileinfodetailview.js b/apps/files/js/mainfileinfodetailview.js index abf7da52ff..69c796e492 100644 --- a/apps/files/js/mainfileinfodetailview.js +++ b/apps/files/js/mainfileinfodetailview.js @@ -128,8 +128,8 @@ $iconDiv.addClass('icon-loading icon-32'); this.loadPreview(this.model.getFullPath(), this.model.get('mimetype'), this.model.get('etag'), $iconDiv, $container, this.model.isImage()); } else { - // TODO: special icons / shared / external - $iconDiv.css('background-image', 'url("' + OC.MimeType.getIconUrl('dir') + '")'); + var iconUrl = this.model.get('icon') || OC.MimeType.getIconUrl('dir'); + $iconDiv.css('background-image', 'url("' + iconUrl + '")'); OC.Util.scaleFixForIE8($iconDiv); } this.$el.find('[title]').tooltip({placement: 'bottom'}); diff --git a/apps/files/tests/js/filelistSpec.js b/apps/files/tests/js/filelistSpec.js index 994e1d3284..05e6fcc612 100644 --- a/apps/files/tests/js/filelistSpec.js +++ b/apps/files/tests/js/filelistSpec.js @@ -1166,7 +1166,7 @@ describe('OCA.Files.FileList tests', function() { it('renders provided icon for file when provided', function() { var fileData = { type: 'file', - name: 'test dir', + name: 'test file', icon: OC.webroot + '/core/img/filetypes/application-pdf.svg', mimetype: 'application/pdf' }; @@ -1178,7 +1178,7 @@ describe('OCA.Files.FileList tests', function() { it('renders preview when no icon was provided and preview is available', function() { var fileData = { type: 'file', - name: 'test dir', + name: 'test file', isPreviewAvailable: true }; var $tr = fileList.add(fileData); @@ -1192,7 +1192,7 @@ describe('OCA.Files.FileList tests', function() { it('renders default file type icon when no icon was provided and no preview is available', function() { var fileData = { type: 'file', - name: 'test dir', + name: 'test file', isPreviewAvailable: false }; var $tr = fileList.add(fileData); @@ -1200,6 +1200,47 @@ describe('OCA.Files.FileList tests', function() { expect(OC.TestUtil.getImageUrl($imgDiv)).toEqual(OC.webroot + '/core/img/filetypes/file.svg'); expect(previewLoadStub.notCalled).toEqual(true); }); + it('does not render preview for directories', function() { + var fileData = { + type: 'dir', + mimetype: 'httpd/unix-directory', + name: 'test dir', + isPreviewAvailable: true + }; + var $tr = fileList.add(fileData); + var $td = $tr.find('td.filename'); + expect(OC.TestUtil.getImageUrl($td.find('.thumbnail'))).toEqual(OC.webroot + '/core/img/filetypes/folder.svg'); + expect(previewLoadStub.notCalled).toEqual(true); + }); + it('render external storage icon for external storage root', function() { + var fileData = { + type: 'dir', + mimetype: 'httpd/unix-directory', + name: 'test dir', + isPreviewAvailable: true, + mountType: 'external-root' + }; + var $tr = fileList.add(fileData); + var $td = $tr.find('td.filename'); + expect(OC.TestUtil.getImageUrl($td.find('.thumbnail'))).toEqual(OC.webroot + '/core/img/filetypes/folder-external.svg'); + expect(previewLoadStub.notCalled).toEqual(true); + }); + it('render external storage icon for external storage subdir', function() { + var fileData = { + type: 'dir', + mimetype: 'httpd/unix-directory', + name: 'test dir', + isPreviewAvailable: true, + mountType: 'external' + }; + var $tr = fileList.add(fileData); + var $td = $tr.find('td.filename'); + expect(OC.TestUtil.getImageUrl($td.find('.thumbnail'))).toEqual(OC.webroot + '/core/img/filetypes/folder-external.svg'); + expect(previewLoadStub.notCalled).toEqual(true); + // default icon override + expect($tr.attr('data-icon')).toEqual(OC.webroot + '/core/img/filetypes/folder-external.svg'); + }); + }); describe('viewer mode', function() { it('enabling viewer mode hides files table and action buttons', function() { diff --git a/apps/files/tests/js/mainfileinfodetailviewSpec.js b/apps/files/tests/js/mainfileinfodetailviewSpec.js index f4403196f2..460629806c 100644 --- a/apps/files/tests/js/mainfileinfodetailviewSpec.js +++ b/apps/files/tests/js/mainfileinfodetailviewSpec.js @@ -112,6 +112,20 @@ describe('OCA.Files.MainFileInfoDetailView tests', function() { lazyLoadPreviewStub.restore(); }); + it('uses icon from model if present in model', function() { + var lazyLoadPreviewStub = sinon.stub(fileList, 'lazyLoadPreview'); + testFileInfo.set('mimetype', 'httpd/unix-directory'); + testFileInfo.set('icon', OC.MimeType.getIconUrl('dir-external')); + view.setFileInfo(testFileInfo); + + expect(lazyLoadPreviewStub.notCalled).toEqual(true); + + expect(view.$el.find('.thumbnail').hasClass('icon-loading')).toEqual(false); + expect(view.$el.find('.thumbnail').css('background-image')) + .toContain('filetypes/folder-external.svg'); + + lazyLoadPreviewStub.restore(); + }); it('displays thumbnail', function() { var lazyLoadPreviewStub = sinon.stub(fileList, 'lazyLoadPreview'); diff --git a/apps/files_sharing/js/share.js b/apps/files_sharing/js/share.js index 30a803f320..63225a0d8e 100644 --- a/apps/files_sharing/js/share.js +++ b/apps/files_sharing/js/share.js @@ -130,6 +130,13 @@ // remove icon, if applicable OC.Share.markFileAsShared($tr, false, false); } + var newIcon = $tr.attr('data-icon'); + // in case markFileAsShared decided to change the icon, + // we need to modify the model + // (FIXME: yes, this is hacky) + if (fileInfoModel.get('icon') !== newIcon) { + fileInfoModel.set('icon', newIcon); + } }); fileList.registerTabView(shareTab); }, diff --git a/core/js/mimetype.js b/core/js/mimetype.js index b0de8eb841..3cc33ce283 100644 --- a/core/js/mimetype.js +++ b/core/js/mimetype.js @@ -46,6 +46,8 @@ OC.MimeType = { return 'folder'; } else if (mimeType === 'dir-shared' && $.inArray('folder-shared', files) !== -1) { return 'folder-shared'; + } else if (mimeType === 'dir-public' && $.inArray('folder-public', files) !== -1) { + return 'folder-public'; } else if (mimeType === 'dir-external' && $.inArray('folder-external', files) !== -1) { return 'folder-external'; } else if ($.inArray(icon, files) !== -1) { diff --git a/core/js/share.js b/core/js/share.js index 1131ae8f11..e14e19a254 100644 --- a/core/js/share.js +++ b/core/js/share.js @@ -253,14 +253,25 @@ OC.Share = _.extend(OC.Share || {}, { // update folder icon if (type === 'dir' && (hasShares || hasLink || owner)) { if (hasLink) { - shareFolderIcon = OC.imagePath('core', 'filetypes/folder-public'); + shareFolderIcon = OC.MimeType.getIconUrl('dir-public'); } else { - shareFolderIcon = OC.imagePath('core', 'filetypes/folder-shared'); + shareFolderIcon = OC.MimeType.getIconUrl('dir-shared'); } $tr.find('.filename .thumbnail').css('background-image', 'url(' + shareFolderIcon + ')'); + $tr.attr('data-icon', shareFolderIcon); } else if (type === 'dir') { - shareFolderIcon = OC.imagePath('core', 'filetypes/folder'); + var mountType = $tr.attr('data-mounttype'); + // FIXME: duplicate of FileList._createRow logic for external folder, + // need to refactor the icon logic into a single code path eventually + if (mountType && mountType.indexOf('external') === 0) { + shareFolderIcon = OC.MimeType.getIconUrl('dir-external'); + $tr.attr('data-icon', shareFolderIcon); + } else { + shareFolderIcon = OC.MimeType.getIconUrl('dir'); + // back to default + $tr.removeAttr('data-icon'); + } $tr.find('.filename .thumbnail').css('background-image', 'url(' + shareFolderIcon + ')'); } // update share action text / icon diff --git a/core/js/tests/specs/shareSpec.js b/core/js/tests/specs/shareSpec.js index 9e80f4fe19..5c76ea600b 100644 --- a/core/js/tests/specs/shareSpec.js +++ b/core/js/tests/specs/shareSpec.js @@ -142,6 +142,13 @@ describe('OC.Share tests', function() { checkIcon('filetypes/folder-public'); }); + it('shows external storage icon if external mount point', function() { + $file.attr('data-type', 'dir'); + $file.attr('data-mountType', 'external'); + OC.Share.markFileAsShared($file, false, false); + + checkIcon('filetypes/folder-external'); + }); }); describe('displaying the recipoients', function() {