Skip to content

fix(core): Sort Priority Zero Based #4807

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jan 22, 2016
Merged

fix(core): Sort Priority Zero Based #4807

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions src/features/saveState/test/saveState.spec.js
View file
Open in desktop
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
18 changes: 9 additions & 9 deletions src/js/core/factories/Grid.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ angular.module('ui.grid')
* @methodOf ui.grid.core.api:PublicApi
* @description The visibility of a column has changed,
* the column itself is passed out as a parameter of the event.
*
*
* @param {$scope} scope The scope of the controller. This is used to deregister this event when the scope is destroyed.
* @param {Function} callBack Will be called when the event is emited. The function passes back the GridCol that has changed.
*
Expand Down Expand Up @@ -1087,17 +1087,17 @@ angular.module('ui.grid')
* append to the newRows and add to newHash
* run the processors
* ```
*
*
* Rows are identified using the hashKey if configured. If not configured, then rows
* are identified using the gridOptions.rowEquality function
*
*
* This method is useful when trying to select rows immediately after loading data without
* using a $timeout/$interval, e.g.:
*
*
* $scope.gridOptions.data = someData;
* $scope.gridApi.grid.modifyRows($scope.gridOptions.data);
* $scope.gridApi.selection.selectRow($scope.gridOptions.data[0]);
*
*
* OR to persist row selection after data update (e.g. rows selected, new data loaded, want
* originally selected rows to be re-selected))
*/
Expand Down 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
Open in desktop
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.localeCompare(strB);
}
};
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@
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="{{isSortPriorityVisible() ? i18n.headerCell.priority + ' ' + col.sort.priority : null}}"
title="{{isSortPriorityVisible() ? i18n.headerCell.priority + ' ' + ( col.sort.priority + 1 ) : null}}"
Copy link
Contributor

Choose a reason for hiding this comment

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

In Grid.js line 1880 aren't you bumping up the sortPriority, so adding 1 again here would be incorrect?

aria-hidden="true">
</i>
<sub
ui-grid-visible="isSortPriorityVisible()"
class="ui-grid-sort-priority-number">
{{col.sort.priority}}
{{col.sort.priority + 1}}
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

</sub>
</span>
</div>
Expand Down
Loading