Refactored columnSorting plugin - stage 2 #5228
Conversation
| sort(column, order) { | ||
| this.setSortingColumn(column, order); | ||
| sort(column, order = this.getNextOrderState(column)) { | ||
| const allowSorting = this.hot.runHooks('beforeColumnSort', column, order); |
There was a problem hiding this comment.
Is it necessary to run a hook, even if a column is undefined?
| } | ||
| } | ||
|
|
||
| return ASC_SORT_STATE; |
There was a problem hiding this comment.
Could you use return once? With the cached result in a variable.
@budnix mentioned that will be useful when we implement automatic code coverage.
There was a problem hiding this comment.
For discussion with @jansiegel. I will remember that.
| const sortFunction = this.getSortFunctionForColumn(columnMeta); | ||
| const visualColumn = this.hot.toVisualColumn(this.sortColumn); | ||
| const columnMeta = this.hot.getCellMeta(0, visualColumn); | ||
| const sortFunction = getSortFunctionForColumn(columnMeta); | ||
| const emptyRows = this.hot.countEmptyRows(); | ||
| let nrOfRows; |
There was a problem hiding this comment.
Maybe nrOfRows could be const?
| return column; | ||
| }; | ||
|
|
||
| const hot = handsontable({ |
There was a problem hiding this comment.
You can use only handsontable({}).
| getPlugin('ColumnSorting').sort(0, 'asc'); | ||
|
|
||
| // changing column order: 0 <-> 1 | ||
| hot.updateSettings({modifyCol}); |
There was a problem hiding this comment.
Just updateSettings() should be called in proper context.
| const sortedColumn = this.$container.find('th span.columnSorting')[1]; | ||
| const afterValue = window.getComputedStyle(sortedColumn, ':after').getPropertyValue('content'); | ||
|
|
||
| expect(afterValue.indexOf(String.fromCharCode(9650))).toBeGreaterThan(-1); |
There was a problem hiding this comment.
Maybe you can cache String.fromCharCode(9650) once at a very beginning, with a comment, what char it should be.
…ing data when maxRows option is set #5064
Context
How has this been tested?
Manual tests on column sorting, including changing the order of columns.
Types of changes
Related issue(s):
Checklist: