Skip to content

Commit

Permalink
fix(common): Date Sorting was shuffling other lines with same dates (#…
Browse files Browse the repository at this point in the history
…831)

- fixed bug mentioned in Stack Overflow: [question](https://stackoverflow.com/questions/74657131/slickgrid-header-menu-sorting-is-not-working-properly)
- for example if we had 10 lines with null values and 90 lines of valid dates, and we were sorting ascending multiple times, the dates were sorted correctly but the 10 null date lines were constantly being shuffled
  • Loading branch information
ghiscoding committed Dec 3, 2022
1 parent 9c93e7a commit db34213
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 14 deletions.
2 changes: 1 addition & 1 deletion packages/common/src/interfaces/gridOption.interface.ts
Expand Up @@ -120,7 +120,7 @@ export interface GridOption {
/**
* Defaults to false, can the cell value (dataContext) be undefined?
* Typically undefined values are disregarded when sorting, when setting this flag it will adds extra logic to Sorting and also sort undefined value.
* This is an extra flag that user has to enable by themselve because Sorting undefined values has unwanted behavior in some use case
* This is an extra flag that user has to enable by themselve because Sorting undefined values has unintended behavior in some use case
* (for example Row Detail has UI inconsistencies since undefined is used in the plugin's logic)
*/
cellValueCouldBeUndefined?: boolean;
Expand Down
34 changes: 21 additions & 13 deletions packages/common/src/sortComparers/dateUtilities.ts
@@ -1,21 +1,29 @@
import { FieldType } from '../enums/fieldType.enum';
import { Column, GridOption, SortComparer } from '../interfaces/index';
import { SortComparer } from '../interfaces/index';
import { mapMomentDateFormatWithFieldType } from '../services/utilities';
import * as moment_ from 'moment-mini';
const moment = (moment_ as any)['default'] || moment_; // patch to fix rollup "moment has no default export" issue, document here https://github.com/rollup/rollup/issues/670

export function compareDates(value1: any, value2: any, sortDirection: number, sortColumn: Column, gridOptions: GridOption, format: string | moment_.MomentBuiltinFormat, strict?: boolean) {
export function compareDates(value1: any, value2: any, sortDirection: number, format: string | moment_.MomentBuiltinFormat, strict?: boolean) {
let diff = 0;
const checkForUndefinedValues = sortColumn?.valueCouldBeUndefined ?? gridOptions?.cellValueCouldBeUndefined ?? false;
const date1 = moment(value1, format, strict);
const date2 = moment(value2, format, strict);

if (value1 === null || value1 === '' || (checkForUndefinedValues && value1 === undefined) || !date1.isValid()) {
diff = -1;
} else if (value2 === null || value2 === '' || (checkForUndefinedValues && value2 === undefined) || !date2.isValid()) {
diff = 1;
if (value1 === value2) {
diff = 0;
} else {
diff = date1.valueOf() < date2.valueOf() ? -1 : 1;
// use moment to validate the date
let date1 = moment(value1, format, strict);
let date2 = moment(value2, format, strict);

// when moment date is invalid, we'll create a temporary old Date
if (!date1.isValid()) {
date1 = new Date(1001, 1, 1);
}
if (!date2.isValid()) {
date2 = new Date(1001, 1, 1);
}

// we can use valueOf on both moment & Date to sort
diff = date1.valueOf() - date2.valueOf();
}

return sortDirection * diff;
Expand All @@ -25,10 +33,10 @@ export function compareDates(value1: any, value2: any, sortDirection: number, so
export function getAssociatedDateSortComparer(fieldType: typeof FieldType[keyof typeof FieldType]): SortComparer {
const FORMAT = (fieldType === FieldType.date) ? moment.ISO_8601 : mapMomentDateFormatWithFieldType(fieldType);

return ((value1: any, value2: any, sortDirection: number, sortColumn: Column, gridOptions: GridOption) => {
return ((value1: any, value2: any, sortDirection: number) => {
if (FORMAT === moment.ISO_8601) {
return compareDates(value1, value2, sortDirection, sortColumn, gridOptions, FORMAT, false) as number;
return compareDates(value1, value2, sortDirection, FORMAT, false) as number;
}
return compareDates(value1, value2, sortDirection, sortColumn, gridOptions, FORMAT, true) as number;
return compareDates(value1, value2, sortDirection, FORMAT, true) as number;
}) as SortComparer;
}

0 comments on commit db34213

Please sign in to comment.