Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #4555 - DataTable: Add nullSortOrder prop #4570

Merged
merged 4 commits into from
Oct 10, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion components/lib/calendar/Calendar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -2671,7 +2671,7 @@ export default {
let innerHTML = '';

if (this.responsiveOptions) {
const comparer = new Intl.Collator(undefined, { numeric: true }).compare;
const comparer = ObjectUtils.localeComparator();
let responsiveOptions = [...this.responsiveOptions].filter((o) => !!(o.breakpoint && o.numMonths)).sort((o1, o2) => -1 * comparer(o1.breakpoint, o2.breakpoint));

for (let i = 0; i < responsiveOptions.length; i++) {
Expand Down
13 changes: 3 additions & 10 deletions components/lib/carousel/Carousel.vue
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ import ChevronLeftIcon from 'primevue/icons/chevronleft';
import ChevronRightIcon from 'primevue/icons/chevronright';
import ChevronUpIcon from 'primevue/icons/chevronup';
import Ripple from 'primevue/ripple';
import { DomHandler, UniqueComponentId } from 'primevue/utils';
import { DomHandler, UniqueComponentId, ObjectUtils } from 'primevue/utils';
import BaseCarousel from './BaseCarousel.vue';

export default {
Expand Down Expand Up @@ -548,20 +548,13 @@ export default {

if (this.responsiveOptions && !this.isUnstyled) {
let _responsiveOptions = [...this.responsiveOptions];
const comparer = new Intl.Collator(undefined, { numeric: true }).compare;
const comparer = ObjectUtils.localeComparator();

_responsiveOptions.sort((data1, data2) => {
const value1 = data1.breakpoint;
const value2 = data2.breakpoint;
let result = null;

if (value1 == null && value2 != null) result = -1;
else if (value1 != null && value2 == null) result = 1;
else if (value1 == null && value2 == null) result = 0;
else if (typeof value1 === 'string' && typeof value2 === 'string') result = comparer(value1, value2);
else result = value1 < value2 ? -1 : value1 > value2 ? 1 : 0;

return -1 * result;
return ObjectUtils.sort(value1, value2, -1, comparer);
});

for (let i = 0; i < _responsiveOptions.length; i++) {
Expand Down
4 changes: 4 additions & 0 deletions components/lib/datatable/BaseDataTable.vue
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ export default {
type: Number,
default: 1
},
nullSortOrder: {
type: Number,
default: 1
},
multiSortMeta: {
type: Array,
default: null
Expand Down
5 changes: 5 additions & 0 deletions components/lib/datatable/DataTable.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -887,6 +887,11 @@ export interface DataTableProps {
* Order to sort the data by default.
*/
sortOrder?: number | undefined;
/**
* Determines how null values are sorted.
* @defaultValue 1
*/
nullSortOrder?: number;
/**
* Default sort order of an unsorted column.
* @defaultValue 1
Expand Down
38 changes: 12 additions & 26 deletions components/lib/datatable/DataTable.vue
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,7 @@ export default {
d_rows: this.rows,
d_sortField: this.sortField,
d_sortOrder: this.sortOrder,
d_nullSortOrder: this.nullSortOrder,
d_multiSortMeta: this.multiSortMeta ? [...this.multiSortMeta] : [],
d_groupRowsSortMeta: null,
d_selectionKeys: null,
Expand Down Expand Up @@ -381,6 +382,9 @@ export default {
sortOrder(newValue) {
this.d_sortOrder = newValue;
},
nullSortOrder(newValue) {
this.d_nullSortOrder = newValue;
},
multiSortMeta(newValue) {
this.d_multiSortMeta = newValue;
},
Expand Down Expand Up @@ -530,27 +534,19 @@ export default {
}

let data = [...value];
let resolvedFieldDatas = new Map();
let resolvedFieldData = new Map();

for (let item of data) {
resolvedFieldDatas.set(item, ObjectUtils.resolveFieldData(item, this.d_sortField));
resolvedFieldData.set(item, ObjectUtils.resolveFieldData(item, this.d_sortField));
}

const comparer = new Intl.Collator(undefined, { numeric: true }).compare;
const comparer = ObjectUtils.localeComparator();

data.sort((data1, data2) => {
let value1 = resolvedFieldDatas.get(data1);
let value2 = resolvedFieldDatas.get(data2);

let result = null;
let value1 = resolvedFieldData.get(data1);
let value2 = resolvedFieldData.get(data2);

if (value1 == null && value2 != null) result = -1;
else if (value1 != null && value2 == null) result = 1;
else if (value1 == null && value2 == null) result = 0;
else if (typeof value1 === 'string' && typeof value2 === 'string') result = comparer(value1, value2);
else result = value1 < value2 ? -1 : value1 > value2 ? 1 : 0;

return this.d_sortOrder * result;
return ObjectUtils.sort(value1, value2, this.d_sortOrder, comparer, this.d_nullSortOrder);
});

return data;
Expand Down Expand Up @@ -579,23 +575,13 @@ export default {
multisortField(data1, data2, index) {
const value1 = ObjectUtils.resolveFieldData(data1, this.d_multiSortMeta[index].field);
const value2 = ObjectUtils.resolveFieldData(data2, this.d_multiSortMeta[index].field);
let result = null;

if (typeof value1 === 'string' || value1 instanceof String) {
if (value1.localeCompare && value1 !== value2) {
const comparer = new Intl.Collator(undefined, { numeric: true }).compare;

return this.d_multiSortMeta[index].order * comparer(value1, value2);
}
} else {
result = value1 < value2 ? -1 : 1;
}
const comparer = ObjectUtils.localeComparator();

if (value1 === value2) {
return this.d_multiSortMeta.length - 1 > index ? this.multisortField(data1, data2, index + 1) : 0;
}

return this.d_multiSortMeta[index].order * result;
return ObjectUtils.sort(value1, value2, this.d_multiSortMeta[index].order, comparer, this.d_nullSortOrder);
},
addMultiSortField(field) {
let index = this.d_multiSortMeta.findIndex((meta) => meta.field === field);
Expand Down
11 changes: 2 additions & 9 deletions components/lib/dataview/DataView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -110,20 +110,13 @@ export default {
sort() {
if (this.value) {
const value = [...this.value];
const comparer = new Intl.Collator(undefined, { numeric: true }).compare;
const comparer = ObjectUtils.localeComparator();

value.sort((data1, data2) => {
let value1 = ObjectUtils.resolveFieldData(data1, this.sortField);
let value2 = ObjectUtils.resolveFieldData(data2, this.sortField);
let result = null;

if (value1 == null && value2 != null) result = -1;
else if (value1 != null && value2 == null) result = 1;
else if (value1 == null && value2 == null) result = 0;
else if (typeof value1 === 'string' && typeof value2 === 'string') result = comparer(value1, value2);
else result = value1 < value2 ? -1 : value1 > value2 ? 1 : 0;

return this.sortOrder * result;
return ObjectUtils.sort(value1, value2, this.sortOrder, comparer);
});

return value;
Expand Down
13 changes: 3 additions & 10 deletions components/lib/galleria/GalleriaThumbnails.vue
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ import ChevronLeftIcon from 'primevue/icons/chevronleft';
import ChevronRightIcon from 'primevue/icons/chevronright';
import ChevronUpIcon from 'primevue/icons/chevronup';
import Ripple from 'primevue/ripple';
import { DomHandler } from 'primevue/utils';
import { DomHandler, ObjectUtils } from 'primevue/utils';

export default {
name: 'GalleriaThumbnails',
Expand Down Expand Up @@ -438,20 +438,13 @@ export default {

if (this.responsiveOptions && !this.isUnstyled) {
this.sortedResponsiveOptions = [...this.responsiveOptions];
const comparer = new Intl.Collator(undefined, { numeric: true }).compare;
const comparer = ObjectUtils.localeComparator();

this.sortedResponsiveOptions.sort((data1, data2) => {
const value1 = data1.breakpoint;
const value2 = data2.breakpoint;
let result = null;

if (value1 == null && value2 != null) result = -1;
else if (value1 != null && value2 == null) result = 1;
else if (value1 == null && value2 == null) result = 0;
else if (typeof value1 === 'string' && typeof value2 === 'string') result = comparer(value1, value2);
else result = value1 < value2 ? -1 : value1 > value2 ? 1 : 0;

return -1 * result;
return ObjectUtils.sort(value1, value2, -1, comparer);
});

for (let i = 0; i < this.sortedResponsiveOptions.length; i++) {
Expand Down
30 changes: 7 additions & 23 deletions components/lib/treetable/TreeTable.vue
Original file line number Diff line number Diff line change
Expand Up @@ -429,20 +429,13 @@ export default {
},
sortNodesSingle(nodes) {
let _nodes = [...nodes];
const comparer = new Intl.Collator(undefined, { numeric: true }).compare;
const comparer = ObjectUtils.localeComparator();

_nodes.sort((node1, node2) => {
const value1 = ObjectUtils.resolveFieldData(node1.data, this.d_sortField);
const value2 = ObjectUtils.resolveFieldData(node2.data, this.d_sortField);
let result = null;

if (value1 == null && value2 != null) result = -1;
else if (value1 != null && value2 == null) result = 1;
else if (value1 == null && value2 == null) result = 0;
else if (typeof value1 === 'string' && typeof value2 === 'string') result = comparer(value1, value2);
else result = value1 < value2 ? -1 : value1 > value2 ? 1 : 0;

return this.d_sortOrder * result;
return ObjectUtils.sort(value1, value2, this.d_sortOrder, comparer);
});

return _nodes;
Expand All @@ -462,22 +455,13 @@ export default {
multisortField(node1, node2, index) {
const value1 = ObjectUtils.resolveFieldData(node1.data, this.d_multiSortMeta[index].field);
const value2 = ObjectUtils.resolveFieldData(node2.data, this.d_multiSortMeta[index].field);
let result = null;

if (value1 == null && value2 != null) result = -1;
else if (value1 != null && value2 == null) result = 1;
else if (value1 == null && value2 == null) result = 0;
else {
if (value1 === value2) {
return this.d_multiSortMeta.length - 1 > index ? this.multisortField(node1, node2, index + 1) : 0;
} else {
if ((typeof value1 === 'string' || value1 instanceof String) && (typeof value2 === 'string' || value2 instanceof String))
return this.d_multiSortMeta[index].order * new Intl.Collator(undefined, { numeric: true }).compare(value1, value2);
else result = value1 < value2 ? -1 : 1;
}
const comparer = ObjectUtils.localeComparator();

if (value1 === value2) {
return this.d_multiSortMeta.length - 1 > index ? this.multisortField(node1, node2, index + 1) : 0;
}

return this.d_multiSortMeta[index].order * result;
return ObjectUtils.sort(value1, value2, this.d_multiSortMeta[index].order, comparer);
},
filter(value) {
let filteredNodes = [];
Expand Down
31 changes: 31 additions & 0 deletions components/lib/utils/ObjectUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,37 @@ export default {
return index;
},

sort(value1, value2, order = 1, comparator, nullSortOrder = 1) {
const result = this.compare(value1, value2, comparator, order);
let finalSortOrder = order;

// nullSortOrder == 1 means Excel like sort nulls at bottom
if (this.isEmpty(value1) || this.isEmpty(value2)) {
finalSortOrder = nullSortOrder === 1 ? order : nullSortOrder;
}

return finalSortOrder * result;
},

compare(value1, value2, comparator, order = 1) {
let result = -1;
const emptyValue1 = this.isEmpty(value1);
const emptyValue2 = this.isEmpty(value2);

if (emptyValue1 && emptyValue2) result = 0;
else if (emptyValue1) result = order;
else if (emptyValue2) result = -order;
else if (typeof value1 === 'string' && typeof value2 === 'string') result = comparator(value1, value2);
else result = value1 < value2 ? -1 : value1 > value2 ? 1 : 0;

return result;
},

localeComparator() {
//performance gain using Int.Collator. It is not recommended to use localeCompare against large arrays.
return new Intl.Collator(undefined, { numeric: true }).compare;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The collator only needs to be instantiated once, not once per function call to get the collator.

const localeComparator = new Intl.Collator(undefined, { numeric: true }).compare;
export default {
   ...
   ...
   localeComparator() {
      return localeComparator;
   }
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct in PrimeReact we only instantiate it once outside of the loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MoMack20 I have a new PR in to add a localeCode configuration for localization, so I think that might not entirely be true with this change (or I can revisit it in that PR): #4628

},

nestedKeys(obj = {}, parentKey = '') {
return Object.entries(obj).reduce((o, [key, value]) => {
const currentKey = parentKey ? `${parentKey}.${key}` : key;
Expand Down
2 changes: 2 additions & 0 deletions components/lib/utils/Utils.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ export declare class ObjectUtils {
static isPrintableCharacter(char: string): boolean;
static findLast(value: any[], callback: () => any): any;
static findLastIndex(value: any[], callback: () => any): number;
static sort(value1: any, value2: any, order: number, comparator: (a: any, b: any) => any, nullSortOrder: number): number;
static compare(value1: any, value2: any, comparator: (a: any, b: any) => any, order: number): number;
static nestedKeys(obj: object, parentKey?: string): string[];
}

Expand Down
10 changes: 9 additions & 1 deletion doc/common/apidoc/index.json
Original file line number Diff line number Diff line change
Expand Up @@ -18854,6 +18854,14 @@
"default": "",
"description": "Order to sort the data by default."
},
{
"name": "nullSortOrder",
"optional": true,
"readonly": false,
"type": "number",
"default": "1",
"description": "Determines how null values are sorted."
},
{
"name": "defaultSortOrder",
"optional": true,
Expand Down Expand Up @@ -55597,4 +55605,4 @@
}
}
}
}
}
Loading