Kendo grid groupdescriptor and UI part vs. sortdescriptor

[hidegh]

Describe the bug We're using the grid with grouping, and now we found some issues:

1. both the sorting on the group field as on the column is enabled, but they can be set different values - see first screenshot
2. if i fix the group field sort order via TS code, the UI does not changes the icon (on the 2nd picture there's a screenshot where value was set and a new dataStateChange ev. was generated
3. although the GroupDescriptors dir property allows an undefined value (and yes, in some case even that makes sense, like when group data is returned via OData as it is with our extension) - the UI does not allows such an options (always shows asc order, even with dir property not being truthy).

To Reproduce Steps to reproduce the behavior: Set up a simple grid, use one column in grouping:

1. initially groups sort direction is undefined, yet asc. sort is used - State data and UI icon mis-aligned
2. change sort order (dir) value on group / column (sort-descriptor)

Expected behavior A clear and concise description of what you expected to happen.

Screenshots If applicable, add screenshots to help explain your problem.

Problem Nr. 1 - same col. 2 diff. sort order

Problem Nr 2. - changed group descriptor order bound value, yet UI not changing sort icon

Browser Chrome (latest)

Additional context Gird 3.14.2

[danielkaradachki]

The behavior is by design. Not set dir value indicates ascending direction. We will consider providing support for separate value that indicates that indicates unsorted state.

[hidegh]

@danielkaradachki please don't forget that there's a more important BUG here: changing the sort order from the group button (State.groups) does not get automatically into the State.sort (so header) data - this allows a mismatch between them (I had to patch State in TS). it does not make sense to have different sort order inside the group and inside the sort property of the State object - and as it's there by design, they should be equal any time.

the small UI glitch is just the side effect when I try to make those 2 values equal.

actually a simple solution would be to disable the sort via group button or disable the sort via header if group button is avail (but neither is something that would make a good UX)...

[danielkaradachki]

The group dir indicates the order of the groups. The sort dir indicates the order of the items within a group.

[hidegh]

@danielkaradachki please show me a use case/scenario where those 2 sort orders might differ for the same field!

so here's a picture, I do group by job/supervisor. job is main group, supervisor is level 2 grouping. tell me what you imagine should happen when I press change the supervisor group order on the grouping header, and what different thing should happen when I change the order on the table header for the same column (supervisor).

I expect in both cases the same, sort of supervisor entries below given L1 (job) group alphabetically - either asc or desc (or not sorted at all - initially)...

[danielkaradachki]

I am still not sure if I understand the issue. Changing the sort order of the grouped column with the header should have no effect. All items in the groups have the same value for the grouped columns. If you wish to automatically sort the group when sorting with the header then you could handle the dataStateChange event and change the state.

[hidegh]

@danielkaradachki "Changing the sort order of the grouped column with the header should have no effect." ... this is exactly what I'm saying, you got the same column definition on 2 places, also the direction of sorting. so it's expectable that those 2 values are synced.

the issue is with UX as with how the sort is handled:

we do agree in this: if we have a group field and a sort there, it does not make sense to have any kind of sort on the header as inside group those values are the same.

but considering that grouping is nothing more than sorting by that field in the first place and then displaying those sorted items under a header, a bit indented...then suddenly we might agree in that that the sort on the grid headers and sort on the group header is actually the same, so they should behave...

with UX the issue is this: as a user I was used to sort via grid headers - which now suddenly does not works, despite the icon changing.

here's a sample, try out sorting of beverages via group then via grid header: https://stackblitz.com/edit/kendo-grid-group-sort?file=app/app.component.ts

TODO:

1. Allow the possibility of no sort direction defined inside GroupDescriptor and on the UI
2. Make sure that if the sort changes due pressing the group (GroupDescriptor), then the change is propoagated to the SortDescriptor and vice versa

[danielkaradachki]

Automatically sorting the group when clicking the header will be a breaking change. It also seems a bit inconsistent. Sorting the not grouped columns will change the order within the groups and sorting the grouped columns will change the order of the groups. Therefore, we will not change the current behavior for now. I can suggest to submit a feature request in our feedback portal. If the idea is popular, we will consider adding an option or changing the current behavior for future versions.

[hidegh]

@danielkaradachki

"Sorting the not grouped columns will change the order within the groups and sorting he grouped columns will change the order of the groups." - in the messages above it was already discussed, that we're talking about the same column! Also see pictures, provided sample.

There's no need for any breaking change, nor change of sorting behaviour.

Currently there's a BUG in the behaviour (see messages above), so labeling this as Enhancement is not apropiate. As it was already described, you got ambiguous data inside the grid's State object. In the provided example the state.group[by Category].dir != state.sort[by Category].dir.

Also allowing an unsorted grouping state icon (so no up/down icon) should be a possibility - that would let me at least to do the rest of necessary patches around this bug.

[hidegh]

Seems we got a communication issue, as this BUG is still marked as enhancement. So I decided to include our native english speaking tester's screenshot and comment:

The sorting done by the grouping header and the column header do not mirror each other, and this renders the sorting done by the column headers as pointless when they are grouped.

(these are my words) The issue is: same column, yet 2 different directions shown on the UI, so it's a nonsense...

[tsvetomir]

The current behavior is indeed inconsistent.

The root cause is that we have two, potentially conflicting, sources of truth for the column sort order. Ideally, there should be only one - the SortDescriptor. Ideally, grouping shouldn't define a sort order — this is an implementation detail of the Data Query library. Other implementations may be able to perform grouping without sorting.

Keeping the direction in sync for the GroupDescriptor and the SortDescriptor seems like a possible workaround until we figure out a long-term fix. See this example on StackBlitz for a reference implementation.

[hidegh]

A quick fix, that solves both group/column header sort order change and consolidation. Too verbose, but the real reason I don't like this code: I need to call it on every change manually. But I do see a possible workaround by adding this into a directive (and remembering prev. value and comparing to it) - just need to check if other events on the grid doing changes on the State would not be overriden / bypassed.

    /**
     * This function:
     * 1/ compares whether a sort-order change on the group/column occured and applies sort & direction accordingly on Group or SortDecriptor!
     * 2/ ensures that grouping fields are added to the start of State's SortDescriptor array - as we do generate OData queries based on Sortdescriptors!
     * @param oldState 
     * @param newState 
     */
    public static consolidateSortOrder(oldState: State, newState: State): State {

        if (!newState.group || !newState.group.length)
            return newState;

        let localNewState = _.cloneDeep(newState) as State;

        //
        // create sort (from group sort)

        // move group sorts into 1st positions
        // if group sorts does not exists, add them to proper place
        let newSort: SortDescriptor[] = [];

        if (localNewState.group) {
            localNewState.group.forEach(g => {

                let sortDescriptor = localNewState.sort 
                    ? localNewState.sort.find(s => s.field === g.field)
                    : undefined

                if (sortDescriptor)
                    newSort.push(sortDescriptor);
                else
                    newSort.push({ field: g.field, dir: g.dir } as SortDescriptor);
            });
        }

        if (localNewState.sort)
        localNewState.sort.forEach(s => {
                let alreadyExists = newSort.some(i => i.field === s.field);
                if (!alreadyExists)
                    newSort.push(s);
            });

        if (newSort && newSort.length)
            localNewState.sort = newSort;

        //
        // now patch changes
        let patchedSortFields: string[] = [];

        if (localNewState.group) {
            // patch SortDescriptor based on GroupDescriptor sort changes...
            localNewState.group.forEach(newGroup => {

                let oldGroup = oldState.group
                    ? oldState.group.find(og => og.field === newGroup.field)
                    : undefined;

                if (!oldGroup || oldGroup.dir != newGroup.dir) {
                    // when sort changed on GroupDescriptor (or no group desc. previously)...    
                    // ...check corresponding SortDescriptor
                    // ...patch sort order there!
                    let sortField = localNewState.sort
                        ? localNewState.sort.find(sf => sf.field === newGroup.field)
                        : undefined;

                    if (sortField && sortField.dir != newGroup.dir) {
                        sortField.dir = newGroup.dir;
                        patchedSortFields.push(newGroup.field);
                    }
                }
            });
        }

        if (localNewState.sort) {
            // patch SortDescriptor based on GroupDescriptor sort changes...
            localNewState.sort.forEach(newSort => {

                let sortFieldAlreadyPatched = patchedSortFields.some(psf => psf === newSort.field);
                if (sortFieldAlreadyPatched)
                    return;

                let oldSort = oldState.sort
                    ? oldState.sort.find(os => os.field === newSort.field)
                    : undefined;

                if (!oldSort || oldSort.dir != newSort.dir) {
                    // when sort changed on SortDescriptor (but not due previous patch)...or no prev. sort (so change as well)...
                    // ...check corresponding GroupDescriptor
                    // ...patch sort order there!
                    let groupField = localNewState.group
                        ? localNewState.group.find(gf => gf.field === newSort.field)
                        : undefined;

                    if (groupField && groupField.dir != newSort.dir) {
                        groupField.dir = newSort.dir;
                    }
                }
            });
        }

        return localNewState;
    }

[tsvetomir]

I would definitely like to put this into a directive for actual usage. This would allow some further improvements like listening on sortChange instead of comparing the state fields by reference.

The example was more or less a prototype to validate if this is the desired behavior.