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

sortDirection cannot be applied for more than one column. #966

Closed
T-pirithiviraj opened this issue Sep 29, 2019 · 18 comments · Fixed by #971
Closed

sortDirection cannot be applied for more than one column. #966

T-pirithiviraj opened this issue Sep 29, 2019 · 18 comments · Fixed by #971
Labels
bug wontfix Support for this request is not planned at this time

Comments

@T-pirithiviraj
Copy link

If we apply sortDirection for more than one column, it is throwing an error.

Expected Behavior

sortDirection should work for all the column in case we update the state.

Current Behavior

Open the console, apply sort direction for more than one column it is throwing an error as sortDirection can be applied for only one column.

Steps to Reproduce (for bugs)

  1. open the console.
  2. In the state, define sortDirection in an array.
  3. Include this sort direction from state in column.
  4. Now reload the dataTable, It is throwing an error as sortDirection can applied for only one column, if we try to use more than one column it will take the first column only.
    Sample code:
this.state: {
             page: 0,
          count: 1,
          changedColumn: null,
          sortDirection: ['asc','none','none','none','none','none','none', 'none'], ...
     }
sortColumnChange(sortDirection) {
this.setState ...
}
render() {
const columns = [
            {
                name: 'reAlias',
                label: 'Alias',
                options: { sortDirection: this.state.sortDirection[0] }
            },
            {
                name: 'cookieGroup',
                label: 'Group',
                options: { sortDirection: this.state.sortDirection[1] }
            }, ...
}
## Your Environment
<!--- Include as many relevant details about the environment with which you experienced the bug. -->

| Tech         | Version |
|--------------|---------|
| Material-UI  |  |
| MUI-datatables  |    2.11.1     |
| React        |         |
| browser      |     chrome    |
| etc          |         |
@gabrielliwerant
Copy link
Collaborator

In order to apply a new sort direction, you have to remove one of the existing sort directions. Sorting along one than one column at once is not possible, as I understand it (not possible even in theory).

@gabrielliwerant gabrielliwerant added the wontfix Support for this request is not planned at this time label Sep 29, 2019
@danielbyun
Copy link
Contributor

danielbyun commented Sep 30, 2019

Hi @gabrielliwerant ,

seems pretty redundant he's creating new issues for the basically same issue, but I'm not sure if I'm understanding correctly.

I understand we have to keep the values in the state that we want to track while using serverSide = true, and I am doing that to toggle from 'asc' and 'desc' when listening to an event from onColumnSortChange from the options object I'm defining.

example:
onColumnSortChange: (changedColumn, direction) => {
console.log(changedColumn, direction);
let order = "desc";

    if (direction === "ascending") order = "asc";
    if (direction === "descending") order = "desc";

    switch (changedColumn) {
      case "id":
        console.log(`changedColumn: ${changedColumn}, order: ${order}`);
        return this.sortChange(changedColumn, order);
      case "name":
        return this.sortChange(changedColumn, order);
      case "height":
        return this.sortChange(changedColumn, order);
      case "weight":
        return this.sortChange(changedColumn, order);
      case "age":
        return this.sortChange(changedColumn, order);
      case "premiumStart":
        return this.sortChange(changedColumn, order);
      case "premiumEnd":
        return this.sortChange(changedColumn, order);
      default:
        return null;
    }
  },

which in the console I get a response like this: changedColumn: id, order: desc

But in order to apply whether a column is ascending or descending, we have to supply the sortDirection to the column we want to sort, correct?

but when we want to update the sortDirection with the value (the order value) we get after fetching the data, like @T-pirithiviraj stated, it gives an error that sortDirection cannot be applied to multiple columns.

If this is the case, where I cannot supply sortDirection option to the columns (more than one), how do I approach sorting for individual columns? They currently all sort individually but it does not toggle from 'desc' to 'asc' because I cannot update this value to the column.

Sorry for the lengthy question.
And I apologize in advance if I'm not understanding something on my end, as I am pretty new to React.js and in development in general.

Thank you for this wonderful library.

@T-pirithiviraj
Copy link
Author

Thanks @danielbyun , That was my exact question too. We are doing set state with the direction but how to update that sortDirection in individual column if it is serverSide = true ?

sorry for the repeated same issue. I am not getting clear picture on server side how to use? since I am new to react and this library.

@gabrielliwerant
Copy link
Collaborator

sortDirection: 'none' removes the current sort direction, so you have to set that before you can apply another one to another column.

@wdh2100
Copy link
Collaborator

wdh2100 commented Sep 30, 2019

Unlike the previous sortDirection: nullvalue, the use of sortDirection: 'none'value is confusing

@danielbyun
Copy link
Contributor

Hello,

Here is a small example of what I'm trying to do. I've actually pulled this from @gabrielliwerant 's example for issue #468 .

https://codesandbox.io/s/muidatatables-custom-toolbar-tl55g

As you can see in the console, sortDirection is set for more than one column. Only the first column will be considered. warning is given.

If we just use your example, the 'Name' column sorts fine (whether it actually sorts perfectly, doesn't matter). How would I approach filtering the 'Title' column as well, if needed, filtering all of the columns (individually)?

*note: not trying to apply multiple sorting, at least not yet.

I keep track of nameColumnSortDirection in my state and assign that to sortDirection for my first column. But I need to have sortDirection for my other columns as well to keep track of the 'direction' it's going.

Thank you for your guidance.
Once again, I really appreciate this library. Thank you in advance!

@gabrielliwerant
Copy link
Collaborator

gabrielliwerant commented Sep 30, 2019

Hello @danielbyun. Firstly, you'd have to track each column sort direction separately. As of now, if you use the same nameColumnSortDirection for all of them, it will try to set all of them at once, and you'll get the error. You could try an array instead:

(psuedocode)

state = {
  columnSortDirections: [
    'none',
    'none',
    'asc',
    'none'
  ]
};

const columns = {
  {
    name: 'Name',
    options: { sortDirection: this.state.columnSortDirections[0] }
  },
  {
    name: 'Location',
    options: { sortDirection: this.state.columnSortDirections[1] }
  },
  {
    name: 'Title',
    options: { sortDirection: this.state.columnSortDirections[2] }
  },
  {
    name: 'Salary',
    options: { sortDirection: this.state.columnSortDirections[3] }
  }
};

Then you can try to set it accordingly so that only one is sorted at a time:

(psuedocode)

const mySortHandler = (column, sortDirection, data) => {
  let newColumnSortDirections = ['none', 'none', 'none', 'none'];
  newColumnSortDirections[column.index] = sortDirection;

  this.setState({ columnSortDirections: newColumnSortDirections });
}

@danielbyun
Copy link
Contributor

Thank you for your response and your pseudocode.

The root of the question is that when sortDirection is defined at all (no matter if the value is different from the other columns) to multiple columns as you have included, it will throw the sortDirection is set for more than one column. Only the first column will be considered. error in the console and sorting will not behave normally.

@T-pirithiviraj
Copy link
Author

T-pirithiviraj commented Oct 1, 2019

Thanks for the response @gabrielliwerant .
yeah @danielbyun , That is why i raised this issue. I apologize in advance if I'm not understanding something on my end. But the fact is I am getting error in console, if we apply sortDirection: key for more than one column.

@danielbyun
Copy link
Contributor

https://codesandbox.io/s/muidatatables-custom-toolbar-f2uc1

The sortName function is probably not set up completely, but here is an example of the errors that display on the console when the option sortDirection is set to multiple columns.

@gabrielliwerant
Copy link
Collaborator

gabrielliwerant commented Oct 1, 2019

@T-pirithiviraj and @danielbyun You cannot set sort direction for more than one column. Only one column can be sorted at a time. That's just what is physically possible, it's not a library constraint (though the library prevents it from entering into a wonky state by cutting off the attempt and displaying the message so that you know to change your code).

@gabrielliwerant
Copy link
Collaborator

Are you saying that this is not working for 'none' also? If so that's a bug. But your example has a bunch of directions set, so I'm not sure.

@gabrielliwerant
Copy link
Collaborator

Not being able to set more than one sort direction at once is intentional and won't be fixed, however, 'none' does not count as a direction, and should be allowed. Pardon my confusion, as I couldn't understand the issue based on the attempts to apply sort direction, whereas what should be allowed is removing a sort direction with the 'none' option.

@T-pirithiviraj
Copy link
Author

Are you saying that this is not working for 'none' also? If so that's a bug. But your example has a bunch of directions set, so I'm not sure.

yeah @gabrielliwerant . This is not working for none also.If you set none for other columns, the console error is there.

@danielbyun
Copy link
Contributor

Huh.

Then I definitely am understanding the implementation of sortDirection on the columns wrong.

Hey @T-pirithiviraj , do you mind making a small example on codeSandBox on how you're implementing the sortDirection so that each column can be sorted individually?

But your example has a bunch of directions set, so I'm not sure.

Seems like I'm not doing it correctly as @gabrielliwerant stated.

@danielbyun
Copy link
Contributor

Hey @gabrielliwerant , thank you and great work!

I finally understand how it works now, and it works wonderfully.

I might make a small little example to show how it works for future reference because I have referred to many examples that so many users here have contributed.

Thanks again for the library! Also thanks to @T-pirithiviraj for creating three issues for this sorting problem haha.

Sorting Example

@gabrielliwerant
Copy link
Collaborator

Glad to hear it @danielbyun! Any interest in adding a serverside sorting example to this repo? I would be glad to consider adding it.

@danielbyun
Copy link
Contributor

Sure thing, @gabrielliwerant !
Will get to it this weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug wontfix Support for this request is not planned at this time
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants