Skip to content

Commit

Permalink
fix(archive): Folders come before files when sorting (#1171)
Browse files Browse the repository at this point in the history 
* 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>
  • Loading branch information
@mergify
Mingze and mergify[bot] authored Feb 6, 2020
1 parent b072491 commit 4a618fe
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 22 deletions.
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
19 changes: 13 additions & 6 deletions src/lib/viewers/archive/ArchiveExplorer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class ArchiveExplorer extends React.Component {
fullPath: ROOT_FOLDER,
searchQuery: '',
sortBy: 'name',
sortDirection: SortDirection.DESC,
sortDirection: SortDirection.ASC,
view: VIEW_FOLDER,
};
}
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
Expand All @@ -189,8 +198,6 @@ class ArchiveExplorer extends React.Component {

return 0;
});

return sortDirection === SortDirection.DESC ? sortedItems : sortedItems.reverse();
}

/**
Expand Down
35 changes: 26 additions & 9 deletions src/lib/viewers/archive/__tests__/ArchiveExplorer-test-react.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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,
Expand All @@ -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,
},
];
});

Expand Down Expand Up @@ -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]]);
});
});

Expand Down Expand Up @@ -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]]);
});
});

Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand Down
9 changes: 7 additions & 2 deletions test/integration/archive/ArchiveViewer.e2e.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});

Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit 4a618fe

Please sign in to comment.