fix(react-grid): clear expanded groups after ungrouping #202
Conversation
| @@ -65,6 +80,27 @@ export class GroupingState extends React.PureComponent { | |||
| grouping, | |||
| ]} | |||
| /> | |||
| <Watcher | |||
There was a problem hiding this comment.
Why do you use Watcher to track changes in local Plugin context? It seems like overkill.
| describe('GroupingState', () => { | ||
| let resetConsole; | ||
| beforeAll(() => { | ||
| resetConsole = setupConsole({ ignore: ['validateDOMNesting'] }); |
There was a problem hiding this comment.
Are you sure that ignoring 'validateDOMNesting' is needed here?
| @@ -72,3 +74,37 @@ export const nextExpandedGroups = (prevExpandedGroups, groupKey) => { | |||
|
|
|||
| return expandedGroups; | |||
| }; | |||
|
|
|||
| const ungroupedColumnIndex = (prevGrouping, nextGrouping) => { | |||
| if (prevGrouping.length > nextGrouping.length) { | |||
There was a problem hiding this comment.
Why do you not use "guard if" here? It makes function much easier to read.
|
|
||
| export const expandedGroupsWithChangedGrouping = (prevGrouping, nextGrouping, expandedGroups) => { | ||
| const index = ungroupedColumnIndex(prevGrouping, nextGrouping); | ||
| if (index !== null) { |
| .join(SEPARATOR), | ||
| ); | ||
|
|
||
| result = new Set(result); |
There was a problem hiding this comment.
It is a bad practice to reassign a variable with a differed value type.
|
|
||
| const ungroupedColumnIndex = (prevGrouping, nextGrouping) => { | ||
| if (prevGrouping.length > nextGrouping.length) { | ||
| for (let i = 0; i < prevGrouping.length; i += 1) { |
There was a problem hiding this comment.
A complexity of this algorithm is too big. Feels like this function can be written with O(n) complexity.
| return null; | ||
| }; | ||
|
|
||
| export const expandedGroupsWithChangedGrouping = (prevGrouping, nextGrouping, expandedGroups) => { |
There was a problem hiding this comment.
The name of the function does not describe the result.
|
|
||
| export const expandedGroupsDependOnGrouping = (prevGrouping, nextGrouping, expandedGroups) => { | ||
| const index = ungroupedColumnIndex(prevGrouping, nextGrouping); | ||
| if (index === null) { |
There was a problem hiding this comment.
-1 is commonly used if an index is not found.
| return result; | ||
| }; | ||
|
|
||
| const uniqueArrayWithoutEmptyStrings = (array) => { |
There was a problem hiding this comment.
array.filter(s => s) seems to be simplier and can be inlined
| return Array.from(tmpSet); | ||
| }; | ||
|
|
||
| export const expandedGroupsDependOnGrouping = (prevGrouping, nextGrouping, expandedGroups) => { |
…oups-after-ungrouping
No description provided.