DataTable: Removable sort not working with custom `sortFunction`

[ShaneCallananAerlytix]

Describe the bug

When removableSort is set to true, it's possible for a user to click once to sort a column in ascending order, twice for descending order, and a third time to reset the sort order to its default.

However, when using this alongside a custom sortFunction the third click never triggers this function, leaving the sort order unchanged from when it was last sorted.

Should expect sortFunction to get called with an order of 0 when user removes sort on a column, but sortFunction never gets called.

Reproducer

https://codesandbox.io/p/sandbox/primereact-test-forked-8nj79t

PrimeReact version

10.2.1

React version

17.x

Language

TypeScript

Build / Runtime

Create React App (CRA)

Browser(s)

Chrome 121

Steps to reproduce the behavior

1. Open codesandbox linked
2. Open console window in the sandbox output
3. Sort the age column in ascending and descending order (notice message logged to console)
4. Sort the age column a third time to remove sorting (notice no message logged to console, indicating the sortFunction is never called)

Expected behavior

sortFunction called when removing sort from a column

[NumanAnees]

I am also facing this issue.

[iantrudell]

Also facing this issue when removableSort is true and a custom onSort is present

[KirilCycle]

I think i know why this happens ill take a look

[KirilCycle]

Bug occurs because 'onSortChange' fucntion will reset sortField/multiSortMeta

else if (props.removableSort) {
                removeSortMeta(newMetaData, multiSortMeta);
            }

 if (props.removableSort) {
                sortField = sortOrder ? sortField : null;
            }

And then inside 'proccessedData' condition wont invoke sortSingle/sortMultiple

 if (sortField || ObjectUtils.isNotEmpty(multiSortMeta)) {
                    if (props.sortMode === 'single') {
                        data = sortSingle(data, sortField, sortOrder);
                    } else if (props.sortMode === 'multiple') {
                        data = sortMultiple(data, multiSortMeta);
                    }
                }

I modified condition, and use 'columnField' ref to restore sort field

 if (sortField || ObjectUtils.isNotEmpty(multiSortMeta) || (props.removableSort && columnSortFunction.current && columnField.current)) {
                    if (props.sortMode === 'single') {
                        data = sortSingle(data, sortField || columnField.current, sortOrder);
                    } else if (props.sortMode === 'multiple') {
                        data = sortMultiple(data, multiSortMeta);
                    }
                }

I've already tested it, works fine @melloware What do you think about this approach, should I open a PR?

[deepsweech]

Bug occurs because 'onSortChange' fucntion will reset sortField/multiSortMeta

else if (props.removableSort) {
                removeSortMeta(newMetaData, multiSortMeta);
            }

 if (props.removableSort) {
                sortField = sortOrder ? sortField : null;
            }

And then inside 'proccessedData' condition wont invoke sortSingle/sortMultiple

 if (sortField || ObjectUtils.isNotEmpty(multiSortMeta)) {
                    if (props.sortMode === 'single') {
                        data = sortSingle(data, sortField, sortOrder);
                    } else if (props.sortMode === 'multiple') {
                        data = sortMultiple(data, multiSortMeta);
                    }
                }

I modified condition, and use 'columnField' ref to restore sort field

 if (sortField || ObjectUtils.isNotEmpty(multiSortMeta) || (props.removableSort && columnSortFunction.current && columnField.current)) {
                    if (props.sortMode === 'single') {
                        data = sortSingle(data, sortField || columnField.current, sortOrder);
                    } else if (props.sortMode === 'multiple') {
                        data = sortMultiple(data, multiSortMeta);
                    }
                }

I've already tested it, works fine @melloware What do you think about this approach, should I open a PR?

yes please

[abhikarsh-gupta]

@KirilCycle Were you able to open a PR for this? I intend to launch my application in a week's time and this is a bug that's bothering a lot of the users..

[KirilCycle]

@abhikarsh-gupta I don't think I'll do it, a bit busy

[KirilCycle]

@abhikarsh-gupta ill try to find this branch later, she must be ready, if it so ill open pr today

[abhikarsh-gupta]

Thank you for your help on this @KirilCycle !! will look forward to that and meanwhile also try out the approach you've mentioned.