Skip to content

Commit

Permalink
fix(core): Sort Priority Zero Based
Browse files Browse the repository at this point in the history 
Before sort priority was a mix of 0 and 1 based.
You could create columns with a sort priority of zero but once the user
clicked on a column to sort it its index would change to 1. There was no
way for the sort index to return to 0 from within the grid's menus.
This mix of 0 and 1 based sorting was confusing and created UI problems
when displaying the sort index in the column header.

Closes angular-ui#4685
BREAKING CHANGE:
**GridOptions.columnDef.sort.priority** now expects the lowest value
to be 0.
The Grid Header will display a sort priority of 0 as 1.
Using `if(col.sort.priority)` to determine if a column is sorted is no
longer valid as `0 == false`.
Saved grid objects may be affected by this.
  • Loading branch information
@JLLeitschuh
JLLeitschuh committed Dec 3, 2015
1 parent becebb5 commit 6fe0528
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 96 deletions.
6 changes: 3 additions & 3 deletions src/features/saveState/test/saveState.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -409,9 +409,9 @@ describe('ui.grid.saveState uiGridSaveStateService', function () {

expect( onSortChangedHook.calls.length ).toEqual( 1 );

expect( onSortChangedHook ).toHaveBeenCalledWith(
grid,
[ grid.getOnlyDataColumns()[3], grid.getOnlyDataColumns()[2] ]
expect( onSortChangedHook ).toHaveBeenCalledWith(
grid,
[ grid.getOnlyDataColumns()[2], grid.getOnlyDataColumns()[3] ]
);
});

Expand Down
8 changes: 4 additions & 4 deletions src/js/core/factories/Grid.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1876,12 +1876,12 @@ angular.module('ui.grid')
p = 0;

self.columns.forEach(function (col) {
if (col.sort && col.sort.priority && col.sort.priority > p) {
p = col.sort.priority;
if (col.sort && col.sort.priority !== 'undefined' && col.sort.priority >= p) {
p = col.sort.priority + 1;
}
});

return p + 1;
return p;
};

/**
Expand Down Expand Up @@ -1960,7 +1960,7 @@ angular.module('ui.grid')

if (!add) {
self.resetColumnSorting(column);
column.sort.priority = 0;
column.sort.priority = undefined;
// Get the actual priority since there may be columns which have suppressRemoveSort set
column.sort.priority = self.getNextColumnSortPriority();
}
Expand Down
72 changes: 36 additions & 36 deletions src/js/core/services/rowSorter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ var module = angular.module('ui.grid');
/**
* @ngdoc object
* @name ui.grid.class:RowSorter
* @description RowSorter provides the default sorting mechanisms,
* including guessing column types and applying appropriate sort
* @description RowSorter provides the default sorting mechanisms,
* including guessing column types and applying appropriate sort
* algorithms
*
*/
*
*/

module.service('rowSorter', ['$parse', 'uiGridConstants', function ($parse, uiGridConstants) {
var currencyRegexStr =
var currencyRegexStr =
'(' +
uiGridConstants.CURRENCY_SYMBOLS
.map(function (a) { return '\\' + a; }) // Escape all the currency symbols ($ at least will jack up this regex)
Expand Down Expand Up @@ -95,7 +95,7 @@ module.service('rowSorter', ['$parse', 'uiGridConstants', function ($parse, uiGr
* @methodOf ui.grid.class:RowSorter
* @name basicSort
* @description Sorts any values that provide the < method, including strings
* or numbers. Handles nulls and undefined through calling handleNulls
* or numbers. Handles nulls and undefined through calling handleNulls
* @param {object} a sort value a
* @param {object} b sort value b
* @returns {number} normal sort function, returns -ve, 0, +ve
Expand All @@ -120,7 +120,7 @@ module.service('rowSorter', ['$parse', 'uiGridConstants', function ($parse, uiGr
* @ngdoc method
* @methodOf ui.grid.class:RowSorter
* @name sortNumber
* @description Sorts numerical values. Handles nulls and undefined through calling handleNulls
* @description Sorts numerical values. Handles nulls and undefined through calling handleNulls
* @param {object} a sort value a
* @param {object} b sort value b
* @returns {number} normal sort function, returns -ve, 0, +ve
Expand All @@ -139,8 +139,8 @@ module.service('rowSorter', ['$parse', 'uiGridConstants', function ($parse, uiGr
* @ngdoc method
* @methodOf ui.grid.class:RowSorter
* @name sortNumberStr
* @description Sorts numerical values that are stored in a string (i.e. parses them to numbers first).
* Handles nulls and undefined through calling handleNulls
* @description Sorts numerical values that are stored in a string (i.e. parses them to numbers first).
* Handles nulls and undefined through calling handleNulls
* @param {object} a sort value a
* @param {object} b sort value b
* @returns {number} normal sort function, returns -ve, 0, +ve
Expand All @@ -154,36 +154,36 @@ module.service('rowSorter', ['$parse', 'uiGridConstants', function ($parse, uiGr
numB, // The parsed number form of 'b'
badA = false,
badB = false;

// Try to parse 'a' to a float
numA = parseFloat(a.replace(/[^0-9.-]/g, ''));

// If 'a' couldn't be parsed to float, flag it as bad
if (isNaN(numA)) {
badA = true;
}

// Try to parse 'b' to a float
numB = parseFloat(b.replace(/[^0-9.-]/g, ''));

// If 'b' couldn't be parsed to float, flag it as bad
if (isNaN(numB)) {
badB = true;
}

// We want bad ones to get pushed to the bottom... which effectively is "greater than"
if (badA && badB) {
return 0;
}

if (badA) {
return 1;
}

if (badB) {
return -1;
}

return numA - numB;
}
};
Expand All @@ -193,7 +193,7 @@ module.service('rowSorter', ['$parse', 'uiGridConstants', function ($parse, uiGr
* @ngdoc method
* @methodOf ui.grid.class:RowSorter
* @name sortAlpha
* @description Sorts string values. Handles nulls and undefined through calling handleNulls
* @description Sorts string values. Handles nulls and undefined through calling handleNulls
* @param {object} a sort value a
* @param {object} b sort value b
* @returns {number} normal sort function, returns -ve, 0, +ve
Expand All @@ -205,7 +205,7 @@ module.service('rowSorter', ['$parse', 'uiGridConstants', function ($parse, uiGr
} else {
var strA = a.toString().toLowerCase(),
strB = b.toString().toLowerCase();

return strA === strB ? 0 : (strA < strB ? -1 : 1);
}
};
Expand Down Expand Up @@ -234,7 +234,7 @@ module.service('rowSorter', ['$parse', 'uiGridConstants', function ($parse, uiGr
}
var timeA = a.getTime(),
timeB = b.getTime();

return timeA === timeB ? 0 : (timeA < timeB ? -1 : 1);
}
};
Expand All @@ -244,8 +244,8 @@ module.service('rowSorter', ['$parse', 'uiGridConstants', function ($parse, uiGr
* @ngdoc method
* @methodOf ui.grid.class:RowSorter
* @name sortBool
* @description Sorts boolean values, true is considered larger than false.
* Handles nulls and undefined through calling handleNulls
* @description Sorts boolean values, true is considered larger than false.
* Handles nulls and undefined through calling handleNulls
* @param {object} a sort value a
* @param {object} b sort value b
* @returns {number} normal sort function, returns -ve, 0, +ve
Expand All @@ -258,7 +258,7 @@ module.service('rowSorter', ['$parse', 'uiGridConstants', function ($parse, uiGr
if (a && b) {
return 0;
}

if (!a && !b) {
return 0;
}
Expand All @@ -273,17 +273,17 @@ module.service('rowSorter', ['$parse', 'uiGridConstants', function ($parse, uiGr
* @ngdoc method
* @methodOf ui.grid.class:RowSorter
* @name getSortFn
* @description Get the sort function for the column. Looks first in
* @description Get the sort function for the column. Looks first in
* rowSorter.colSortFnCache using the column name, failing that it
* looks at col.sortingAlgorithm (and puts it in the cache), failing that
* it guesses the sort algorithm based on the data type.
*
*
* The cache currently seems a bit pointless, as none of the work we do is
* processor intensive enough to need caching. Presumably in future we might
* inspect the row data itself to guess the sort function, and in that case
* it would make sense to have a cache, the infrastructure is in place to allow
* that.
*
*
* @param {Grid} grid the grid to consider
* @param {GridCol} col the column to find a function for
* @param {array} rows an array of grid rows. Currently unused, but presumably in future
Expand Down Expand Up @@ -336,7 +336,7 @@ module.service('rowSorter', ['$parse', 'uiGridConstants', function ($parse, uiGr
* @description Used where multiple columns are present in the sort criteria,
* we determine which column should take precedence in the sort by sorting
* the columns based on their sort.priority
*
*
* @param {gridColumn} a column a
* @param {gridColumn} b column b
* @returns {number} normal sort function, returns -ve, 0, +ve
Expand All @@ -358,11 +358,11 @@ module.service('rowSorter', ['$parse', 'uiGridConstants', function ($parse, uiGr
}
}
// Only A has a priority
else if (a.sort.priority || a.sort.priority === 0) {
else if (a.sort.priority || a.sort.priority === undefined) {
return -1;
}
// Only B has a priority
else if (b.sort.priority || b.sort.priority === 0) {
else if (b.sort.priority || b.sort.priority === undefined) {
return 1;
}
// Neither has a priority
Expand All @@ -379,14 +379,14 @@ module.service('rowSorter', ['$parse', 'uiGridConstants', function ($parse, uiGr
* @description Prevents the internal sorting from executing. Events will
* still be fired when the sort changes, and the sort information on
* the columns will be updated, allowing an external sorter (for example,
* server sorting) to be implemented. Defaults to false.
*
* server sorting) to be implemented. Defaults to false.
*
*/
/**
* @ngdoc method
* @methodOf ui.grid.class:RowSorter
* @name sort
* @description sorts the grid
* @description sorts the grid
* @param {Object} grid the grid itself
* @param {array} rows the rows to be sorted
* @param {array} columns the columns in which to look
Expand All @@ -398,7 +398,7 @@ module.service('rowSorter', ['$parse', 'uiGridConstants', function ($parse, uiGr
if (!rows) {
return;
}

if (grid.options.useExternalSorting){
return rows;
}
Expand Down Expand Up @@ -460,7 +460,7 @@ module.service('rowSorter', ['$parse', 'uiGridConstants', function ($parse, uiGr
idx++;
}

// Chrome doesn't implement a stable sort function. If our sort returns 0
// Chrome doesn't implement a stable sort function. If our sort returns 0
// (i.e. the items are equal), and we're at the last sort column in the list,
// then return the previous order using our custom
// index variable
Expand All @@ -477,13 +477,13 @@ module.service('rowSorter', ['$parse', 'uiGridConstants', function ($parse, uiGr
};

var newRows = rows.sort(rowSortFn);

// remove the custom index field on each row, used to make a stable sort out of unstable sorts (e.g. Chrome)
var clearIndex = function( row, idx ){
delete row.entity.$$uiGridIndex;
};
rows.forEach(clearIndex);

return newRows;
};

Expand Down
4 changes: 2 additions & 2 deletions src/templates/ui-grid/uiGridHeaderCell.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@
aria-label="{{getSortDirectionAriaLabel()}}">
<i
ng-class="{ 'ui-grid-icon-up-dir': col.sort.direction == asc, 'ui-grid-icon-down-dir': col.sort.direction == desc, 'ui-grid-icon-blank': !col.sort.direction }"
title="{{col.sort.priority ? i18n.headerCell.priority + ' ' + col.sort.priority : null}}"
title="{{col.sort.priority ? (i18n.headerCell.priority + 1) + ' ' + col.sort.priority : null}}"
aria-hidden="true">
</i>
<sub
class="ui-grid-sort-priority-number">
{{col.sort.priority}}
{{col.sort.priority + 1}}
</sub>
</span>
</div>
Expand Down
Loading

0 comments on commit 6fe0528

Please sign in to comment.