Fixed insertion of files

Removed "insert" flag, inserting is by default for FileList.add().
Added "animate" flag to FileList.add().
Added logic to correctly detect when to insert/append elements whenever
the insertion point is visible or not.
Fixed "render next page" logic to work correctly when many pages of
files have been added.

apps/files/js/file-upload.js

@@ -606,7 +606,7 @@ OC.Upload = {
 								{dir:$('#dir').val(), filename:name},
 								function(result) {
 									if (result.status === 'success') {
-										FileList.add(result.data, {hidden: hidden, insert: true});
+										FileList.add(result.data, {hidden: hidden, animate: true});
 									} else {
 										OC.dialogs.alert(result.data.message, t('core', 'Could not create file'));
 									}
@@ -619,7 +619,7 @@ OC.Upload = {
 								{dir:$('#dir').val(), foldername:name},
 								function(result) {
 									if (result.status === 'success') {
-										FileList.add(result.data, {hidden: hidden, insert: true});
+										FileList.add(result.data, {hidden: hidden, animate: true});
 									} else {
 										OC.dialogs.alert(result.data.message, t('core', 'Could not create folder'));
 									}
@@ -657,7 +657,7 @@ OC.Upload = {
 								var file = data;
 								$('#uploadprogressbar').fadeOut();
 
-								FileList.add(file, {hidden: hidden, insert: true});
+								FileList.add(file, {hidden: hidden, animate: true});
 							});
 							eventSource.listen('error',function(error) {
 								$('#uploadprogressbar').fadeOut();

apps/files/js/filelist.js

@@ -26,8 +26,6 @@ window.FileList = {
 
 	// number of files per page
 	pageSize: 20,
-	// zero based page number
-	pageNumber: 0,
 	totalPages: 0,
 
 	/**
@@ -227,9 +225,6 @@ window.FileList = {
 	},
 
 	_onScroll: function(e) {
-		if (this.pageNumber + 1 >= this.totalPages) {
-			return;
-		}
 		if ($(window).scrollTop() + $(window).height() > $(document).height() - 500) {
 			this._nextPage(true);
 		}
@@ -324,16 +319,17 @@ window.FileList = {
 	 * @param animate true to animate the new elements
 	 */
 	_nextPage: function(animate) {
-		var tr, index, count = this.pageSize,
+		var index = this.$fileList.children().length,
+			count = this.pageSize,
+			tr,
 			newTrs = [],
 			selected = this.isAllSelected();
 
-		if (this.pageNumber + 1 >= this.totalPages) {
+		if (index >= this.files.length) {
 			return;
 		}
 
 		this.pageNumber++;
-		index = this.pageNumber * this.pageSize;
 
 		while (count > 0 && index < this.files.length) {
 			tr = this._renderRow(this.files[index], {updateSummary: false});
@@ -343,7 +339,7 @@ window.FileList = {
 				tr.find('input:checkbox').prop('checked', true);
 			}
 			if (animate) {
-				tr.addClass('appear transparent'); // TODO
+				tr.addClass('appear transparent');
 				newTrs.push(tr);
 			}
 			index++;
@@ -516,34 +512,57 @@ window.FileList = {
 
 	/**
 	 * Adds an entry to the files array and also into the DOM
+	 * in a sorted manner.
 	 *
 	 * @param fileData map of file attributes
 	 * @param options map of attributes:
-	 * - "insert" true to insert in a sorted manner, false to append (default)
 	 * - "updateSummary" true to update the summary after adding (default), false otherwise
 	 * @return new tr element (not appended to the table)
 	 */
 	add: function(fileData, options) {
 		var index = -1;
-		var $tr = this._renderRow(fileData, options);
+		var $tr;
+		var $rows;
+		var $insertionPoint;
 		options = options || {};
 
-		this.isEmpty = false;
+		// there are three situations to cover:
+		// 1) insertion point is visible on the current page
+		// 2) insertion point is on a not visible page (visible after scrolling)
+		// 3) insertion point is at the end of the list
 
-		if (options.insert) {
+		$rows = this.$fileList.children();
 		index = this._findInsertionIndex(fileData);
-			if (index < this.files.length) {
+		if (index > this.files.length) {
+			index = this.files.length;
+		}
+		else {
+			$insertionPoint = $rows.eq(index);
+		}
+
+		// is the insertion point visible ?
+		if ($insertionPoint.length) {
+			// only render if it will really be inserted
+			$tr = this._renderRow(fileData, options);
+			$insertionPoint.before($tr);
+		}
+		else {
+			// if insertion point is after the last visible
+			// entry, append
+			if (index === $rows.length) {
+				$tr = this._renderRow(fileData, options);
+				this.$fileList.append($tr);
+			}
+		}
+
+		this.isEmpty = false;
 		this.files.splice(index, 0, fileData);
-				this.$fileList.children().eq(index).before($tr);
+
-			}
+		if ($tr && options.animate) {
-			else {
+			$tr.addClass('appear transparent');
-				this.files.push(fileData);
+			window.setTimeout(function() {
-				this.$fileList.append($tr);
+				$tr.removeClass('transparent');
-			}
+			});
-		}
-		else {
-			this.files.push(fileData);
-			this.$fileList.append($tr);
 		}
 
 		// defaults to true if not defined
@@ -880,7 +899,7 @@ window.FileList = {
 							// reinsert row
 							FileList.files.splice(tr.index(), 1);
 							tr.remove();
-							FileList.add(fileInfo, {insert: true});
+							FileList.add(fileInfo);
 						}
 					});
 				}
@@ -1351,7 +1370,7 @@ $(document).ready(function() {
 				FileList.remove(file.name);
 
 				// create new file context
-				data.context = FileList.add(file, {insert: true});
+				data.context = FileList.add(file, {animate: true});
 			}
 		}
 	});

apps/files/tests/js/fileactionsSpec.js

@@ -30,6 +30,7 @@ describe('FileActions tests', function() {
 		$body.append('<input type="hidden" id="permissions" value="31"></input>');
 		// dummy files table
 		$filesTable = $body.append('<table id="filestable"></table>');
+		FileList.files = [];
 	});
 	afterEach(function() {
 		$('#dir, #permissions, #filestable').remove();

apps/files/tests/js/filelistSpec.js

@@ -236,7 +236,7 @@ describe('FileList tests', function() {
 			var $tr;
 			var fileData = {
 				type: 'file',
-				name: 'P comes after O.txt'
+				name: 'ZZZ.txt'
 			};
 			FileList.setFiles(testFiles);
 			$tr = FileList.add(fileData);
@@ -245,7 +245,7 @@ describe('FileList tests', function() {
 		it('inserts files in a sorted manner when insert option is enabled', function() {
 			var $tr;
 			for (var i = 0; i < testFiles.length; i++) {
-				FileList.add(testFiles[i], {insert: true});
+				FileList.add(testFiles[i]);
 			}
 			expect(FileList.files[0].name).toEqual('somedir');
 			expect(FileList.files[1].name).toEqual('One.txt');
@@ -259,9 +259,9 @@ describe('FileList tests', function() {
 				name: 'P comes after O.txt'
 			};
 			for (var i = 0; i < testFiles.length; i++) {
-				FileList.add(testFiles[i], {insert: true});
+				FileList.add(testFiles[i]);
 			}
-			$tr = FileList.add(fileData, {insert: true});
+			$tr = FileList.add(fileData);
 			// after "One.txt"
 			expect($tr.index()).toEqual(2);
 			expect(FileList.files[2]).toEqual(fileData);
@@ -273,9 +273,9 @@ describe('FileList tests', function() {
 				name: 'somedir2 comes after somedir'
 			};
 			for (var i = 0; i < testFiles.length; i++) {
-				FileList.add(testFiles[i], {insert: true});
+				FileList.add(testFiles[i]);
 			}
-			$tr = FileList.add(fileData, {insert: true});
+			$tr = FileList.add(fileData);
 			expect($tr.index()).toEqual(1);
 			expect(FileList.files[1]).toEqual(fileData);
 		});
@@ -286,9 +286,9 @@ describe('FileList tests', function() {
 				name: 'zzz.txt'
 			};
 			for (var i = 0; i < testFiles.length; i++) {
-				FileList.add(testFiles[i], {insert: true});
+				FileList.add(testFiles[i]);
 			}
-			$tr = FileList.add(fileData, {insert: true});
+			$tr = FileList.add(fileData);
 			expect($tr.index()).toEqual(4);
 			expect(FileList.files[4]).toEqual(fileData);
 		});
@@ -428,7 +428,7 @@ describe('FileList tests', function() {
 			var $input, request;
 
 			for (var i = 0; i < testFiles.length; i++) {
-				FileList.add(testFiles[i], {insert: true});
+				FileList.add(testFiles[i]);
 			}
 
 			// trigger rename prompt
@@ -516,14 +516,12 @@ describe('FileList tests', function() {
 	});
 	describe('List rendering', function() {
 		it('renders a list of files using add()', function() {
-			var addSpy = sinon.spy(FileList, 'add');
 			expect(FileList.files.length).toEqual(0);
 			expect(FileList.files).toEqual([]);
 			FileList.setFiles(testFiles);
 			expect($('#fileList tr').length).toEqual(4);
 			expect(FileList.files.length).toEqual(4);
 			expect(FileList.files).toEqual(testFiles);
-			addSpy.restore();
 		});
 		it('updates summary using the file sizes', function() {
 			var $summary;
@@ -593,6 +591,117 @@ describe('FileList tests', function() {
 			expect($summary.find('.info').text()).toEqual('0 folders and 1 file');
 		});
 	});
+	describe('Rendering next page on scroll', function() {
+
+		function generateFiles(startIndex, endIndex) {
+			var files = [];
+			var name;
+			for (var i = startIndex; i <= endIndex; i++) {
+				name = 'File with index ';
+				if (i < 10) {
+					// do not rely on localeCompare here
+					// and make the sorting predictable
+					// cross-browser
+					name += '0';
+				}
+				name += i + '.txt';
+				files.push({
+					id: i,
+					type: 'file',
+					name: name,
+					mimetype: 'text/plain',
+					size: i * 2,
+					etag: 'abc'
+				});
+			}
+			return files;
+		}
+
+		beforeEach(function() {
+			FileList.setFiles(generateFiles(0, 64));
+		});
+		it('renders only the first page', function() {
+			expect(FileList.files.length).toEqual(65);
+			expect($('#fileList tr').length).toEqual(20);
+		});
+		it('renders the second page when scrolling down (trigger nextPage)', function() {
+			// TODO: can't simulate scrolling here, so calling nextPage directly
+			FileList._nextPage(true);
+			expect($('#fileList tr').length).toEqual(40);
+			FileList._nextPage(true);
+			expect($('#fileList tr').length).toEqual(60);
+			FileList._nextPage(true);
+			expect($('#fileList tr').length).toEqual(65);
+			FileList._nextPage(true);
+			// stays at 65
+			expect($('#fileList tr').length).toEqual(65);
+		});
+		it('inserts into the DOM if insertion point is in the visible page ', function() {
+			FileList.add({
+				id: 2000,
+				type: 'file',
+				name: 'File with index 15b.txt'
+			});
+			expect($('#fileList tr').length).toEqual(21);
+			expect(FileList.findFileEl('File with index 15b.txt').index()).toEqual(16);
+		});
+		it('does not inserts into the DOM if insertion point is not the visible page ', function() {
+			FileList.add({
+				id: 2000,
+				type: 'file',
+				name: 'File with index 28b.txt'
+			});
+			expect($('#fileList tr').length).toEqual(20);
+			expect(FileList.findFileEl('File with index 28b.txt').length).toEqual(0);
+			FileList._nextPage(true);
+			expect($('#fileList tr').length).toEqual(40);
+			expect(FileList.findFileEl('File with index 28b.txt').index()).toEqual(29);
+		});
+		it('appends into the DOM when inserting a file after the last visible element', function() {
+			FileList.add({
+				id: 2000,
+				type: 'file',
+				name: 'File with index 19b.txt'
+			});
+			expect($('#fileList tr').length).toEqual(21);
+			FileList._nextPage(true);
+			expect($('#fileList tr').length).toEqual(41);
+		});
+		it('appends into the DOM when inserting a file on the last page when visible', function() {
+			FileList._nextPage(true);
+			expect($('#fileList tr').length).toEqual(40);
+			FileList._nextPage(true);
+			expect($('#fileList tr').length).toEqual(60);
+			FileList._nextPage(true);
+			expect($('#fileList tr').length).toEqual(65);
+			FileList._nextPage(true);
+			FileList.add({
+				id: 2000,
+				type: 'file',
+				name: 'File with index 88.txt'
+			});
+			expect($('#fileList tr').length).toEqual(66);
+			FileList._nextPage(true);
+			expect($('#fileList tr').length).toEqual(66);
+		});
+		it('shows additional page when appending a page of files and scrolling down', function() {
+			var newFiles = generateFiles(66, 81);
+			for (var i = 0; i < newFiles.length; i++) {
+				FileList.add(newFiles[i]);
+			}
+			expect($('#fileList tr').length).toEqual(20);
+			FileList._nextPage(true);
+			expect($('#fileList tr').length).toEqual(40);
+			FileList._nextPage(true);
+			expect($('#fileList tr').length).toEqual(60);
+			FileList._nextPage(true);
+			expect($('#fileList tr').length).toEqual(80);
+			FileList._nextPage(true);
+			expect($('#fileList tr').length).toEqual(81);
+			FileList._nextPage(true);
+			expect($('#fileList tr').length).toEqual(81);
+		});
+	});
 	describe('file previews', function() {
 		var previewLoadStub;