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

[useSort] Add sort order custom sortMethod #1615

Merged
merged 1 commit into from Jan 9, 2020

Conversation

gargroh
Copy link
Contributor

@gargroh gargroh commented Oct 25, 2019

Added sort order as parameter for calling sortMethod(will be useful to provide more control in custom sortMethod)

Example :- To keep empty strings at bottom when sort in asc/desc order.

function stringSort(rowA, rowB, columnID, desc) {
  let a = getRowValueByColumnID(rowA, columnID);
  let b = getRowValueByColumnID(rowB, columnID);

  return compareBasic(a, b, desc);
}

function compareBasic(a, b, desc) {
  if (a === b) {
    return 0;
  }

  if (a === '') {
    return desc ? -1 : 1;
  }

  if (b === '') {
    return desc ? 1 : -1;
  }

  return a > b ? 1 : -1;
}

Added sort order as parameter for calling sortMethod(will be useful to provide more control in custom sortMethod)
@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 4add851:

Sandbox Source
elastic-euclid-nyx7e Configuration

@tannerlinsley
Copy link
Collaborator

The sort method here is always supposed to sort ascending. It's the responsibility of the orderBy function to reverse the array if the order has been flipped.

@gargroh
Copy link
Contributor Author

gargroh commented Nov 6, 2019

To put full picture, here is the code


const SORT_RANKS = ['', null, undefined];

function basicSort(rowA: any, rowB: any, columnID: any, desc: boolean) {
  let a = getRowValueByColumnID(rowA, columnID);
  let b = getRowValueByColumnID(rowB, columnID);

  return compareBasic(a, b, desc);
}

export function compareBasic(a: any, b: any, desc: boolean) {
  // This is for date column
  if (a instanceof Date) {
    a = a.getTime();
  }
  if (b instanceof Date) {
    b = b.getTime();
  }

  // Lowercase comparison only
  a = makeMeReadyForComparison(a);
  b = makeMeReadyForComparison(b);

  if (a === b) {
    return 0;
  }

  // This is to maintain order between '', null, undefined
  if (isEmpty(a) && isEmpty(b)) {
    const rankA = SORT_RANKS.findIndex((r) => r === a);
    const rankB = SORT_RANKS.findIndex((r) => r === b);
    const order = rankA - rankB;
    return desc ? -order : order;
  }

  // This is to keep excel like behaviour, empty values in bottom
  if (isEmpty(a)) {
    return desc ? -1 : 1;
  }
  if (isEmpty(b)) {
    return desc ? 1 : -1;
  }

  return a > b ? 1 : -1;
}

function getRowValueByColumnID(row: any, columnID: any) {
  return row.values[columnID];
}

function isEmpty(value: any) {
  return typeof value === 'undefined' || value === null || value === '';
}

As a user, somebody may require special handling for undefined, null and '' sorting, which may require asc/desc here, to solve something like above problem..

@tannerlinsley
Copy link
Collaborator

So what I'm hearing here is that there are scenarios where you would always want certains values to be at the visual "bottom" end of a sort, regardless of the sort direction?

@size-plugin
Copy link
Contributor

size-plugin bot commented Jan 8, 2020

Size report for the changes in this PR:

sizes-cjs Overall size: 23.1 kB (+4 B)

 index.js ⏤  23.1 kB (+4 B)

sizes-es Overall size: 22.9 kB (+4 B)

 index.es.js ⏤  22.9 kB (+4 B)

commit: 4add851

like it?

⭐️ me 😊

@gargroh
Copy link
Contributor Author

gargroh commented Jan 9, 2020

Sort direction will matter between equal values(for stable sort), following are the snapshot of required output :-

Column 1 is Row Index
Coumn 2 is number column can be considered as Visits column of react table examples.

Image 1 - Visits are sorted in desc order

image

Image 2 - Visits are sorted in acs order
image

'', Null, Undefined will have some ranks, and stable directional sort should occur based on their rank(code - #1615 (comment)).

@tannerlinsley
Copy link
Collaborator

Okay, I'm on board now. Good job everyone :)

@tannerlinsley
Copy link
Collaborator

I'll merge this now, but we need to make sure we get the docs updated too :)

@tannerlinsley tannerlinsley merged commit 6ebf318 into TanStack:master Jan 9, 2020
@shaktals
Copy link

shaktals commented Jan 9, 2020

The need for this prop just appeared to me, amazing to find it just merged. Thanks @tannerlinsley, thanks community!!!

gargroh added a commit to gargroh/react-table that referenced this pull request Jan 10, 2020
tannerlinsley pushed a commit that referenced this pull request Jan 10, 2020
aarudenko99 pushed a commit to aarudenko99/react-datagrid that referenced this pull request Mar 26, 2020
aarudenko99 pushed a commit to aarudenko99/react-datagrid that referenced this pull request Mar 26, 2020
opensource521 pushed a commit to opensource521/react-table-maker that referenced this pull request Apr 10, 2020
@gargroh gargroh deleted the patch-8 branch May 2, 2020 15:05
zach393 pushed a commit to zach393/reactTable that referenced this pull request Dec 11, 2020
Cheetah0723 added a commit to Cheetah0723/Table that referenced this pull request Nov 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants