-
Notifications
You must be signed in to change notification settings - Fork 6
Add two new helper functions to format AttemptResults #30
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
base: master
Are you sure you want to change the base?
Changes from all commits
e42fea9
4ba7b8c
508a3a4
491f9af
393af0e
9878c29
08b2b3c
376b454
1ee7980
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,7 @@ | ||
| /** | ||
| * Formats Centiseconds to a string like 3:30 or 0:43 | ||
| * @param centiTime | ||
| */ | ||
| export function formatCentiseconds(centiTime: number): string { | ||
| if (centiTime === -1) { | ||
| return 'DNF'; | ||
|
|
@@ -14,9 +18,94 @@ export function formatCentiseconds(centiTime: number): string { | |
| return `${s}.${prefix(cs)}`; | ||
| } | ||
|
|
||
| /** | ||
| * Pads out a number with a 0 if it's under 10 | ||
| * @param n | ||
| */ | ||
| function prefix(n: number): string { | ||
| if (n < 10) { | ||
| return `0${n}`; | ||
| } | ||
| return `${n}`; | ||
| } | ||
|
|
||
| export const SECOND_IN_CS = 100; | ||
| export const MINUTE_IN_CS = 60 * SECOND_IN_CS; | ||
| export const HOUR_IN_CS = 60 * MINUTE_IN_CS; | ||
|
|
||
| interface PluralizeParams { | ||
| count: number; | ||
| word: string; | ||
| options?: { | ||
| fixed?: number; | ||
| abbreviate?: boolean; | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Adds an s to a word if count is over 1. | ||
| * Takes options to pad the count and abbreviate the word with its | ||
| * first letter | ||
| * @param count | ||
| * @param word | ||
| * @param options | ||
| */ | ||
| export function pluralize({ count, word, options = {} }: PluralizeParams) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wouldn't export this function, there's libraries out there that specialize in this use-case.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just exported it for testing. How do we do that? |
||
| const countStr = | ||
| options.fixed && count % 1 > 0 ? count.toFixed(options.fixed) : count; | ||
| const countDesc = options.abbreviate | ||
| ? word[0] | ||
| : ` ${count === 1 ? word : `${word}s`}`; | ||
| return countStr + countDesc; | ||
| } | ||
|
|
||
| interface CentiSecondsToHumanReadableParams { | ||
| c: number; | ||
| options?: { | ||
| short?: boolean; | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Converts Centiseconds to a human-readable string like "5:30 minutes" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "5:30 minutes" doesn't actually make much sense...what's the use case here? I would expect "5 seconds" or "10 minutes" or "5:30" but in English, I would read the original as "5 minutes and 30 seconds minutes" Localization should somehow be taken into account for a library like this but I'm not sure if localization makes sense here...
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah sorry that comment doesn't make sense, it's 5 minutes and 30 seconds |
||
| * @param c | ||
| * @param options | ||
| */ | ||
| export function centiSecondsToHumanReadable({ | ||
|
coder13 marked this conversation as resolved.
|
||
| c, | ||
| options = {}, | ||
| }: CentiSecondsToHumanReadableParams) { | ||
| let centiseconds = c; | ||
| let str = ''; | ||
|
|
||
| const hours = centiseconds / HOUR_IN_CS; | ||
| centiseconds %= HOUR_IN_CS; | ||
| if (hours >= 1) { | ||
| str += `${pluralize({ | ||
| count: Math.floor(hours), | ||
| word: 'hour', | ||
| options: { abbreviate: options.short }, | ||
| })} `; | ||
| } | ||
|
|
||
| const minutes = centiseconds / MINUTE_IN_CS; | ||
| centiseconds %= MINUTE_IN_CS; | ||
| if (minutes >= 1) { | ||
| str += `${pluralize({ | ||
| count: Math.floor(minutes), | ||
| word: 'minute', | ||
| options: { abbreviate: options.short }, | ||
| })} `; | ||
| } | ||
|
|
||
| const seconds = centiseconds / SECOND_IN_CS; | ||
| if (seconds > 0 || str.length === 0) { | ||
| str += `${pluralize({ | ||
| count: seconds, | ||
| word: 'second', | ||
| options: { fixed: 2, abbreviate: options.short }, | ||
| })} `; | ||
| } | ||
|
|
||
| return str.trim(); | ||
| } | ||
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.
An edge case that I frequently run into is that "time" is different depending on FMC single vs average. For example
29is an FMC single score result and2933is an FMC average score result. This needs to be taken into account for them to render properly.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.
That is true, I based this function on https://github.com/thewca/worldcubeassociation.org/blob/d3338d0175c608eb34bc1d3be60034cf0ac44e46/WcaOnRails/app/webpacker/lib/utils/edit-events.js#L99 where AttemptResults can't be an average. AttemptResultQualification can though, so if we want to use the method for that also I can change it.