Merge pull request #20122 from owncloud/files-consolidateiconupdate

Fix icon update to be more consistent

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

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
+					var iconUrl = this.model.get('icon') || OC.MimeType.getIconUrl('dir');
-					$iconDiv.css('background-image', 'url("' + OC.MimeType.getIconUrl('dir') + '")');
+					$iconDiv.css('background-image', 'url("' + iconUrl + '")');
 					OC.Util.scaleFixForIE8($iconDiv);
 				}
 				this.$el.find('[title]').tooltip({placement: 'bottom'});

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() {

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');
 

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);
 		},

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) {

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

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() {