Skip to content
This repository has been archived by the owner on Feb 27, 2020. It is now read-only.

Commit

Permalink
fix(filters): do not sort the selected items to the top of the list (#80
Browse files Browse the repository at this point in the history 
)
  • Loading branch information
@flannanl @zeusorion
flannanl authored and zeusorion committed May 9, 2019
1 parent e69d6f8 commit 00091a7
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 91 deletions.
41 changes: 22 additions & 19 deletions src/components/MultiSelect/__tests__/NestedFilterableMultiselect-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ describe('NestedFilterableMultiselect', () => {

wrapper
.find(listItemName)
.at(0)
.at(1)
.simulate('click');
expect(mockProps.onChange).toHaveBeenCalledTimes(4);
expect(mockProps.onChange).toHaveBeenCalledWith({
Expand Down Expand Up @@ -615,7 +615,7 @@ describe('NestedFilterableMultiselect', () => {

wrapper
.find('.bx--checkbox-label')
.at(0)
.at(1)
.simulate('click');
expect(mockProps.onChange).toHaveBeenCalledTimes(4);
expect(mockProps.onChange).toHaveBeenCalledWith({
Expand All @@ -634,6 +634,7 @@ describe('NestedFilterableMultiselect', () => {
.at(0)
.find('span')
.simulate('click');
// Check the first suboption
wrapper
.find('.bx--checkbox-label')
.at(1)
Expand All @@ -659,6 +660,7 @@ describe('NestedFilterableMultiselect', () => {
).toBe(true);
expect(wrapper.find('ListBoxSelection').prop('selectionCount')).toBe(1);

// Check the second suboption
wrapper
.find('.bx--checkbox-label')
.at(2)
Expand All @@ -683,7 +685,8 @@ describe('NestedFilterableMultiselect', () => {
.prop('indeterminate')
).toBe(false);
expect(wrapper.find('ListBoxSelection').prop('selectionCount')).toBe(2);
// Un-select the child items

// Un-select the first suboption
wrapper
.find('.bx--checkbox-label')
.at(1)
Expand All @@ -708,9 +711,10 @@ describe('NestedFilterableMultiselect', () => {
).toBe(true);
expect(wrapper.find('ListBoxSelection').prop('selectionCount')).toBe(1);

// Un-select the second suboption
wrapper
.find('.bx--checkbox-label')
.at(1)
.at(2)
.simulate('click');
expect(mockProps.onChange).toHaveBeenCalledTimes(4);
expect(mockProps.onChange).toHaveBeenCalledWith({
Expand Down Expand Up @@ -914,14 +918,13 @@ describe('NestedFilterableMultiselect', () => {
.at(1)
.simulate('click');
expect(mockProps.onChange).toHaveBeenCalledTimes(1);
// checked item is now at the top
expect(
wrapper.find('Checkbox[name="Nested item 2"]').prop('indeterminate')
).toBe(false);
// expand suboptions
wrapper
.find('.bx--checkbox-label')
.at(0)
.at(1)
.find('span')
.simulate('click');
expect(
Expand All @@ -930,15 +933,15 @@ describe('NestedFilterableMultiselect', () => {
// unselect subOption
wrapper
.find('.bx--checkbox-label')
.at(2)
.at(3)
.simulate('click');
expect(
wrapper.find('Checkbox[name="Nested item 2"]').prop('indeterminate')
).toBe(true);
// expand subChild
wrapper
.find('.bx--checkbox-label')
.at(1)
.at(2)
.find('span')
.simulate('click');
expect(
Expand All @@ -947,7 +950,7 @@ describe('NestedFilterableMultiselect', () => {
// unselect subChild
wrapper
.find('.bx--checkbox-label')
.at(2)
.at(3)
.simulate('click');
expect(
wrapper.find('Checkbox[name="Sub item 1"]').prop('indeterminate')
Expand All @@ -970,16 +973,16 @@ describe('NestedFilterableMultiselect', () => {
expect(
wrapper.find('Checkbox[name="Nested item 2"]').prop('checked')
).toBe(true);
// checked item is now at the top, expand suboptions
// expand suboptions
wrapper
.find('.bx--checkbox-label')
.at(0)
.at(1)
.find('span')
.simulate('click');
// unselect 1 subOption
wrapper
.find('.bx--checkbox-label')
.at(1)
.at(2)
.simulate('click');
expect(
wrapper.find('Checkbox[name="Nested item 2"]').prop('indeterminate')
Expand All @@ -990,7 +993,7 @@ describe('NestedFilterableMultiselect', () => {
// unselect 2 subOption
wrapper
.find('.bx--checkbox-label')
.at(1)
.at(3)
.simulate('click');
expect(
wrapper.find('Checkbox[name="Nested item 2"]').prop('checked')
Expand All @@ -1010,16 +1013,16 @@ describe('NestedFilterableMultiselect', () => {
expect(
wrapper.find('Checkbox[name="Nested item 2"]').prop('checked')
).toBe(true);
// checked item is now at the top, expand suboptions
// expand suboptions
wrapper
.find('.bx--checkbox-label')
.at(0)
.at(1)
.find('span')
.simulate('click');
// unselect 1 subOption
wrapper
.find('.bx--checkbox-label')
.at(2)
.at(3)
.simulate('click');
expect(
wrapper.find('Checkbox[name="Nested item 2"]').prop('indeterminate')
Expand All @@ -1030,13 +1033,13 @@ describe('NestedFilterableMultiselect', () => {
// expand subChild
wrapper
.find('.bx--checkbox-label')
.at(1)
.at(2)
.find('span')
.simulate('click');
// unselect 1 subChild
wrapper
.find('.bx--checkbox-label')
.at(2)
.at(3)
.simulate('click');
expect(
wrapper.find('Checkbox[name="Sub item 1"]').prop('indeterminate')
Expand All @@ -1053,7 +1056,7 @@ describe('NestedFilterableMultiselect', () => {
// unselect 2 subChild
wrapper
.find('.bx--checkbox-label')
.at(2)
.at(4)
.simulate('click');
expect(
wrapper.find('Checkbox[name="Sub item 1"]').prop('indeterminate')
Expand Down
56 changes: 0 additions & 56 deletions src/components/MultiSelect/tools/__tests__/sorting-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,62 +89,6 @@ describe('defaultSortItems', () => {
]);
});

it('should order a selected item before all other options', () => {
const mockItems = ['Option 1', 'Option 10', 'Option 11', 'Option 2'].map(
label => ({ id: label, label })
);

// Set `selectedItems` to ['Option 11']
mockOptions.selectedItems = [mockItems[2]];

expect(defaultSortItems(mockItems, mockOptions)).toEqual([
{
id: 'Option 11',
label: 'Option 11',
},
{
id: 'Option 1',
label: 'Option 1',
},
{
id: 'Option 2',
label: 'Option 2',
},
{
id: 'Option 10',
label: 'Option 10',
},
]);
});

it('should sort selected items and order them before all other options', () => {
const mockItems = ['Option 1', 'Option 10', 'Option 11', 'Option 2'].map(
label => ({ id: label, label })
);

// Set `selectedItems` to ['Option 11', 'Option 2']
mockOptions.selectedItems = [mockItems[2], mockItems[3]];

expect(defaultSortItems(mockItems, mockOptions)).toEqual([
{
id: 'Option 2',
label: 'Option 2',
},
{
id: 'Option 11',
label: 'Option 11',
},
{
id: 'Option 1',
label: 'Option 1',
},
{
id: 'Option 10',
label: 'Option 10',
},
]);
});

it('should sort parent and child', () => {
const mockItems = [
{
Expand Down
17 changes: 1 addition & 16 deletions src/components/MultiSelect/tools/sorting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,10 @@ export const defaultCompareItems = (itemA, itemB, { locale }) =>
*/
export const defaultSortItems = (
items,
{ selectedItems, itemToString, compareItems, locale = 'en' }
{ itemToString, compareItems, locale = 'en' }
) => {
const itemArr = [...items];
return items.sort((itemA, itemB) => {
const hasItemA = selectedItems.some(item => item.id === itemA.id);
const hasItemB = selectedItems.some(item => item.id === itemB.id);

const hierarchyA = buildHierarchy(itemA, itemArr);
const hierarchyB = buildHierarchy(itemB, itemArr);
const depth =
Expand All @@ -85,18 +82,6 @@ export const defaultSortItems = (
return -1;
}

const hasCurrentA = selectedItems.some(item => item.id === currentA.id);
const hasCurrentB = selectedItems.some(item => item.id === currentB.id);

// Prefer whichever item is in the `selectedItems` array first
if (hasCurrentA && !hasCurrentB) {
return -1;
}

if (hasCurrentB && !hasCurrentA) {
return 1;
}

compareResult = compareItems(
itemToString(currentA),
itemToString(currentB),
Expand Down

0 comments on commit 00091a7

Please sign in to comment.