Skip to content

Commit dcd7a41

Browse files
ghiscodingghiscoding-SE
ghiscoding
and
ghiscoding-SE
authored
feat(sort): add valueCouldBeUndefined column flag to help sorting (#429)
- this is an opt-in flag because it could bring unwanted behavior and for example it breaks the Row Detail UI when sorting undefined values, so user can opt-in but it's off by default Co-authored-by: Ghislain Beaulac <ghislain.beaulac@se.com>
1 parent 30cf1b4 commit dcd7a41

9 files changed

+95
-31
lines changed

‎src/app/modules/angular-slickgrid/models/column.interface.ts

+7
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,13 @@ export interface Column {
225225
/** Editor Validator */
226226
validator?: EditorValidator;
227227

228+
/**
229+
* Can the value be undefined? Typically undefined values are disregarded when sorting, when set this flag will adds extra logic to Sorting and also sort undefined value.
230+
* This is an extra flag that user has to enable by themselve because Sorting undefined values has unwanted behavior in some use case
231+
* (for example Row Detail has UI inconsistencies since undefined is used in the plugin's logic)
232+
*/
233+
valueCouldBeUndefined?: boolean;
234+
228235
/** Width of the column in pixels (number only). */
229236
width?: number;
230237
}

‎src/app/modules/angular-slickgrid/sorters/__tests__/dateIsoSorter.spec.ts

+19-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { sortByFieldType } from '../sorterUtilities';
2-
import { FieldType, SortDirectionNumber } from '../../models';
2+
import { Column, FieldType, SortDirectionNumber } from '../../models';
33

44
describe('the Date ISO (without time) Sorter', () => {
55
it('should return an array of US dates sorted ascending when only valid dates are provided', () => {
@@ -30,4 +30,22 @@ describe('the Date ISO (without time) Sorter', () => {
3030
inputArray.sort((value1, value2) => sortByFieldType(FieldType.dateIso, value1, value2, direction));
3131
expect(inputArray).toEqual(['2001-01-01', '1998-12-14', '1998-10-08', '1998-08-08', null]);
3232
});
33+
34+
it('should return a sorted ascending array and move the undefined values to the end of the array when "valueCouldBeUndefined" is set', () => {
35+
// from MDN specification quote: All undefined elements are sorted to the end of the array.
36+
const columnDef = { id: 'name', field: 'name', valueCouldBeUndefined: true } as Column;
37+
const direction = SortDirectionNumber.asc;
38+
const inputArray = ['1998-10-08', undefined, '1998-08-08', '2001-01-01', '1998-12-14'];
39+
inputArray.sort((value1, value2) => sortByFieldType(FieldType.dateIso, value1, value2, direction, columnDef));
40+
expect(inputArray).toEqual(['1998-08-08', '1998-10-08', '1998-12-14', '2001-01-01', undefined]);
41+
});
42+
43+
it('should return a sorted descending array and move the undefined values to the end of the array when "valueCouldBeUndefined" is set', () => {
44+
// from MDN specification quote: All undefined elements are sorted to the end of the array.
45+
const columnDef = { id: 'name', field: 'name', valueCouldBeUndefined: true } as Column;
46+
const direction = SortDirectionNumber.desc;
47+
const inputArray = ['1998-10-08', undefined, '1998-08-08', '2001-01-01', '1998-12-14'];
48+
inputArray.sort((value1, value2) => sortByFieldType(FieldType.dateIso, value1, value2, direction, columnDef));
49+
expect(inputArray).toEqual(['2001-01-01', '1998-12-14', '1998-10-08', '1998-08-08', undefined]);
50+
});
3351
});

‎src/app/modules/angular-slickgrid/sorters/__tests__/numericSorter.spec.ts

+24-6
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { SortDirectionNumber } from '../../models/sortDirectionNumber.enum';
1+
import { Column, SortDirectionNumber } from '../../models';
22
import { numericSorter } from '../numericSorter';
33

44
describe('the Numeric Sorter', () => {
@@ -18,11 +18,11 @@ describe('the Numeric Sorter', () => {
1818

1919
it(`should return an array with unsorted characters showing at the beginning
2020
then comes numbers sorted ascending when digits and chars are provided`, () => {
21-
const direction = SortDirectionNumber.asc;
22-
const inputArray = [4, 'y', 39, 1, -15, -2, 0, 5, 500, 50];
23-
inputArray.sort((value1, value2) => numericSorter(value1, value2, direction));
24-
expect(inputArray).toEqual(['y', -15, -2, 0, 1, 4, 5, 39, 50, 500]);
25-
});
21+
const direction = SortDirectionNumber.asc;
22+
const inputArray = [4, 'y', 39, 1, -15, -2, 0, 5, 500, 50];
23+
inputArray.sort((value1, value2) => numericSorter(value1, value2, direction));
24+
expect(inputArray).toEqual(['y', -15, -2, 0, 1, 4, 5, 39, 50, 500]);
25+
});
2626

2727
it(`should return an array with numbers sorted descending showing at the beginning then characters`, () => {
2828
const direction = SortDirectionNumber.desc;
@@ -37,4 +37,22 @@ describe('the Numeric Sorter', () => {
3737
inputArray.sort((value1, value2) => numericSorter(value1, value2, direction));
3838
expect(inputArray).toEqual(['z', 'a', '', null]);
3939
});
40+
41+
it('should return a sorted ascending array and move the undefined values to the end of the array when "valueCouldBeUndefined" is set', () => {
42+
// from MDN specification quote: All undefined elements are sorted to the end of the array.
43+
const columnDef = { id: 'name', field: 'name', valueCouldBeUndefined: true } as Column;
44+
const direction = SortDirectionNumber.asc;
45+
const inputArray = [4, undefined, 39, 1, -15, -2, 0, 5, 500, 50];
46+
inputArray.sort((value1, value2) => numericSorter(value1, value2, direction, columnDef));
47+
expect(inputArray).toEqual([-15, -2, 0, 1, 4, 5, 39, 50, 500, undefined]);
48+
});
49+
50+
it('should return a sorted descending array and move the undefined values to the end of the array when "valueCouldBeUndefined" is set', () => {
51+
// from MDN specification quote: All undefined elements are sorted to the end of the array.
52+
const columnDef = { id: 'name', field: 'name', valueCouldBeUndefined: true } as Column;
53+
const direction = SortDirectionNumber.desc;
54+
const inputArray = [4, undefined, 39, 1, -15, -2, 0, 5, 500, 50];
55+
inputArray.sort((value1, value2) => numericSorter(value1, value2, direction, columnDef));
56+
expect(inputArray).toEqual([500, 50, 39, 5, 4, 1, 0, -2, -15, undefined]);
57+
});
4058
});

‎src/app/modules/angular-slickgrid/sorters/__tests__/sorterUtilities.spec.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,19 @@ describe('sorterUtilities', () => {
66
it('should call the Sorters.numeric when FieldType is number', () => {
77
const spy = jest.spyOn(Sorters, 'numeric');
88
sortByFieldType(FieldType.number, 0, 4, SortDirectionNumber.asc, { id: 'field1', field: 'field1' });
9-
expect(spy).toHaveBeenCalledWith(0, 4, SortDirectionNumber.asc);
9+
expect(spy).toHaveBeenCalledWith(0, 4, SortDirectionNumber.asc, { id: 'field1', field: 'field1' });
1010
});
1111

1212
it('should call the Sorters.numeric when FieldType is integer', () => {
1313
const spy = jest.spyOn(Sorters, 'numeric');
1414
sortByFieldType(FieldType.integer, 0, 4, SortDirectionNumber.asc, { id: 'field1', field: 'field1' });
15-
expect(spy).toHaveBeenCalledWith(0, 4, SortDirectionNumber.asc);
15+
expect(spy).toHaveBeenCalledWith(0, 4, SortDirectionNumber.asc, { id: 'field1', field: 'field1' });
1616
});
1717

1818
it('should call the Sorters.numeric when FieldType is float', () => {
1919
const spy = jest.spyOn(Sorters, 'numeric');
2020
sortByFieldType(FieldType.float, 0, 4, SortDirectionNumber.asc, { id: 'field1', field: 'field1' });
21-
expect(spy).toHaveBeenCalledWith(0, 4, SortDirectionNumber.asc);
21+
expect(spy).toHaveBeenCalledWith(0, 4, SortDirectionNumber.asc, { id: 'field1', field: 'field1' });
2222
});
2323

2424
it('should call the Sorters.objectString when FieldType is objectString', () => {

‎src/app/modules/angular-slickgrid/sorters/__tests__/stringSorter.spec.ts

+19-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { SortDirectionNumber } from '../../models/sortDirectionNumber.enum';
1+
import { Column, SortDirectionNumber } from '../../models';
22
import { stringSorter } from '../stringSorter';
33

44
describe('the String Sorter', () => {
@@ -43,4 +43,22 @@ describe('the String Sorter', () => {
4343
inputArray.sort((value1, value2) => stringSorter(value1, value2, direction));
4444
expect(inputArray).toEqual(['zebra', 'amazon', 'abc', 'John', 'Abe', '@at', '', null, null]);
4545
});
46+
47+
it('should return a sorted ascending array and move the undefined values to the end of the array when "valueCouldBeUndefined" is set', () => {
48+
// from MDN specification quote: All undefined elements are sorted to the end of the array.
49+
const columnDef = { id: 'name', field: 'name', valueCouldBeUndefined: true } as Column;
50+
const direction = SortDirectionNumber.asc;
51+
const inputArray = ['amazon', undefined, 'zebra', undefined, '', '@at', 'John', 'Abe', 'abc'];
52+
inputArray.sort((value1, value2) => stringSorter(value1, value2, direction, columnDef));
53+
expect(inputArray).toEqual(['', '@at', 'Abe', 'John', 'abc', 'amazon', 'zebra', undefined, undefined]);
54+
});
55+
56+
it('should return a sorted descending array and move the undefined values to the end of the array when "valueCouldBeUndefined" is set', () => {
57+
// from MDN specification quote: All undefined elements are sorted to the end of the array.
58+
const columnDef = { id: 'name', field: 'name', valueCouldBeUndefined: true } as Column;
59+
const direction = SortDirectionNumber.desc;
60+
const inputArray = ['amazon', undefined, 'zebra', undefined, '', '@at', 'John', 'Abe', 'abc'];
61+
inputArray.sort((value1, value2) => stringSorter(value1, value2, direction, columnDef));
62+
expect(inputArray).toEqual(['zebra', 'amazon', 'abc', 'John', 'Abe', '@at', '', undefined, undefined]);
63+
});
4664
});

‎src/app/modules/angular-slickgrid/sorters/dateUtilities.ts

+8-7
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
import { mapMomentDateFormatWithFieldType } from '../services/utilities';
2-
import { FieldType, Sorter } from '../models/index';
2+
import { Column, FieldType, Sorter } from '../models/index';
33
import * as moment_ from 'moment-mini';
44
const moment = moment_; // patch to fix rollup "moment has no default export" issue, document here https://github.com/rollup/rollup/issues/670
55

6-
export function compareDates(value1: any, value2: any, sortDirection: number, format: string | moment_.MomentBuiltinFormat, strict?: boolean) {
6+
export function compareDates(value1: any, value2: any, sortDirection: number, sortColumn: Column, format: string | moment_.MomentBuiltinFormat, strict?: boolean) {
77
let diff = 0;
8+
const checkForUndefinedValues = sortColumn && sortColumn.valueCouldBeUndefined || false;
89

9-
if (value1 === null || value1 === '' || !moment(value1, format, strict).isValid()) {
10+
if (value1 === null || value1 === '' || (checkForUndefinedValues && value1 === undefined) || !moment(value1, format, strict).isValid()) {
1011
diff = -1;
11-
} else if (value2 === null || value2 === '' || !moment(value2, format, strict).isValid()) {
12+
} else if (value2 === null || value2 === '' || (checkForUndefinedValues && value2 === undefined) || !moment(value2, format, strict).isValid()) {
1213
diff = 1;
1314
} else {
1415
const date1 = moment(value1, format, strict);
@@ -23,10 +24,10 @@ export function compareDates(value1: any, value2: any, sortDirection: number, fo
2324
export function getAssociatedDateSorter(fieldType: FieldType): Sorter {
2425
const FORMAT = (fieldType === FieldType.date) ? moment.ISO_8601 : mapMomentDateFormatWithFieldType(fieldType);
2526

26-
return (value1: any, value2: any, sortDirection: number) => {
27+
return (value1: any, value2: any, sortDirection: number, sortColumn: Column) => {
2728
if (FORMAT === moment.ISO_8601) {
28-
return compareDates(value1, value2, sortDirection, FORMAT, false);
29+
return compareDates(value1, value2, sortDirection, sortColumn, FORMAT, false);
2930
}
30-
return compareDates(value1, value2, sortDirection, FORMAT, true);
31+
return compareDates(value1, value2, sortDirection, sortColumn, FORMAT, true);
3132
};
3233
}

‎src/app/modules/angular-slickgrid/sorters/numericSorter.ts

+5-4
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
import { Sorter } from './../models/sorter.interface';
1+
import { Column, Sorter } from './../models/index';
22

3-
export const numericSorter: Sorter = (value1: any, value2: any, sortDirection: number) => {
4-
const x = (isNaN(value1) || value1 === '' || value1 === null) ? -99e+10 : parseFloat(value1);
5-
const y = (isNaN(value2) || value2 === '' || value2 === null) ? -99e+10 : parseFloat(value2);
3+
export const numericSorter: Sorter = (value1: any, value2: any, sortDirection: number, sortColumn?: Column) => {
4+
const checkForUndefinedValues = sortColumn && sortColumn.valueCouldBeUndefined || false;
5+
const x = (isNaN(value1) || value1 === '' || value1 === null || (checkForUndefinedValues && value1 === undefined)) ? -99e+10 : parseFloat(value1);
6+
const y = (isNaN(value2) || value2 === '' || value2 === null || (checkForUndefinedValues && value2 === undefined)) ? -99e+10 : parseFloat(value2);
67
return sortDirection * (x === y ? 0 : (x > y ? 1 : -1));
78
};

‎src/app/modules/angular-slickgrid/sorters/sorterUtilities.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ export function sortByFieldType(fieldType: FieldType, value1: any, value2: any,
99
case FieldType.float:
1010
case FieldType.integer:
1111
case FieldType.number:
12-
sortResult = Sorters.numeric(value1, value2, sortDirection);
12+
sortResult = Sorters.numeric(value1, value2, sortDirection, sortColumn);
1313
break;
1414
case FieldType.date:
1515
case FieldType.dateIso:
@@ -37,13 +37,13 @@ export function sortByFieldType(fieldType: FieldType, value1: any, value2: any,
3737
case FieldType.dateTimeUsShort:
3838
case FieldType.dateTimeUsShortAmPm:
3939
case FieldType.dateTimeUsShortAM_PM:
40-
sortResult = getAssociatedDateSorter(fieldType).call(this, value1, value2, sortDirection);
40+
sortResult = getAssociatedDateSorter(fieldType).call(this, value1, value2, sortDirection, sortColumn);
4141
break;
4242
case FieldType.object:
4343
sortResult = Sorters.objectString(value1, value2, sortDirection, sortColumn);
4444
break;
4545
default:
46-
sortResult = Sorters.string(value1, value2, sortDirection);
46+
sortResult = Sorters.string(value1, value2, sortDirection, sortColumn);
4747
break;
4848
}
4949

‎src/app/modules/angular-slickgrid/sorters/stringSorter.ts

+7-6
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
1-
import { Sorter, SortDirectionNumber } from './../models/index';
1+
import { Column, Sorter, SortDirectionNumber } from './../models/index';
22

3-
export const stringSorter: Sorter = (value1: any, value2: any, sortDirection: number | SortDirectionNumber) => {
3+
export const stringSorter: Sorter = (value1: any, value2: any, sortDirection: number | SortDirectionNumber, sortColumn?: Column) => {
44
if (sortDirection === undefined || sortDirection === null) {
55
sortDirection = SortDirectionNumber.neutral;
66
}
77
let position = 0;
8+
const checkForUndefinedValues = sortColumn && sortColumn.valueCouldBeUndefined || false;
89

9-
if (value1 === null) {
10+
if (value1 === value2) {
11+
position = 0;
12+
} else if (value1 === null || (checkForUndefinedValues && value1 === undefined)) {
1013
position = -1;
11-
} else if (value2 === null) {
14+
} else if (value2 === null || (checkForUndefinedValues && value2 === undefined)) {
1215
position = 1;
13-
} else if (value1 === value2) {
14-
position = 0;
1516
} else if (sortDirection) {
1617
position = value1 < value2 ? -1 : 1;
1718
} else {

0 commit comments

Comments
 (0)