diff --git a/src/cdk/coercion/number-property.ts b/src/cdk/coercion/number-property.ts index 38d783974a6..ecfc17d781f 100644 --- a/src/cdk/coercion/number-property.ts +++ b/src/cdk/coercion/number-property.ts @@ -10,8 +10,16 @@ export function coerceNumberProperty(value: any): number; export function coerceNumberProperty(value: any, fallback: D): number | D; export function coerceNumberProperty(value: any, fallbackValue = 0) { + return _isNumberValue(value) ? Number(value) : fallbackValue; +} + +/** + * Whether the provided value is considered a number. + * @docs-private + */ +export function _isNumberValue(value: any): boolean { // parseFloat(value) handles most of the cases we're interested in (it treats null, empty string, // and other non-number values as NaN, where Number just uses 0) but it considers the string // '123hello' to be a valid number. Therefore we also check if Number(value) is NaN. - return isNaN(parseFloat(value as any)) || isNaN(Number(value)) ? fallbackValue : Number(value); + return !isNaN(parseFloat(value as any)) && !isNaN(Number(value)); } diff --git a/src/lib/table/table-data-source.ts b/src/lib/table/table-data-source.ts index ee96a393282..d660696f437 100644 --- a/src/lib/table/table-data-source.ts +++ b/src/lib/table/table-data-source.ts @@ -15,6 +15,7 @@ import {combineLatest} from 'rxjs/operators/combineLatest'; import {map} from 'rxjs/operators/map'; import {startWith} from 'rxjs/operators/startWith'; import {empty} from 'rxjs/observable/empty'; +import {_isNumberValue} from '@angular/cdk/coercion'; /** * Data source that accepts a client-side data array and includes native support of filtering, @@ -88,7 +89,8 @@ export class MatTableDataSource implements DataSource { private _paginator: MatPaginator|null; /** - * Data accessor function that is used for accessing data properties for sorting. + * Data accessor function that is used for accessing data properties for sorting through + * the default sortData function. * This default function assumes that the sort header IDs (which defaults to the column name) * matches the data's properties (e.g. column Xyz represents data['Xyz']). * May be set to a custom function for different behavior. @@ -98,21 +100,54 @@ export class MatTableDataSource implements DataSource { sortingDataAccessor: ((data: T, sortHeaderId: string) => string|number) = (data: T, sortHeaderId: string): string|number => { const value: any = data[sortHeaderId]; + return _isNumberValue(value) ? Number(value) : value; + } - // If the value is a string and only whitespace, return the value. - // Otherwise +value will convert it to 0. - if (typeof value === 'string' && !value.trim()) { - return value; - } + /** + * Gets a sorted copy of the data array based on the state of the MatSort. Called + * after changes are made to the filtered data or when sort changes are emitted from MatSort. + * By default, the function retrieves the active sort and its direction and compares data + * by retrieving data using the sortingDataAccessor. May be overridden for a custom implementation + * of data ordering. + * @param data The array of data that should be sorted. + * @param sort The connected MatSort that holds the current sort state. + */ + sortData: ((data: T[], sort: MatSort) => T[]) = (data: T[], sort: MatSort): T[] => { + const active = sort.active; + const direction = sort.direction; + if (!active || direction == '') { return data; } - return isNaN(+value) ? value : +value; + return data.sort((a, b) => { + let valueA = this.sortingDataAccessor(a, active); + let valueB = this.sortingDataAccessor(b, active); + + // If both valueA and valueB exist (truthy), then compare the two. Otherwise, check if + // one value exists while the other doesn't. In this case, existing value should come first. + // This avoids inconsistent results when comparing values to undefined/null. + // If neither value exists, return 0 (equal). + let comparatorResult = 0; + if (valueA && valueB) { + // Check if one value is greater than the other; if equal, comparatorResult should remain 0. + if (valueA > valueB) { + comparatorResult = 1; + } else if (valueA < valueB) { + comparatorResult = -1; + } + } else if (valueA) { + comparatorResult = 1; + } else if (valueB) { + comparatorResult = -1; + } + + return comparatorResult * (direction == 'asc' ? 1 : -1); + }); } /** * Checks if a data object matches the data source's filter string. By default, each data object * is converted to a string of its properties and returns true if the filter has * at least one occurrence in that string. By default, the filter string has its whitespace - * trimmed and the match is case-insensitive. May be overriden for a custom implementation of + * trimmed and the match is case-insensitive. May be overridden for a custom implementation of * filter matching. * @param data Data object used to check against the filter. * @param filter Filter string that has been set on the data source. @@ -172,7 +207,7 @@ export class MatTableDataSource implements DataSource { _filterData(data: T[]) { // If there is a filter string, filter out data that does not contain it. // Each data object is converted to a string using the function defined by filterTermAccessor. - // May be overriden for customization. + // May be overridden for customization. this.filteredData = !this.filter ? data : data.filter(obj => this.filterPredicate(obj, this.filter)); @@ -188,16 +223,9 @@ export class MatTableDataSource implements DataSource { */ _orderData(data: T[]): T[] { // If there is no active sort or direction, return the data without trying to sort. - if (!this.sort || !this.sort.active || this.sort.direction == '') { return data; } + if (!this.sort) { return data; } - const active = this.sort.active; - const direction = this.sort.direction; - - return data.slice().sort((a, b) => { - let valueA = this.sortingDataAccessor(a, active); - let valueB = this.sortingDataAccessor(b, active); - return (valueA < valueB ? -1 : 1) * (direction == 'asc' ? 1 : -1); - }); + return this.sortData(data.slice(), this.sort); } /** diff --git a/src/lib/table/table.spec.ts b/src/lib/table/table.spec.ts index 2b2f1e360f6..c6957ffdcb6 100644 --- a/src/lib/table/table.spec.ts +++ b/src/lib/table/table.spec.ts @@ -209,6 +209,42 @@ describe('MatTable', () => { ['a_2', 'b_2', 'c_2'], ['a_3', 'b_3', 'c_3'], ]); + + // Expect that empty string row comes before the other values + component.sort.sort(component.sortHeader); + fixture.detectChanges(); + expectTableToMatchContent(tableElement, [ + ['Column A\xa0Sorted by a descending', 'Column B', 'Column C'], + ['a_3', 'b_3', 'c_3'], + ['a_2', 'b_2', 'c_2'], + ['', 'b_1', 'c_1'], + ]); + }); + + it('should by default correctly sort undefined values', () => { + // Activate column A sort + dataSource.data[0].a = undefined; + + // Expect that undefined row comes before the other values + component.sort.sort(component.sortHeader); + fixture.detectChanges(); + expectTableToMatchContent(tableElement, [ + ['Column A\xa0Sorted by a ascending', 'Column B', 'Column C'], + ['', 'b_1', 'c_1'], + ['a_2', 'b_2', 'c_2'], + ['a_3', 'b_3', 'c_3'], + ]); + + + // Expect that undefined row comes after the other values + component.sort.sort(component.sortHeader); + fixture.detectChanges(); + expectTableToMatchContent(tableElement, [ + ['Column A\xa0Sorted by a descending', 'Column B', 'Column C'], + ['a_3', 'b_3', 'c_3'], + ['a_2', 'b_2', 'c_2'], + ['', 'b_1', 'c_1'], + ]); }); it('should be able to page the table contents', fakeAsync(() => { @@ -243,9 +279,9 @@ describe('MatTable', () => { }); interface TestData { - a: string; - b: string; - c: string; + a: string|undefined; + b: string|undefined; + c: string|undefined; } class FakeDataSource extends DataSource {