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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add toggle to display real times #79

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

Kineolyan
Copy link
Contributor

This small PR adds a toggle to show real times of component operations.

image

For those who love precision 馃暀

@ogorodnikoff2012
Copy link
Contributor

ogorodnikoff2012 commented Mar 21, 2024

Do we really need to call Math.round here?

const createTimeFormatter = (
values: readonly number[]
): ((time: number) => string) => {
const maxValue = Math.max(...values);
if (maxValue < 1000) {
return (t) => `${t} ms`;
} else if (maxValue < 60000) {
return (t) => `${Math.round(t / 1000).toFixed(2)} s`;
} else {
return (t) => `${Math.round(t / 60000).toFixed(1)} mins`;
}
};

For me, "5.00 s (5333 ms)" looks a bit confusing. "5.33 s (5333 ms)" would look better IMHO.
Otherwise, LGTM 馃檲

@Kineolyan
Copy link
Contributor Author

Do we really need to call Math.round here?

No we don't. It was an initial approach that I should have discarded. I fixed it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants