bug(table-data-source): Sorting of a string column/property breaks if one record contains a number only

[stnor]

Reproduction

[edit, thanks @jelbourn]
https://stackblitz.com/edit/angular-xpwhup?file=src/app/table-sorting-example.ts

Expected Behavior

The sorting of strings should work as one expects when values are strings.

Actual Behavior

The sortingDataAccessor won't sort a string column correctly if one or more records contain a number only eg '1'.

Ex: ['One' ,'2', 'Three']

Suggested change

Use typeof value === 'number' instead of _isNumberValue perhaps, or something more elaborate?

Environment

• Angular: 10
• CDK/Material:10

[jelbourn]

Reproduction: https://stackblitz.com/edit/angular-xpwhup?file=src/app/table-sorting-example.ts

Confirmed that this happens and that it's weird

[Hugoer]

Hi! I could try to take care of that myself. The solution I propose is to compare the data type after the call to _isNumberValue, if one of them is numeric and the other string, it would convert the numeric value to string, so if both are numeric the sort by number would be kept (faster ) and if not it would be ordered by text. I did a test on the stackblitz mentioned by jelbourn and it would work

[jelbourn]

@andrewseguin thoughts?

[sahilmore-git]

fix(material/table): Sorting of a string column/property breaks if one record contains a number only

If the number is a string and in between strings we treat it as a number and group it at the start/end of the table based on it's ascending or descending. I have fixed the issue in my commit here and I think it will work properly.

Fixes #20140

[sahilmore-git]

@jelbourn Please correct me if I am wrong.

[sahilmore-git]

@stnor any views ??

[stnor]

LGTM

[Solonel]

Hello, I still have the issue on Angular 13.3.3, value 058233665663 is not place well
Example on mat table that didn't work :
https://stackblitz.com/edit/angular-mw3b1x?file=src/app/table-basic-example.html
Example with sort header that work :
https://stackblitz.com/edit/angular-fsdnz2?file=src/app/sort-overview-example.ts

[viniciusschuelter]

I probably fixed that #25444, @andrewseguin @crisbeto can u take a look?

[angular-automatic-lock-bot]

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.}