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

fix(table): empty string should be sorted right #8011

Merged
merged 2 commits into from Nov 2, 2017

Conversation

andrewseguin
Copy link
Contributor

@andrewseguin andrewseguin commented Oct 24, 2017

Previously, an empty string gets converted to 0 and comes after other values. E.g. ' ' would come after 'x'.

Fixes #7460

@andrewseguin andrewseguin added P2 The issue is important to a large percentage of users, with a workaround pr: needs review labels Oct 24, 2017
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Oct 24, 2017
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, add merge-ready when ready


// If the value is a string and only whitespace, return the value.
// Otherwise +value will convert it to 0.
if (typeof value === 'string' && !value.trim()) { return value; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This conditional is a bit long to be single-lined

@andrewseguin andrewseguin added the action: merge The PR is ready for merge by the caretaker label Oct 24, 2017
@Bsujeet
Copy link

Bsujeet commented Oct 27, 2017

i just wanted to ask if the table should sort the column where the is same i.e all the rows of that column contain same data.

this is my table before sort.

image

this is my table after sort on DiskType column..

image

do i need to change anything in my sort function...

Thanks

@andrewseguin
Copy link
Contributor Author

@Bsujeet What you are looking for is a stable sort, but the ES7 spec does not specify that native sorting must be stable. Each browser has its own way of sorting when array.sort is called and there's no consistency.

For a stable sort, you'll want to adjust your comparator to account for the object's index if the comparison is equal. For some discussion on how to do this, try starting with this StackOverflow question.

@andrewseguin andrewseguin merged commit 58627c4 into angular:master Nov 2, 2017
@Bsujeet
Copy link

Bsujeet commented Nov 3, 2017

Thanks it was helpfull

@andrewseguin andrewseguin deleted the table-fix-sort branch November 28, 2017 20:38
@andrewseguin andrewseguin restored the table-fix-sort branch November 28, 2017 20:38
@andrewseguin andrewseguin deleted the table-fix-sort branch November 28, 2017 20:39
@prabhat112
Copy link

I have 5.0.0 and I still have this issue. In which release has this been released ?

@andrewseguin
Copy link
Contributor Author

Should be out already - feel free to open an issue with reproduction steps so we can investigate if the issue persists

@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement P2 The issue is important to a large percentage of users, with a workaround
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Table] Sort example incorrectly sorts empty string
5 participants