Skip to content

Commit

Permalink
fix: Wrong sort priorities 4653 and 4196
Browse files Browse the repository at this point in the history
4653: sort Priority 1 is presented as 0 in the sort object. It shows true for !column.sort.priority so it should be checked with column.sort.priority === undefined.
4196: add function to decrease higher priorities if a column sort with lower priority is removed.

Added test cases to verify behaviour.
  • Loading branch information
jgrasl committed May 30, 2016
1 parent 68681d7 commit 17296cd
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 2 deletions.
16 changes: 14 additions & 2 deletions src/js/core/factories/Grid.js
Expand Up @@ -1979,7 +1979,7 @@ angular.module('ui.grid')
// Get the actual priority since there may be columns which have suppressRemoveSort set
column.sort.priority = self.getNextColumnSortPriority();
}
else if (!column.sort.priority){
else if (column.sort.priority === undefined){
column.sort.priority = self.getNextColumnSortPriority();
}

Expand All @@ -1997,7 +1997,7 @@ angular.module('ui.grid')
if (column.sortDirectionCycle[i]) {
column.sort.direction = column.sortDirectionCycle[i];
} else {
column.sort = {};
removeSortOfColumn(column, self);
}
}
else {
Expand All @@ -2009,6 +2009,18 @@ angular.module('ui.grid')
return $q.when(column);
};

var removeSortOfColumn = function removeSortOfColumn(column, grid) {
//Decrease priority for every col where priority is higher than the removed sort's priority.
grid.columns.forEach(function (col) {
if (col.sort && col.sort.priority !== undefined && col.sort.priority > column.sort.priority) {
col.sort.priority -= 1;
}
});

//Remove sort
column.sort = {};
};

/**
* communicate to outside world that we are done with initial rendering
*/
Expand Down
26 changes: 26 additions & 0 deletions test/unit/core/factories/Grid.spec.js
Expand Up @@ -810,6 +810,32 @@ describe('Grid factory', function () {
expect( column.sort.direction ).toEqual(uiGridConstants.ASC);
expect( column.sort.priority ).toEqual(0);
});

it( 'if two column has sort 1 and 2 on the ui which is 0 and 1 in the sort object and the sort change for the first do not change the priority', function() {
var priorColumn1 = new GridColumn({ name: 'a', sort: { direction: uiGridConstants.ASC, priority: 0 } });
var priorColumn2 = new GridColumn({ name: 'b', sort: { direction: uiGridConstants.ASC, priority: 1 } });
grid.columns.push( priorColumn1 );
grid.columns.push( priorColumn2 );

grid.sortColumn( priorColumn1, true );

expect( priorColumn1.sort ).toEqual({ direction: uiGridConstants.DESC, priority: 0});
});

it( 'if three column has sort 1,2 and 3 on the ui which is 0,1 and 2 in the sort object and the sort removed for the second decrease priority for the third but do not change for the first', function() {
var priorColumn1 = new GridColumn({ name: 'a', sort: { direction: uiGridConstants.ASC, priority: 0 } });
var priorColumn2 = new GridColumn({ name: 'b', sort: { direction: uiGridConstants.DESC, priority: 1 } });
var priorColumn3 = new GridColumn({ name: 'c', sort: { direction: uiGridConstants.ASC, priority: 2 } });
grid.columns.push( priorColumn1 );
grid.columns.push( priorColumn2 );
grid.columns.push( priorColumn3 );

grid.sortColumn( priorColumn2, true );

expect( priorColumn1.sort ).toEqual({ direction: uiGridConstants.ASC, priority: 0 });
expect( priorColumn2.sort ).toEqual({ });
expect( priorColumn3.sort ).toEqual({ direction: uiGridConstants.ASC, priority: 1 });
});
});


Expand Down

0 comments on commit 17296cd

Please sign in to comment.