fix(react-grid): correlate sorting order with grouping order #414
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| ]; | ||
| let nextSorting = []; | ||
| if (keepOther === true) { | ||
| nextSorting = sorting.slice(); |
There was a problem hiding this comment.
Check case when seamless immutable is used. Probably, sorting.slice() should be replaced to Array.from.
dxbykov
reviewed
Oct 17, 2017
| this.cancelGroupingChange = this.applyReducer.bind(this, cancelGroupingChange); | ||
| this.setColumnSorting = this.setColumnSorting.bind(this); | ||
| } | ||
| getState() { |
There was a problem hiding this comment.
It's strange that 'get' function creates a new object on each call.
| this.setState(nextState); | ||
|
|
||
| const { onGroupingChange } = this.props; | ||
| if (onGroupingChange && nextState.grouping !== state.grouping) { |
There was a problem hiding this comment.
nextState.grouping, nextState.expandedGroups can be replaced to const { grouping, expandedGroups } = nextState;
SergeyAlexeev
approved these changes
Oct 18, 2017
dxbykov
reviewed
Oct 18, 2017
| @@ -64,7 +64,7 @@ tableHeaderRows | Getter | Array<[TableRow](table-view.md#table-row)> | He | |||
| sorting | Getter | Array<[Sorting](sorting-state.md#sorting)> | Column sorting. | |||
| columns | Getter | Array<[Column](#column)> | Table columns. | |||
| grouping | Getter | Array<[Grouping](grouping-state.md#grouping)> | Columns used for grouping. | |||
| setColumnSorting | Action | ({ columnName: string, direction: 'asc' | 'desc', keepOther: boolean, cancel: boolean }) => void | Changes column sorting. | |||
| setColumnSorting | Action | ({ columnName: string, direction: 'asc' | 'desc', keepOther: boolean | Array<String>, cancel: boolean }) => void | Changes a column's sort direction. Keeps existing sorting if `keepOther` is set to `true`. The `keepOther` can handle contains the names of columns will be keeped when sorting is applied. Cancels sorting by the current column if `cancel` is set to `true`. | |||
gsobolev
reviewed
Oct 18, 2017
| { columnName: 'test', direction: 'asc' }, | ||
| { columnName: 'test3', direction: 'asc' }, | ||
| ]); | ||
| it('should set corretct sorting if sortIndex is specified', () => { |
| @@ -89,7 +89,7 @@ columns | Getter | Array<[Column](#column)> | Grid columns. | |||
| draftGrouping | Getter | Array<[DraftGrouping](grouping-state.md#draft-grouping)> | Grouping options used for the preview. | |||
| sorting | Getter | Array<[Sorting](sorting-state.md#sorting)> | The current sorting state. | |||
| groupByColumn | Action | ({ columnName: string }) => void | Toggles a column's grouping state. | |||
| setColumnSorting | Action | ({ columnName: string, direction: 'asc' | 'desc', keepOther: boolean, cancel: boolean }) => void | Updates column sorting. | |||
| setColumnSorting | Action | ({ columnName: string, direction: 'asc' | 'desc', keepOther: boolean | Array<String>, cancel: boolean }) => void | Changes a column's sort direction. Keeps existing sorting if `keepOther` is set to `true`. The `keepOther` can handle contains the names of columns will be kept when sorting is applied. Cancels sorting by the current column if `cancel` is set to `true`. | |||
There was a problem hiding this comment.
Please check this sentence
The
keepOthercan handle contains the names...
| @@ -55,6 +55,8 @@ A string value that consists of values by which rows are grouped, separated by t | |||
| Name | Plugin | Type | Description | |||
| -----|--------|------|------------ | |||
| columns | Getter | Array<[Column](grid.md#column)> | The grid columns. | |||
| sorting? | Getter | Array<[Sorting](sorting-state.md#sorting)> | Applied column sorting. | |||
| setColumnSorting? | Action | ({ columnName: string, direction: 'asc' | 'desc', keepOther: boolean | Array<String>, cancel: boolean }) => void | Changes a column's sort direction. Keeps existing sorting if `keepOther` is set to `true`. The `keepOther` can handle contains the names of columns will be kept when sorting is applied. Cancels sorting by the current column if `cancel` is set to `true`. | |||
| @@ -40,4 +40,4 @@ none | |||
| Name | Plugin | Type | Description | |||
| -----|--------|------|------------ | |||
| sorting | Getter | Array<[Sorting](#sorting)> | Applied column sorting. | |||
| setColumnSorting | Action | ({ columnName: string, direction: 'asc' | 'desc', keepOther: boolean, cancel: boolean, scope: Array<String> }) => void | Changes a column's sort direction. Keeps existing sorting if `keepOther` is set to `true`. Cancels sorting by the current column if `cancel` is set to `true`. The `scope` array contains the names of columns taken into account when sorting. | |||
| setColumnSorting | Action | ({ columnName: string, direction: 'asc' | 'desc', keepOther: boolean | Array<String>, cancel: boolean }) => void | Changes a column's sort direction. Keeps existing sorting if `keepOther` is set to `true`. The `keepOther` can handle contains the names of columns will be kept when sorting is applied. Cancels sorting by the current column if `cancel` is set to `true`. | |||
| @@ -64,7 +64,7 @@ tableHeaderRows | Getter | Array<[TableRow](table-view.md#table-row)> | He | |||
| sorting | Getter | Array<[Sorting](sorting-state.md#sorting)> | Column sorting. | |||
| columns | Getter | Array<[Column](#column)> | Table columns. | |||
| grouping | Getter | Array<[Grouping](grouping-state.md#grouping)> | Columns used for grouping. | |||
| setColumnSorting | Action | ({ columnName: string, direction: 'asc' | 'desc', keepOther: boolean, cancel: boolean }) => void | Changes column sorting. | |||
| setColumnSorting | Action | ({ columnName: string, direction: 'asc' | 'desc', keepOther: boolean | Array<String>, cancel: boolean }) => void | Changes a column's sort direction. Keeps existing sorting if `keepOther` is set to `true`. The `keepOther` can handle contains the names of columns will be kept when sorting is applied. Cancels sorting by the current column if `cancel` is set to `true`. | |||
| expect(defaultDeps.action.setCurrentPage.mock.calls) | ||
| .toEqual([[2]]); | ||
| expect(defaultDeps.action.setCurrentPage.mock.calls[0][0]) | ||
| .toEqual(2); |
There was a problem hiding this comment.
Why not use the .toHaveBeenCalledWith() matcher?
There was a problem hiding this comment.
Action is called with additional arguments (getters and actions)
gsobolev
approved these changes
Oct 18, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #398
BREAKING CHANGES:
The
scopeparameter of thesetColumnSortingaction has been removedThe
GroupingStateplugin now has an optional dependency on theSortingStateplugin. So,GroupingStateshould be placed afterSortingState.Before:
After: