Igniteui-angular: ixgGrid GroupBy feature should have the ability to hide column when grouped

[MikeTechSpeak]

Description

As a developer I want to hide my columns when grouped and show them again when ungrouped
Currently there is not property on the grid or column to say hidewhengrouped

There is a onGroupedDone event that would be viable to have the developer do this himself, but the eventArgs is an array of the currently sorted columns.
The problem lies with ungrouping, the previously grouped column is not in that list.
So to achieve this you need to retain previous sortingExpressions and see which column is missing.

This adds a lot of unneeded burden on the developer to manage what should be a simple property setting to handle this commonly used behavior.

[PavlovVasil]

@MikeTechSpeak, @kdinev, @chronossf

I would suggest the following changes to the Group By specification to be made:

• onGroupingDone event should also get raised when a column gets ungrouped – this makes the name ambiguous. As it is a single event and is used both for grouping and ungrouping, a more suitable name would be onGroupingChanged. The event would implement an IGroupingChangedEventArgs interface which might look like this:

export interface IGroupingChangedEventArgs {
    oldExpressions: Array<ISortingExpression> | null;
    newExpressions: Array<ISortingExpression> | null;
    newlyGroupedColumns: Array<string> | null;
    newlyUngroupedColumns: Array<string> | null;
}

• Since the clearGrouping method should raise the onGroupingChanged event, it should have an overload that allows the user to pass an array of column names to be ungrouped at once. Currently it has to be called separately for each column about to be ungrouped, which would raise the event multiple times.

• It would be useful for the developer to have access both to the old and the new sorting expressions when grouping/ungrouping occurs and these expressions have to be accessible in the event args. Two new event properties – oldSortingExpressions and newSortingExpressions would contain the two arrays.

• The event arguments have to contain the difference between the new and old sorting expressions – this way the last grouped/ungrouped columns would be clearly visible and easily manageable by the developer. Two new event properties – newlyGroupedColumns and newlyUngroupedColumns would contain the last grouping/ungrouping changes.

[ChronosSF]

Looks good. The newlyGroupedColumns and newlyUngroupedColumns (something shorter would be nice though) should be of type

Array<IgxColumnComponent>

In this way the user should be able to do

newlyGroupedColumns.forEach(x => x.hide()); 
newlyUngroupedColumns.forEach(x => x.unhide()); 

which is quite elegant imo.

[MayaKirova]

@MikeTechSpeak @ChronosSF @PavlovVasil @kdinev @mpavlinov
I’d just like to mention that since we are working with Angular here and not pure js we have a ton of mechanism available out of the box which we can use to detect changes on a collection. I don’t necessarily see the need to expose 4 event arguments just for the sake of comparing the state of 2 collections.

I have attached a sample for your reference: SampleGB.zip, which utilizes IterableDiffers to track the changes on the grid’s groupingExpressions collection so that it reacts when a new group expression is added/removed and hides/shows the related column.

Here is a code snippet for your reference on how this looks:

public ngDoCheck(): void {
    const changes = this._differ.diff(this.grid.groupingExpressions);
    if (changes) {
      changes.forEachAddedItem((rec) => {
        const col = this.grid.getColumnByName(rec.item.fieldName);
        col.hidden = true;
      });
      changes.forEachRemovedItem((rec) => {
        const col = this.grid.getColumnByName(rec.item.fieldName);
        col.hidden = false;
      });
    }
}

To me this looks a lot simpler and more natural in the context of an Angular app.
Let me know what you think.

[mpavlinov]

@kdinev @ChronosSF @rkaraivanov @SlavUI @PavlovVasil
Here are my 2 cents. If we decide that this case falls into 80 of the 80/20 rule (and I think it is), then we should introduce an option, as this will not require that developers write a code which adds value to the developer's experience.
We also should improve our API by adding context information in the respective feature events. In this case this is the GroupBy's onGroupingDone (or onGroupingChanged), but as we need to be consistent across the grid, we should review and add context information for the other features events as well.

[kdinev]

@mpavlinov I'm fine with adding an input that does this by default.

[PavlovVasil]

@MikeTechSpeak @ChronosSF @kdinev @mpavlinov @SlavUI @MayaKirova
I that case I will drop the current pull request and create a new one. Let me summarize:

1. A new boolean igxGridComponent input, called _hideGroupedColumns_, would be introduced (unless someone could think of a more appropriate name?). Its default value would be "false".
2. Still the onGroupingDone event should get raised both when grouping or ungrouping is done- currently is it raised only on grouping, which is an issue that should be addressed. As the onGroupingChanged name is more appropriate, and because this is a breaking change, the onGroupingChanged event should be added in addition to the current one. We might deprecate onGroupingDone in the next product version.
3. Grouping columns by calling the grid API and passing the same sorting expressions multiple times should not raise the onGroupingChanged event, because the grid state does not change.

As points 2 and 3 are unrelated to the original feature request, I think it would be more appropriate if I log another separate issue and change the grouping event in a separate pull request.

[mpavlinov]

@PavlovVasil Agree. Focus on point 1 in this Issue and log separate issues for 2 and 3. I'm Ok with the name hideGroupedColumns and its default value.