-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Be case insensitive when sorting on the visualize and dashboard landing pages #13397
Be case insensitive when sorting on the visualize and dashboard landing pages #13397
Conversation
@stacey-gammon Sorry, I'm swamped... removing myself as a reviewer. 🤕 |
no problem @cjcenizal! Just wanted to make sure you were aware since it changes the visualize landing page too, but I can def find someone else for the review. |
isAscending: true, | ||
}, | ||
{ | ||
name: 'description', | ||
getValue: item => item.description, | ||
getValue: item => item.description.toLowerCase(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any chance this would throw an error if the description field is undefined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. It comes back as an empty string in practice, so no error. I could be extra safe and add a check for it, but the way the saved dashboards are filled with defaults, it will never be undefined, so that might be excessive checking. Not to say that things in the future might not change and eventually this could be undefined....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just wanted to make sure. I don't think there is a need to change it as we don't need to guard against behavior that comes from our own code :)
Hopefully we will have functional testing in place before we would ever make a breaking change in the interface of the saved object. I'm not worried about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fixes #12233
Changes dashboard and visualize page to case insensitive sorting.
Before:

After:
