From 402845ed2558004b8755a597a6d43c99d65d2669 Mon Sep 17 00:00:00 2001 From: Mingze Xiao Date: Tue, 4 Feb 2020 17:27:48 -0800 Subject: [PATCH 1/3] fix(archive): Folders come before files when sorting --- package.json | 2 +- src/lib/viewers/archive/ArchiveExplorer.js | 16 +++++++-- .../__tests__/ArchiveExplorer-test-react.js | 35 ++++++++++++++----- .../archive/ArchiveViewer.e2e.test.js | 9 +++-- yarn.lock | 8 ++--- 5 files changed, 52 insertions(+), 18 deletions(-) diff --git a/package.json b/package.json index 2d5fdf89d..1c05589e8 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 5e0bb8a65..af84a6ce1 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, }; } @@ -190,7 +190,19 @@ class ArchiveExplorer extends React.Component { return 0; }); - return sortDirection === SortDirection.DESC ? sortedItems : sortedItems.reverse(); + if (sortDirection === SortDirection.DESC) { + sortedItems.reverse(); + } + + // folders always come before files + sortedItems.sort((a, b) => { + if (a.type === 'folder' && b.type === 'folder') { + return 0; + } + return a.type === 'folder' ? -1 : 1; + }); + + return sortedItems; } /** 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 6e4a84069..cf711f95a 100644 --- a/test/integration/archive/ArchiveViewer.e2e.test.js +++ b/test/integration/archive/ArchiveViewer.e2e.test.js @@ -26,15 +26,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" From 018e80c3ee4342c37f4e61ea1adafbb4743911ed Mon Sep 17 00:00:00 2001 From: Mingze Xiao Date: Wed, 5 Feb 2020 16:21:51 -0800 Subject: [PATCH 2/3] fix(archive): Address comments --- src/lib/viewers/archive/ArchiveExplorer.js | 25 +++++++--------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/src/lib/viewers/archive/ArchiveExplorer.js b/src/lib/viewers/archive/ArchiveExplorer.js index af84a6ce1..f547b48f6 100644 --- a/src/lib/viewers/archive/ArchiveExplorer.js +++ b/src/lib/viewers/archive/ArchiveExplorer.js @@ -167,7 +167,12 @@ class ArchiveExplorer extends React.Component { return itemList; } - const sortedItems = itemList.sort((a = {}, b = {}) => { + return itemList.sort((a = {}, b = {}) => { + // folders are always in front of files + if (a.type !== b.type) { + return a.type === 'folder' ? -1 : 1; + } + const aItem = a[sortBy]; const bItem = b[sortBy]; @@ -180,29 +185,15 @@ class ArchiveExplorer extends React.Component { } if (typeof aItem === 'number' && typeof bItem === 'number') { - return aItem - bItem; + return sortDirection === SortDirection.ASC ? aItem - bItem : bItem - aItem; } if (typeof aItem === 'string' && typeof bItem === 'string') { - return aItem.localeCompare(bItem); + return sortDirection === SortDirection.ASC ? aItem.localeCompare(bItem) : bItem.localeCompare(aItem); } return 0; }); - - if (sortDirection === SortDirection.DESC) { - sortedItems.reverse(); - } - - // folders always come before files - sortedItems.sort((a, b) => { - if (a.type === 'folder' && b.type === 'folder') { - return 0; - } - return a.type === 'folder' ? -1 : 1; - }); - - return sortedItems; } /** From 9d71ff8f444fe5bda4fbb2036c754e8502bd83f3 Mon Sep 17 00:00:00 2001 From: Mingze Xiao Date: Wed, 5 Feb 2020 16:51:18 -0800 Subject: [PATCH 3/3] fix(archive): Address comments --- src/lib/viewers/archive/ArchiveExplorer.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/lib/viewers/archive/ArchiveExplorer.js b/src/lib/viewers/archive/ArchiveExplorer.js index f547b48f6..836eaba69 100644 --- a/src/lib/viewers/archive/ArchiveExplorer.js +++ b/src/lib/viewers/archive/ArchiveExplorer.js @@ -173,8 +173,8 @@ class ArchiveExplorer extends React.Component { return a.type === 'folder' ? -1 : 1; } - const aItem = a[sortBy]; - const bItem = b[sortBy]; + let aItem = a[sortBy]; + let bItem = b[sortBy]; if (aItem === null || aItem === undefined) { return 1; @@ -184,12 +184,16 @@ class ArchiveExplorer extends React.Component { return -1; } + if (sortDirection === SortDirection.DESC) { + [aItem, bItem] = [bItem, aItem]; + } + if (typeof aItem === 'number' && typeof bItem === 'number') { - return sortDirection === SortDirection.ASC ? aItem - bItem : bItem - aItem; + return aItem - bItem; } if (typeof aItem === 'string' && typeof bItem === 'string') { - return sortDirection === SortDirection.ASC ? aItem.localeCompare(bItem) : bItem.localeCompare(aItem); + return aItem.localeCompare(bItem); } return 0;