-
Notifications
You must be signed in to change notification settings - Fork 28
Improve formatting of route distance/duration #359
Conversation
af70eb9
to
9a3b13d
Compare
|
||
export function formatDuration(sec) { | ||
sec = Math.max(60, sec); // For duration < 60s, return '1 min' | ||
let min = Math.round(sec / 60); |
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'm surprised eslint didn't say that it should be const.
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.
Oh nevermind. Didn't see below. :)
9a3b13d
to
7445a55
Compare
if (m > 99000) { | ||
return `${Math.round(m / 1000)} km`; | ||
} | ||
if (m > 1000) { |
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.
We had a debate about this but I still prefer using else if
in such cases. But you can completely ignore this comment.
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.
Seriously, this pyramid was ridiculous…
if (m > 5) {
if (m > 1000) {
if (m > 99000) {
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 wasn't my point. I didn't like that pyramid either.
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.
👍
Description
Why
Improve legibility with respect to common typographic rules.
We should actually do that depending on the locale, but let's improve it later, this fixes most cases.
Test suite