From: Vincent Petry Date: Fri, 4 Apr 2014 14:11:31 +0000 (+0200) Subject: Fixed insertion of files X-Git-Tag: v7.0.0alpha2~339^2~9 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=2883f231d0b08e8eea75715e912caa42f20d9682;p=nextcloud-server.git 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. --- diff --git a/apps/files/js/file-upload.js b/apps/files/js/file-upload.js index 03ebdccb32d..963fc647828 100644 --- a/apps/files/js/file-upload.js +++ b/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(); diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 53bb3a5c868..3bf5b2d9672 100644 --- a/apps/files/js/filelist.js +++ b/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++; @@ -365,7 +361,7 @@ window.FileList = { * This operation will rerender the list and update the summary. * @param filesArray array of file data (map) */ - setFiles:function(filesArray) { + setFiles: function(filesArray) { // detach to make adding multiple rows faster this.files = filesArray; this.pageNumber = -1; @@ -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) { - index = this._findInsertionIndex(fileData); - if (index < this.files.length) { - this.files.splice(index, 0, fileData); - this.$fileList.children().eq(index).before($tr); - } - else { - this.files.push(fileData); + $rows = this.$fileList.children(); + index = this._findInsertionIndex(fileData); + 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); } } - else { - this.files.push(fileData); - this.$fileList.append($tr); + + this.isEmpty = false; + this.files.splice(index, 0, fileData); + + if ($tr && options.animate) { + $tr.addClass('appear transparent'); + window.setTimeout(function() { + $tr.removeClass('transparent'); + }); } // 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}); } } }); diff --git a/apps/files/tests/js/fileactionsSpec.js b/apps/files/tests/js/fileactionsSpec.js index 3c22c84b866..f5eafba509f 100644 --- a/apps/files/tests/js/fileactionsSpec.js +++ b/apps/files/tests/js/fileactionsSpec.js @@ -30,6 +30,7 @@ describe('FileActions tests', function() { $body.append(''); // dummy files table $filesTable = $body.append('
'); + FileList.files = []; }); afterEach(function() { $('#dir, #permissions, #filestable').remove(); diff --git a/apps/files/tests/js/filelistSpec.js b/apps/files/tests/js/filelistSpec.js index 1b155f4f1df..7316cb75315 100644 --- a/apps/files/tests/js/filelistSpec.js +++ b/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;