-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Localize durations using Intl.NumberFormat #18067
Conversation
return ` | ||
${hass.localize("ui.duration.day", { count: d })} | ||
${h}:${leftPad(m)}:${leftPad(s)} | ||
`; |
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.
return ` | |
${hass.localize("ui.duration.day", { count: d })} | |
${h}:${leftPad(m)}:${leftPad(s)} | |
`; | |
return hass.localize("ui.duration.day", { count: d }) + `${h}:${leftPad(m)}:${leftPad(s)}`; |
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 generally prefer template strings above concatenation, it is easier to read and causes less mistakes (like you forgot the space between day and hours there)
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.
Yeah but the original has a load of whitespace, what if it gets passed to somewhere that doesn't do whitespace collapsing?
We can btw also use something like this: Intl.NumberFormat("en", {
style: "unit",
unit: "second",
unitDisplay: "long",
}).format(23) -> |
Or even in the localization if we want to (we dont 😄 ):
|
hass.localize("ui.duration.day", { count: d }) + | ||
` ${h}:${leftPad(m)}:${leftPad(s)}` | ||
); | ||
return `${Intl.NumberFormat("en", { |
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.
Can we pass the locale of the user?
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.
And move it to format date/format time
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 think you may have reviewed the wrong commit. User locale is already passed in.
@@ -19,6 +19,7 @@ import { HomeAssistant } from "../../types"; | |||
import { fireEvent } from "../../common/dom/fire_event"; | |||
import { haStyle } from "../../resources/styles"; | |||
import "../../components/ha-top-app-bar-fixed"; | |||
import "../../resources/intl-polyfill"; |
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.
To prevent having to manage this everywhere, we should just move the format function to a file that imports this.
Proposed change
Localize durations using Intl.NumberFormat
It's not entirely clear why some statements are displayed as "04:05:06" instead of 4 hours, 5 minutes and 6 seconds.
Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed: