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

Table multi column sort functionality #966

Merged
merged 8 commits into from
Jan 13, 2018
Merged

Table multi column sort functionality #966

merged 8 commits into from
Jan 13, 2018

Conversation

bvaughn
Copy link
Owner

@bvaughn bvaughn commented Jan 13, 2018

Counter proposal for PR #946 and #957. cc @CzBuCHi

import {
  createTableMultiSort,
  Column,
  Table,
} from 'react-virtualized';

function sortCallback({
  sortBy,
  sortDirection,
}) {
  // 'sortBy' is an ordered Array of fields.
  // 'sortDirection' is a map of field name to "ASC" or "DESC" directions.
  // Sort your collection however you'd like.
  // When you're done, setState() or update your Flux store, etc.
}

const sortState = createTableMultiSort(sortCallback);

// When rendering your header columns,
// Use the sort state exposed by sortState:
const headerRenderer = ({ dataKey, label }) => {
  const showSortIndicator = sortState.sortBy.includes(dataKey);
  return (
    <>
      <span title={label}>{label}</span>
      {showSortIndicator && (
        <SortIndicator sortDirection={sortState.sortDirection[dataKey]} />
      )}
    </>
  );
};

// Connect sortState to Table by way of the `sort` prop:
<Table
  {...tableProps}
  sort={sortState.sort}
  sortBy={undefined}
  sortDirection={undefined}>
  <Column
    {...columnProps}
    headerRenderer={headerRenderer}
  />
</Table>

This proposal does not include a recommendation for how your application code should apply its multi-criteria sort, but neither did the other two pull requests.

Repository owner deleted a comment from codecov-io Jan 13, 2018
Repository owner deleted a comment from codecov-io Jan 13, 2018
Repository owner deleted a comment from codecov-io Jan 13, 2018
Repository owner deleted a comment from codecov-io Jan 13, 2018
Repository owner deleted a comment from codecov-io Jan 13, 2018
Repository owner deleted a comment from codecov-io Jan 13, 2018
@bvaughn bvaughn merged commit 0f6cac8 into master Jan 13, 2018
@bvaughn bvaughn deleted the table-multi-sort branch January 13, 2018 20:48
@CzBuCHi
Copy link

CzBuCHi commented Jan 15, 2018

i example above im guessing that createMultiSort === createTableMultiSort ...

looked at your code and im confused with createMultiSort.js:

  • dont see anywhere slice call so i think, that calling sort method will change sort defaults

  • soo im guessing, that i can delete my fork then :)

@bvaughn
Copy link
Owner Author

bvaughn commented Jan 15, 2018

i example above im guessing that createMultiSort === createTableMultiSort

Yes.

dont see anywhere slice call so i think, that calling sort method will change sort defaults

Your sortCallback should not modify the sortBy array passed in. Treat it as read-only. I could clone before passing but I didn't want to pay the cost of cloning unnecessarily.

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.

2 participants