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.

Sign up for GitHub

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

Jump to bottom

Fix the table sorting order bug for numerical values #2587

Merged
merged 2 commits into from
Jun 19, 2017
Merged

Fix the table sorting order bug for numerical values #2587

merged 2 commits into from
Jun 19, 2017

Conversation

fbarl
Copy link
Contributor

@fbarl fbarl commented Jun 12, 2017

Fixes #2573.

@fbarl fbarl self-assigned this Jun 12, 2017
@fbarl fbarl requested a review from davkal June 12, 2017 11:48
davkal
davkal suggested changes Jun 14, 2017
View reviewed changes
Copy link
Contributor

@davkal davkal left a comment

Choose a reason for hiding this comment

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

Switching works great, but the order is now ASC by default. It should be DESC, as the user wants to see the busiest items first.

@fbarl
Copy link
Contributor Author

fbarl commented Jun 14, 2017

...the order is now ASC by default. It should be DESC, as the user wants to see the busiest items first.

@davkal I am getting DESC by default for numericals (at least for fields like CPU and Memory in Pods table from dev), for which columns did you get ASC first?

@davkal
Copy link
Contributor

davkal commented Jun 15, 2017

It's ASC on first load (when opening node details for the first time).

@fbarl
Copy link
Contributor Author

fbarl commented Jun 16, 2017

It's ASC on first load (when opening node details for the first time).

@davkal Yes, that's a good catch! I fixed it now by replacing undefined with null (that part of the code is still a bit fragile :/) and it seems to work fine now, so PTAL.

@davkal
Copy link
Contributor

davkal commented Jun 19, 2017

LGTM

@fbarl fbarl merged commit fd8f9be into master Jun 19, 2017
@fbarl fbarl deleted the fix-table-sorting-order-for-numericals branch June 19, 2017 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davkal davkal davkal approved these changes

Assignees

@fbarl fbarl

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fbarl @davkal