angular / components

Component infrastructure and Material Design components for Angular
https://material.angular.io
MIT License
24.27k stars 6.71k forks source link

[Sortable] Sorting is always done case sensitive - add an option to make sort case insensitive #9205

Closed Cyberdada closed 6 years ago

Cyberdada commented 6 years ago

Bug, feature request, or proposal:

Bug, feature request, or proposal - I guess all of the above...

What is the expected behavior?

If a flag existed that allowed you to set sort as case insensitive , a case insensitive sort would work like this: A b C

What is the current behavior?

It works like this: A C b

What are the steps to reproduce?

use https://stackblitz.com/angular/dmykovoxxbv?file=app%2Ftable-sorting-example.ts and change some of the names intial letter to lowercase

What is the use-case or motivation for changing an existing behavior?

I think more people than me need it :)

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

Angular: 5.1.0 Material: 5.0.1 Browser: Chrome TYpescript: 2.6.2

Is there anything else we should know?

eTallang commented 6 years ago

There is a function sortingDataAccessor on MatTableDataSource that can be overwritten to define how the sorting of the different columns should be performed (regarding how to access the data to sort). Setting sortingDataAccessor will solve your problem. However, every time I have used mat-table along with MatTableDataSource and sorting, I have found that I needed to overwrite the sortingDataAccessor, simply because the default data accessor behaves unexpectedly regarding handling null values, numbers vs strings, and as you have experienced, capitalized letters.

I'm not sure how this could be solved generally though, since sorting logic is highly dependent on how the data should be interpeted.

Cyberdada commented 6 years ago

Interesting, thanks. I also noticed that sorting a datecolumn is superslow... I do not understand why though. In my opinion the grid should offer basic numerical + alphanumerical case /not case sensitive + time sorting, and it should be able handle nulls/nan/undefined ;) If your needs go above that you would have to roll your own sorter.

jelbourn commented 6 years ago

Adding an option for case insensitivity would be odd in cases when people do specify a custom comparator

eTallang commented 6 years ago

@jelbourn I agree, I'm not sure providing flags for altering sorting logic is the best approach here. Could a reevaluation of the default sorting behavior be appropriate? In my opinion, the most logical approach to a default sorting function would be to make it case-insensitive, and handle null values and dates more gracefully than today.

andrewseguin commented 6 years ago

Sorting for null/undefined values has a fix here https://github.com/angular/material2/pull/8698

andrewseguin commented 6 years ago

With regard to changing case insensitivity in sort, I think its best to be explicit with how you want the data to be viewed when sorting, which is what the sortingDataAccessor is for. It should be fairly trivial to return your data with toLowerCase() when sorting. I don't expect us to change the default behavior of handling cases since it would be a large breaking change at this point for something that can easily be overriden.

LosD commented 6 years ago

@andrewseguin At least it should be documented properly with a big label of "We know that our default is weird, but we can't change it now, here's how to fix it:".

When you break the least-surprise rule, you need to be very explicit about it.

skined90 commented 6 years ago

Can someone please write an example how to override sortingDataAccessor?

LosD commented 6 years ago

@skined90 Luckily, it's pretty simple. Here I based the solution on the original StackBlitz code linked in the issue: https://stackblitz.com/edit/angular-yfw5ia?file=app/table-sorting-example.ts

Only difference is the added constructor (ngInit might be a better place, but this is what is used in our production code, so this I know works), and the data changed to get some lowercase names.

Edit: Changed to ngOnInit and tested. Works fine. I think that's preferable.

Yet another edit: My own code doesn't have any number fields, if yours do, you'll need to re-implement the default sort's way to treat numbers as a special case. Currently the sort will stop working if you try to sort one of the number columns.

skined90 commented 6 years ago

@LosD Thank you very much!

LosD commented 6 years ago

You're very welcome. :)

YakirSmartag commented 6 years ago

This is a code that does a special case for a string, but otherwise just returns the value:

this.dataSource.sortingDataAccessor = (data: any, sortHeaderId: string): string => {
  if (typeof data[sortHeaderId] === 'string') {
    return data[sortHeaderId].toLocaleLowerCase();
  }

  return data[sortHeaderId];
};
juanjinario commented 5 years ago

This is a code that does a special case for a string, but otherwise just returns the value:

this.dataSource.sortingDataAccessor = (data: any, sortHeaderId: string): string => {
  if (typeof data[sortHeaderId] === 'string') {
    return data[sortHeaderId].toLocaleLowerCase();
  }

  return data[sortHeaderId];
};

How can I sort alphabetically when I have empty fields? ex:

I've this A B "" C

And MatTable/Matsort orders "" A B C

I want that always send the gaps at the end and get: ascending A B C ""

descending C B A ""

LosD commented 5 years ago

Hi @juanjinario

It's not exactly a pretty way, but you can trick it quite easily by putting something sortable in front of the actual string, like a number (or a,b,c or whatever you prefer):

    this.dataSource.sortingDataAccessor = (data, sortHeaderId) => {
      if(!data[sortHeaderId]) {
        return this.sort.direction === "asc" ? '3' : '1';
      }
      return '2' + data[sortHeaderId].toLocaleLowerCase();
    }

(Assumes you have the matSort element as a viewChild named sort)

Edit: Changed the StackBlitz example to make use of it: https://stackblitz.com/edit/angular-yfw5ia-tewcfd?file=app/table-sorting-example.ts (I killed "Oxygen" for the demonstration).

juanjinario commented 5 years ago

Thank you, its a good way for me

amanpunj17 commented 5 years ago

Can someone please tell me how to sort a table like this? a aa ae A Aa Ae

LosD commented 5 years ago

@amanpunj17 Not sure I follow; you have two "ae", and you want one of them to come before the capital letters, and one of them come after?

If it's just "lowercase first, then uppercase" (i.e. switch around upper and lowercase order, but keep lexical order), I'd make a test for caseness, then use the trick above to order by that first.

If it's anything more complex, you'll probably have to describe how your sort order is supposed to work.

LosD commented 5 years ago

@amanpunj17 I didn't mean that I added a caseness test, just that you could replace (or add to) the current empty/null check with a caseness check to the sortingDataAccessor in the example you also linked to.

cigcurios commented 4 years ago

Hi @juanjinario

It's not exactly a pretty way, but you can trick it quite easily by putting something sortable in front of the actual string, like a number (or a,b,c or whatever you prefer):

    this.dataSource.sortingDataAccessor = (data, sortHeaderId) => {
      if(!data[sortHeaderId]) {
        return this.sort.direction === "asc" ? '3' : '1';
      }
      return '2' + data[sortHeaderId].toLocaleLowerCase();
    }

(Assumes you have the matSort element as a viewChild named sort)

Edit: Changed the StackBlitz example to make use of it: https://stackblitz.com/edit/angular-yfw5ia-tewcfd?file=app/table-sorting-example.ts (I killed "Oxygen" for the demonstration).

But what if there is a column for which type is string[]. It throws an error for string[].

LosD commented 4 years ago

@cigcurios then you'll have to make the function recognize that column (you can just look at the sortHeaderId for that), and react appropriately, e.g. sort by first element of the string array.

angular-automatic-lock-bot[bot] commented 4 years ago

This issue has been automatically locked due to inactivity. Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.