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

Improve formatting of draws table #203

Merged
merged 2 commits into from
Aug 6, 2024
Merged

Improve formatting of draws table #203

merged 2 commits into from
Aug 6, 2024

Conversation

WardBrian
Copy link
Collaborator

This rounds to 6 digits and makes it so that resizing the tab doesn't lead to very skinny columns that wrap.

Before:
image

After:
image

@WardBrian WardBrian added the UI label Aug 5, 2024
@WardBrian WardBrian requested a review from jsoules August 5, 2024 20:45
@WardBrian
Copy link
Collaborator Author

Not sure if there is a good way to avoid the treedepth column (which is integer-valued) from getting all the .00000s -- @jsoules?

A very-JS suggestion I've seen online is "divide by 1 afterwards", which 1. makes TSC unhappy, but 2. does seem to work, hilariously

@jsoules
Copy link
Collaborator

jsoules commented Aug 5, 2024

Not sure if there is a good way to avoid the treedepth column (which is integer-valued) from getting all the .00000s -- @jsoules?

A very-JS suggestion I've seen online is "divide by 1 afterwards", which 1. makes TSC unhappy, but 2. does seem to work, hilariously

It's JavaScript, of course it makes no sense and yet works!

So a couple options here, but they're going to involve writing a helper function. Question is just what that helper function would look like. One solution would be to check if the number is an integer (or within tolerance of an integer) and if so don't do the toPrecision call. You could also do some postprocessing on the string to trim right-hand-side zeroes, up to and including a decimal point if any; could result in ragged numbers depending on the situation...

@magland
Copy link
Collaborator

magland commented Aug 5, 2024

How about detecting whether it's an integer column, and then using a different format function. It will give approximately the same result as having the conditional inside the format function... but I think it's more correct in the rare case where you have a mix of integers and non-integers in the same column.

@WardBrian
Copy link
Collaborator Author

image

@@ -108,7 +108,9 @@ const DrawsView: FunctionComponent<DrawsViewProps> = ({
>(300);
const draws2 = useMemo(() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

As long as you're changing things, might as well make this more semantic--formattedDraws or the like?

Copy link
Collaborator

@jsoules jsoules left a comment

Choose a reason for hiding this comment

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

This seems good to me.

@magland
Copy link
Collaborator

magland commented Aug 6, 2024

Looks good to me

@WardBrian WardBrian merged commit ba3756f into main Aug 6, 2024
2 checks passed
@WardBrian WardBrian deleted the draws_table_wrapping branch August 6, 2024 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants