-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
feat(stats): make rowcount more human readable #8232
feat(stats): make rowcount more human readable #8232
Conversation
|
||
return intlFormat(num); | ||
}; | ||
export const needsFormatting = (num: number) => isThousands(num); |
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.
Started down a path of trying to check equality of 3500 vs 3.5K and with internationalization in play (Intl.NumberFormat) I don't feel like parsing here is going to be super reliable on machines with other languages. Future improvement to make for sure.
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.
Non-binding +1 from me!
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.
looks good to me!
Formats the numeric display for the row count (10K, 10M, etc) like other numbers around Datahub. At some point we will want to consolidate
src/utils/formatter/index.ts
andsrc/app/shared/formatNumber.ts
. They're similar but not quite, one uses theIntl.NumberFormat
which may be a more flexible approach.To still provide a way for users to get to the true number values in the UI, you can still hover the row/cols and they'll expand. A tooltip was another approach that felt a little too busy on the UI and covers up the dataset name. This feels a little more out of the way and inline with the rest of the content. I didn't refactor all the fields to use this new approach right away because that would take quite a bit more time and we'll want to see how people feel about this new approach before building upon it further (or rolling it back).
Screen.Recording.2023-06-14.at.9.27.51.AM.mov
Autocomplete
Checklist