Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Data Table default sorter broken when using null values #5281

Closed
Vringe opened this issue Sep 26, 2023 · 5 comments
Closed

Data Table default sorter broken when using null values #5281

Vringe opened this issue Sep 26, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@Vringe
Copy link
Contributor

Vringe commented Sep 26, 2023

TuSimple/naive-ui version (版本)

2.34.4

Vue version (Vue 版本)

3.3.4

Browser and its version (浏览器及其版本)

Chrome (117.0.5938.92)

System and its version (系统及其版本)

MacOS (13.6)

Reappearance link (重现链接)

https://codesandbox.io/s/quirky-keller-3dhh9l

Reappearance steps (重现步骤)

Sort the "Bar" column ascending or descending -> Nothing happens

Expected results (期望的结果)

Rows should be sorted

Actual results (实际的结果)

Rows are not being sorted

Remarks (补充说明)

As soon as I remove the row with the null value, sorting works again as expected.

@github-actions github-actions bot added the untriaged need to sort label Sep 26, 2023
@jizai1125
Copy link
Contributor

jizai1125 commented Sep 27, 2023

If the sorter prop is set to default, it will use Array.sort() by employing a built-in compare function.

function getDefaultSorterFn (
columnKey: ColumnKey
): (row1: InternalRowData, row2: InternalRowData) => number {
return (row1: InternalRowData, row2: InternalRowData) => {
const value1 = row1[columnKey]
const value2 = row2[columnKey]
if (typeof value1 === 'number' && typeof value2 === 'number') {
return value1 - value2
} else if (typeof value1 === 'string' && typeof value2 === 'string') {
return value1.localeCompare(value2)
}
return 0
}
}

So in your case, when null is compared with other numeric values, it will return 0, which means it will maintain the original order.

@Vringe
Copy link
Contributor Author

Vringe commented Sep 27, 2023

Thank you, I understand. I would like to discuss if this behavior is really desired or if this should be changed in the default sorter? In my opinion, columns with null values should be at the very bottom or top when sorting.

@jizai1125
Copy link
Contributor

jizai1125 commented Sep 27, 2023

Thank you, I understand. I would like to discuss if this behavior is really desired or if this should be changed in the default sorter? In my opinion, columns with null values should be at the very bottom or top when sorting.

The sorter prop also accepts your custom compare function if you don't use the built-in compare function.
Ref Uncontrolled filter and sorter.

@Vringe
Copy link
Contributor Author

Vringe commented Sep 27, 2023

Yes, I saw that. Of course, it is convenient to be able to use your own sorter functions.
However, I think that the default behavior should be changed as suggested.

How would you solve that with a custom sorter function without defining it every time? I would like to define it once globally and reference to it. But as far as I can see in the docs, only the whole row is passed to my function without knowing the actual column which makes it impossible to use it in that way.

@07akioni 07akioni added bug Something isn't working and removed untriaged need to sort labels Dec 20, 2023
@07akioni
Copy link
Collaborator

corresponding PR merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants