feat: user-selectable timezone for UI time display#29311
feat: user-selectable timezone for UI time display#29311webalexeu wants to merge 1 commit intoevcc-io:masterfrom
Conversation
|
Proposed improvement for #25354 |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
localIANATimezonefallback that returns a formatted offset string (e.g.UTC+02:00) is not a valid IANA timezone and will be rejected byIntl.DateTimeFormat/supportedValuesOf('timeZone'); consider returning a canonical name likeUTC(or mapping offsets to a stable IANA zone) when you can’t determine a real IANA identifier. parseLocalTimeInTzrelies onIntl.DateTimeFormat(...).formatToPartsround-tripping through a synthetic UTC date, which may behave unintuitively around DST transitions (ambiguous or skipped local times); it would be helpful to either guard or document how these edge cases are handled, or centralize this logic so all callers consistently deal with DST quirks.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `localIANATimezone` fallback that returns a formatted offset string (e.g. `UTC+02:00`) is not a valid IANA timezone and will be rejected by `Intl.DateTimeFormat` / `supportedValuesOf('timeZone')`; consider returning a canonical name like `UTC` (or mapping offsets to a stable IANA zone) when you can’t determine a real IANA identifier.
- `parseLocalTimeInTz` relies on `Intl.DateTimeFormat(...).formatToParts` round-tripping through a synthetic UTC date, which may behave unintuitively around DST transitions (ambiguous or skipped local times); it would be helpful to either guard or document how these edge cases are handled, or centralize this logic so all callers consistently deal with DST quirks.
## Individual Comments
### Comment 1
<location path="assets/js/mixins/formatter.ts" line_range="198" />
<code_context>
return new Intl.DurationFormat(this.$i18n?.locale, { style: "long" }).format(parts);
},
fmtDayString(date: Date) {
- const YY = `${date.getFullYear()}`;
- const MM = `${date.getMonth() + 1}`.padStart(2, "0");
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting shared Intl-based timezone formatting into small helpers and a reusable timezone resolver to reduce duplication and keep the mixin focused on display logic.
You can keep the timezone-aware behavior but trim a lot of duplication by extracting two small helpers and moving timezone resolution out of the mixin.
### 1. Factor out repeated `Intl.DateTimeFormat` + `formatToParts` logic
`fmtDayString`, `fmtTimeString`, `isToday`, and the German branch of `hourShort` all repeat the same pattern. A tiny helper will centralize that:
```ts
// in the same file, above `export default` or in a small util
function formatPartsInTz(
date: Date,
timeZone: string,
options: Intl.DateTimeFormatOptions
) {
const parts = new Intl.DateTimeFormat("en-CA", { timeZone, ...options }).formatToParts(date);
return (type: string, fallback = "00") =>
parts.find((p) => p.type === type)?.value ?? fallback;
}
```
Usage examples:
```ts
fmtDayString(date: Date) {
const get = formatPartsInTz(date, this.timezone, {
year: "numeric",
month: "2-digit",
day: "2-digit",
});
return `${get("year")}-${get("month")}-${get("day")}`;
},
fmtTimeString(date: Date) {
const get = formatPartsInTz(date, this.timezone, {
hour: "2-digit",
minute: "2-digit",
hour12: false,
});
const HH = get("hour").replace("24", "00");
const mm = get("minute");
return `${HH}:${mm}`;
},
hourShort(date: Date) {
const locale = this.$i18n?.locale;
const tz = this.timezone;
if (locale === "de") {
const get = formatPartsInTz(date, tz, {
hour: "numeric",
hour12: false,
});
return Number(get("hour", "0"));
}
return new Intl.DateTimeFormat(locale, {
hour: "numeric",
hour12: is12hFormat(),
timeZone: tz,
}).format(date);
},
```
`isToday` can also reuse `Intl.DateTimeFormat` construction via a helper (see next section).
### 2. Factor out `Intl.DateTimeFormat` creation for the current timezone
You recreate `new Intl.DateTimeFormat(this.$i18n?.locale, { …, timeZone: this.timezone })` in many methods. A small helper reduces noise and makes intent clearer:
```ts
function formatInTz(
locale: string | undefined,
timeZone: string,
options: Intl.DateTimeFormatOptions,
date: Date
) {
return new Intl.DateTimeFormat(locale, { ...options, timeZone }).format(date);
}
```
Then:
```ts
isToday(date: Date) {
const fmt = (d: Date) =>
formatInTz("en-CA", this.timezone, {
year: "numeric",
month: "2-digit",
day: "2-digit",
}, d);
return fmt(date) === fmt(new Date());
},
weekdayPrefix(date: Date) {
if (this.isToday(date)) return "";
return formatInTz(this.$i18n?.locale, this.timezone, { weekday: "short" }, date);
},
weekdayShort(date: Date) {
return formatInTz(this.$i18n?.locale, this.timezone, { weekday: "short" }, date);
},
weekdayLong(date: Date) {
return formatInTz(this.$i18n?.locale, this.timezone, { weekday: "long" }, date);
},
fmtAbsoluteDate(date: Date) {
const weekday = this.weekdayPrefix(date);
const hour = formatInTz(this.$i18n?.locale, this.timezone, {
hour: "numeric",
minute: "numeric",
hour12: is12hFormat(),
}, date);
return `${weekday} ${hour}`.trim();
},
fmtHourMinute(date: Date) {
return formatInTz(this.$i18n?.locale, this.timezone, {
hour: "numeric",
minute: "numeric",
hour12: is12hFormat(),
}, date);
},
fmtFullDateTime(date: Date, short: boolean) {
return formatInTz(this.$i18n?.locale, this.timezone, {
weekday: short ? undefined : "short",
month: short ? "numeric" : "short",
day: "numeric",
hour: "numeric",
minute: "numeric",
hour12: is12hFormat(),
}, date);
},
fmtWeekdayTime(date: Date) {
return formatInTz(this.$i18n?.locale, this.timezone, {
weekday: "short",
hour: "numeric",
minute: "numeric",
hour12: is12hFormat(),
}, date);
},
```
This keeps all behavior but makes each formatter read as “what do I want to show” rather than “how do I wire up `Intl` and `timeZone` again”.
### 3. Move timezone resolution out of the mixin
The `timezone` computed now mixes UI formatting with global settings and store state. You can keep behavior identical but move the logic into a reusable utility, which the mixin calls:
```ts
// e.g. in @/units or @/settings
export function resolveTimezone(userTz: string | undefined, storeTz: string | undefined): string {
const pref = userTz || "";
const browserTz = Intl?.DateTimeFormat?.().resolvedOptions?.().timeZone || "UTC";
if (!pref) return browserTz;
if (pref === "server") return storeTz || browserTz;
return pref;
}
```
Then in this mixin:
```ts
import { resolveTimezone } from "@/settings"; // or "@/units"
computed: {
timezone(): string {
return resolveTimezone(settings.timezone, store.state.timezone);
},
},
```
That keeps this file focused on formatting, isolates the policy of “which timezone wins”, and makes it trivial to reuse/test elsewhere.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -185,45 +196,74 @@ export default defineComponent({ | |||
| return new Intl.DurationFormat(this.$i18n?.locale, { style: "long" }).format(parts); | |||
| }, | |||
| fmtDayString(date: Date) { | |||
There was a problem hiding this comment.
issue (complexity): Consider extracting shared Intl-based timezone formatting into small helpers and a reusable timezone resolver to reduce duplication and keep the mixin focused on display logic.
You can keep the timezone-aware behavior but trim a lot of duplication by extracting two small helpers and moving timezone resolution out of the mixin.
1. Factor out repeated Intl.DateTimeFormat + formatToParts logic
fmtDayString, fmtTimeString, isToday, and the German branch of hourShort all repeat the same pattern. A tiny helper will centralize that:
// in the same file, above `export default` or in a small util
function formatPartsInTz(
date: Date,
timeZone: string,
options: Intl.DateTimeFormatOptions
) {
const parts = new Intl.DateTimeFormat("en-CA", { timeZone, ...options }).formatToParts(date);
return (type: string, fallback = "00") =>
parts.find((p) => p.type === type)?.value ?? fallback;
}Usage examples:
fmtDayString(date: Date) {
const get = formatPartsInTz(date, this.timezone, {
year: "numeric",
month: "2-digit",
day: "2-digit",
});
return `${get("year")}-${get("month")}-${get("day")}`;
},
fmtTimeString(date: Date) {
const get = formatPartsInTz(date, this.timezone, {
hour: "2-digit",
minute: "2-digit",
hour12: false,
});
const HH = get("hour").replace("24", "00");
const mm = get("minute");
return `${HH}:${mm}`;
},
hourShort(date: Date) {
const locale = this.$i18n?.locale;
const tz = this.timezone;
if (locale === "de") {
const get = formatPartsInTz(date, tz, {
hour: "numeric",
hour12: false,
});
return Number(get("hour", "0"));
}
return new Intl.DateTimeFormat(locale, {
hour: "numeric",
hour12: is12hFormat(),
timeZone: tz,
}).format(date);
},isToday can also reuse Intl.DateTimeFormat construction via a helper (see next section).
2. Factor out Intl.DateTimeFormat creation for the current timezone
You recreate new Intl.DateTimeFormat(this.$i18n?.locale, { …, timeZone: this.timezone }) in many methods. A small helper reduces noise and makes intent clearer:
function formatInTz(
locale: string | undefined,
timeZone: string,
options: Intl.DateTimeFormatOptions,
date: Date
) {
return new Intl.DateTimeFormat(locale, { ...options, timeZone }).format(date);
}Then:
isToday(date: Date) {
const fmt = (d: Date) =>
formatInTz("en-CA", this.timezone, {
year: "numeric",
month: "2-digit",
day: "2-digit",
}, d);
return fmt(date) === fmt(new Date());
},
weekdayPrefix(date: Date) {
if (this.isToday(date)) return "";
return formatInTz(this.$i18n?.locale, this.timezone, { weekday: "short" }, date);
},
weekdayShort(date: Date) {
return formatInTz(this.$i18n?.locale, this.timezone, { weekday: "short" }, date);
},
weekdayLong(date: Date) {
return formatInTz(this.$i18n?.locale, this.timezone, { weekday: "long" }, date);
},
fmtAbsoluteDate(date: Date) {
const weekday = this.weekdayPrefix(date);
const hour = formatInTz(this.$i18n?.locale, this.timezone, {
hour: "numeric",
minute: "numeric",
hour12: is12hFormat(),
}, date);
return `${weekday} ${hour}`.trim();
},
fmtHourMinute(date: Date) {
return formatInTz(this.$i18n?.locale, this.timezone, {
hour: "numeric",
minute: "numeric",
hour12: is12hFormat(),
}, date);
},
fmtFullDateTime(date: Date, short: boolean) {
return formatInTz(this.$i18n?.locale, this.timezone, {
weekday: short ? undefined : "short",
month: short ? "numeric" : "short",
day: "numeric",
hour: "numeric",
minute: "numeric",
hour12: is12hFormat(),
}, date);
},
fmtWeekdayTime(date: Date) {
return formatInTz(this.$i18n?.locale, this.timezone, {
weekday: "short",
hour: "numeric",
minute: "numeric",
hour12: is12hFormat(),
}, date);
},This keeps all behavior but makes each formatter read as “what do I want to show” rather than “how do I wire up Intl and timeZone again”.
3. Move timezone resolution out of the mixin
The timezone computed now mixes UI formatting with global settings and store state. You can keep behavior identical but move the logic into a reusable utility, which the mixin calls:
// e.g. in @/units or @/settings
export function resolveTimezone(userTz: string | undefined, storeTz: string | undefined): string {
const pref = userTz || "";
const browserTz = Intl?.DateTimeFormat?.().resolvedOptions?.().timeZone || "UTC";
if (!pref) return browserTz;
if (pref === "server") return storeTz || browserTz;
return pref;
}Then in this mixin:
import { resolveTimezone } from "@/settings"; // or "@/units"
computed: {
timezone(): string {
return resolveTimezone(settings.timezone, store.state.timezone);
},
},That keeps this file focused on formatting, isolates the policy of “which timezone wins”, and makes it trivial to reuse/test elsewhere.
|
I don't think we want to implement this, at least not atm. Backend has various places where timezones matter, this needs extensive test cases. |
73a0745 to
504731d
Compare
Idea is not to touch the backend but just the time displayed on user interface based on the timezone |
Add a timezone selector (browser / server / custom IANA) to User Interface settings. All time displays — dashboard plan status, charging plan modal, forecast chart — now render in the chosen timezone without requiring a page refresh. - formatter: promote `timezone` to computed via resolveTimezone helper; extract formatPartsInTz/formatInTz to eliminate Intl boilerplate; all formatters pass timeZone explicitly - ChargingPlan: watch `timezone` to refresh imperative targetTimeLabel - PlanStaticSettings: parse user-entered time in selected timezone via parseLocalTimeInTz; timezone-aware dayOptions labels - settings/units: persist timezone preference to localStorage; add resolveTimezone (centralises browser/server/custom policy) and document DST behaviour of parseLocalTimeInTz - cmd/root.go: broadcast real IANA timezone name; fall back to "UTC" instead of an invalid offset string when no IANA name can be determined Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
504731d to
ba611c8
Compare
|
This is a massive change. Also many places are touched, that I dont think need adjustment. Other places, like repeating plans stay untouched. These are places where we had issues in the past, we explicitly handle/store timezone info there which we should migrate to another model. Dont think these changed help. The proposed solution in the issue was to make a service-side timezone user-adjustable (default is system tz) and use this consistently throughout the entire application (forecast, devices, plans, history, ...). Many projects (home assistant, ...) use this pattern. Introducing another local storage configured timezone makes this even more complicated. Now we have system/browser/localstorage-tz. As @andig said, this is a delicate topic that needs extensive testing in all mentioned areas. |
Add a timezone selector (browser / server / custom IANA) to User Interface settings. All time displays — dashboard plan status, charging plan modal, forecast chart — now render in the chosen timezone without requiring a page refresh.
timezoneto computed (reads reactive settings directly), add timeZone option to all Intl.DateTimeFormat calls, rewrite fmtDayString/fmtTimeString/isToday to use Intltimezoneto refresh imperative targetTimeLabel