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

Add two new built in sort types #3235

Merged
merged 3 commits into from
Apr 25, 2021
Merged

Conversation

goldflag
Copy link
Contributor

Add two new build in sort types. Current sort types are unchanged.
Suggested in #3093

  1. number
    Sorts only numerical values inside column.

  2. string
    Does what is suggersted in Sorting - can't sort titlecase and lowercase #3137.
    Sorts case-insensitively until there is a tie and then sorts case-sensitive.

New sort types better serve certain edge cases for columns with string or number values. Number strips out non decimal or numerical value from strings and only sorts what is left. String sorts strings case-insensitively until it reaches a title, and then it will do case-sensitive sort (TanStack#3137).
Add new built-in sort functions 'number' and 'string'
Add new sort types (number, string) to
certain columns to achieve sufficient coverage.
@vercel
Copy link

vercel bot commented Apr 25, 2021

Someone is attempting to deploy a commit to a Personal Account owned by @tannerlinsley on Vercel.

@tannerlinsley first needs to authorize it.

@tannerlinsley tannerlinsley merged commit fa662bc into TanStack:master Apr 25, 2021
@tannerlinsley
Copy link
Collaborator

🎉 This PR is included in version 7.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

let aa = a.shift()
let bb = b.shift()

let alower = aa.toLowerCase()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this use localeCompare and avoid all of the case conversions? This will give the wrong results in a lot of locales.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it probably should. @goldflag would you please open a new PR for this?

export function string(rowA, rowB, columnId) {
let [a, b] = getRowValuesByColumnID(rowA, rowB, columnId)

a = a.split('').filter(Boolean)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems this throws an exception if the value of a or b are null

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened an issue to track this:
#3269


const replaceNonNumeric = /[^0-9.]/gi

a = Number(String(a).replace(replaceNonNumeric, ''))
Copy link

@JulienLGRD JulienLGRD Jan 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tannerlinsley This creates an absolute value right? Is that really what we want here? This will break the sort if negative and positive values are compared.
Here is a link to a sandbox that showcases the problem: https://codesandbox.io/s/adoring-satoshi-rrw6r?file=/src/App.js

gargroh added a commit to gargroh/react-table that referenced this pull request Jan 28, 2022
* fix: provide parentRows and data to accessor

* docs: add link to LineUp-lite (TanStack#3022)

see https://lineup-lite.netlify.app for documentation and details.

* docs: use sponsor iframe

* Update index.js

* docs: ad carbonads

* Update index.css

* Update index.css

* feat: Add two new built in sort types  (TanStack#3235)

* feat(sortTypes): Add string and number sort types

New sort types better serve certain edge cases for columns with string or number values. Number strips out non decimal or numerical value from strings and only sorts what is left. String sorts strings case-insensitively until it reaches a title, and then it will do case-sensitive sort (TanStack#3137).

* docs(useSortBy): Add two new sort types

Add new built-in sort functions 'number' and 'string'

* test(useSortBy): Increase coverage for useSortBy

Add new sort types (number, string) to
certain columns to achieve sufficient coverage.

* docs: fix typo (TanStack#3261)

* Update index.js

* Update index.js

* fixed the typo in CodeSandbox link for material UI (TanStack#3555)

* v7 links

* Update README.md

* Update README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants