From 4a618fe5c97a00c69f5f6cdcb1672152e41ebbce Mon Sep 17 00:00:00 2001 From: Mingze Date: Wed, 5 Feb 2020 17:42:21 -0800 Subject: [PATCH] fix(archive): Folders come before files when sorting (#1171) * fix(archive): Folders come before files when sorting * fix(archive): Address comments * fix(archive): Address comments Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- package.json | 2 +- src/lib/viewers/archive/ArchiveExplorer.js | 19 ++++++---- .../__tests__/ArchiveExplorer-test-react.js | 35 ++++++++++++++----- .../archive/ArchiveViewer.e2e.test.js | 9 +++-- yarn.lock | 8 ++--- 5 files changed, 51 insertions(+), 22 deletions(-) diff --git a/package.json b/package.json index 730190891..a0b7dd593 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,7 @@ "babel-plugin-react-remove-properties": "^0.3.0", "babel-plugin-transform-require-ignore": "^0.1.1", "box-annotations": "^2.3.0", - "box-ui-elements": "^11.1.0-beta.41", + "box-ui-elements": "^12.0.0-beta.10", "chai": "^4.2.0", "chai-dom": "^1.8.1", "conventional-changelog-cli": "^2.0.28", diff --git a/src/lib/viewers/archive/ArchiveExplorer.js b/src/lib/viewers/archive/ArchiveExplorer.js index 898cef60a..09e99dbef 100644 --- a/src/lib/viewers/archive/ArchiveExplorer.js +++ b/src/lib/viewers/archive/ArchiveExplorer.js @@ -51,7 +51,7 @@ class ArchiveExplorer extends React.Component { fullPath: ROOT_FOLDER, searchQuery: '', sortBy: 'name', - sortDirection: SortDirection.DESC, + sortDirection: SortDirection.ASC, view: VIEW_FOLDER, }; } @@ -167,9 +167,14 @@ class ArchiveExplorer extends React.Component { return itemList; } - const sortedItems = itemList.sort((a = {}, b = {}) => { - const aItem = a[sortBy]; - const bItem = b[sortBy]; + return itemList.sort((a = {}, b = {}) => { + // folders are always in front of files + if (a.type !== b.type) { + return a.type === 'folder' ? -1 : 1; + } + + let aItem = a[sortBy]; + let bItem = b[sortBy]; if (aItem === null || aItem === undefined) { return 1; @@ -179,6 +184,10 @@ class ArchiveExplorer extends React.Component { return -1; } + if (sortDirection === SortDirection.DESC) { + [aItem, bItem] = [bItem, aItem]; + } + if (typeof aItem === 'number' && typeof bItem === 'number') { return aItem - bItem; } @@ -189,8 +198,6 @@ class ArchiveExplorer extends React.Component { return 0; }); - - return sortDirection === SortDirection.DESC ? sortedItems : sortedItems.reverse(); } /** diff --git a/src/lib/viewers/archive/__tests__/ArchiveExplorer-test-react.js b/src/lib/viewers/archive/__tests__/ArchiveExplorer-test-react.js index d9c059f56..5e2ef4526 100644 --- a/src/lib/viewers/archive/__tests__/ArchiveExplorer-test-react.js +++ b/src/lib/viewers/archive/__tests__/ArchiveExplorer-test-react.js @@ -20,7 +20,7 @@ describe('lib/viewers/archive/ArchiveExplorer', () => { name: 'test', modified_at: '19-Dec-02 16:43', size: 0, - item_collection: ['test/csv-level-1.csv', 'test/subfolder/'], + item_collection: ['test/csv-level-1.csv', 'test/pdf-level-1.pdf', 'test/subfolder/'], }, { type: 'file', @@ -36,11 +36,11 @@ describe('lib/viewers/archive/ArchiveExplorer', () => { name: 'subfolder', modified_at: '19-Dec-02 16:43', size: 0, - item_collection: ['test/test-level-2.jpg'], + item_collection: ['test/subfolder/test-level-2.jpg'], }, { type: 'file', - absolute_path: 'test/test-level-2.jpg', + absolute_path: 'test/subfolder/test-level-2.jpg', name: 'test-level-1.jpg', modified_at: '19-Nov-08 15:08', size: 57379, @@ -54,6 +54,14 @@ describe('lib/viewers/archive/ArchiveExplorer', () => { size: 1, item_collection: null, }, + { + type: 'file', + absolute_path: 'test/pdf-level-1.pdf', + name: 'pdf-level-1.pdf', + modified_at: '19-Nov-04 16:11', + size: 233, + item_collection: null, + }, ]; }); @@ -130,7 +138,7 @@ describe('lib/viewers/archive/ArchiveExplorer', () => { itemList = component.instance().getItemList(data, 'test/'); - expect(itemList).to.eql([data[1], data[2]]); + expect(itemList).to.eql([data[1], data[2], data[5]]); }); }); @@ -163,8 +171,8 @@ describe('lib/viewers/archive/ArchiveExplorer', () => { const itemList = component.instance().getSearchResult(data, 'level-1'); const fuzzyList = component.instance().getSearchResult(data, 'leel1'); - expect(itemList).to.eql([data[1], data[3]]); - expect(fuzzyList).to.eql([data[1], data[3]]); + expect(itemList).to.eql([data[1], data[3], data[5]]); + expect(fuzzyList).to.eql([data[1], data[3], data[5]]); }); }); @@ -189,7 +197,9 @@ describe('lib/viewers/archive/ArchiveExplorer', () => { instance.handleSort({ sortBy: 'size', sortDirection: 'DESC' }); const sortedList = instance.sortItemList(itemList); + // folders come before files expect(sortedList[0]).to.equal(data[2]); + expect(sortedList[1]).to.equal(data[5]); }); it('should sort itemList by name and be in ASC order', () => { @@ -200,7 +210,9 @@ describe('lib/viewers/archive/ArchiveExplorer', () => { instance.handleSort({ sortBy: 'name', sortDirection: 'ASC' }); const sortedList = instance.sortItemList(itemList); + // folders come before files expect(sortedList[0]).to.equal(data[2]); + expect(sortedList[1]).to.equal(data[1]); }); it('should sort itemList by name and be in DESC order', () => { @@ -211,7 +223,9 @@ describe('lib/viewers/archive/ArchiveExplorer', () => { instance.handleSort({ sortBy: 'name', sortDirection: 'DESC' }); const sortedList = instance.sortItemList(itemList); - expect(sortedList[0]).to.equal(data[1]); + // folders come before files + expect(sortedList[0]).to.equal(data[2]); + expect(sortedList[1]).to.equal(data[5]); }); it('should not sort itemList', () => { @@ -221,7 +235,7 @@ describe('lib/viewers/archive/ArchiveExplorer', () => { const sortedList = instance.sortItemList(itemList); - expect(sortedList[0]).to.equal(itemList[0]); + expect(sortedList).to.eql(itemList); }); it('should sort item list with string values and null', () => { @@ -236,7 +250,10 @@ describe('lib/viewers/archive/ArchiveExplorer', () => { const sortedList = instance.sortItemList(itemList); - expect(sortedList[0]).to.equal(itemList[0]); + // folders come before files + expect(sortedList[0]).to.equal(data[2]); + // item with not-null value comes first + expect(sortedList[1]).to.equal(data[5]); }); it('should sort items with number values and null', () => { diff --git a/test/integration/archive/ArchiveViewer.e2e.test.js b/test/integration/archive/ArchiveViewer.e2e.test.js index 4003b0ece..b9a5e72bb 100644 --- a/test/integration/archive/ArchiveViewer.e2e.test.js +++ b/test/integration/archive/ArchiveViewer.e2e.test.js @@ -28,15 +28,20 @@ describe('Archive Viewer', () => { cy.getByTitle('Preview SDK Sample Archive').within(() => { cy.get('button').click(); }); - // default sort by name + // folders are in front of files cy.get('.ReactVirtualized__Table__row') .first() + .contains('Level 1 Folder'); + + // default sort by name + cy.get('.ReactVirtualized__Table__row') + .eq(1) .contains('Audio.mp3'); // reverse cy.getByTitle('Name').click(); cy.get('.ReactVirtualized__Table__row') - .first() + .eq(1) .contains('Preview SDK Sample Excel.xlsx'); }); diff --git a/yarn.lock b/yarn.lock index bc306512b..6e939904e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2314,10 +2314,10 @@ box-annotations@^2.3.0: resolved "https://registry.yarnpkg.com/box-annotations/-/box-annotations-2.3.0.tgz#5cac38171f7f8d9283659e2b243310f19d5ab7d3" integrity sha512-Ea7tPgyJjX7vcnmZIfCorbzHd6oYx/OHVMPnZVQL/dUHR5vRKhLM0610xqwmVlUpk627sqHw5x/APaa+kt4SXg== -box-ui-elements@^11.1.0-beta.41: - version "11.1.0-beta.41" - resolved "https://registry.yarnpkg.com/box-ui-elements/-/box-ui-elements-11.1.0-beta.41.tgz#ab2879b8594bfba8d8f63192670581d6aa8d5d62" - integrity sha512-LZXQW5zrzKpWX8e8p45uIaMLjyXRcRb9XAnh5cdH+9+fUcrurpyEbJySdkUXwF8zSTHB18DA+GVLe6Jedkxs9g== +box-ui-elements@^12.0.0-beta.10: + version "12.0.0-beta.10" + resolved "https://registry.yarnpkg.com/box-ui-elements/-/box-ui-elements-12.0.0-beta.10.tgz#00bf1958d53655f44205a42aad4bc91a75325aff" + integrity sha512-P5JM4tFgfVtxSGHV6ApM+Bu8Un8pC2ZAWrI9L+3k8A6lPrMBZSS+GafYMpH5aeyksqXUH2vXNkG7x3vmmoMdqQ== brace-expansion@^1.1.7: version "1.1.11"