immich-app/immich
1
0
Fork 0
mirror of https://github.com/immich-app/immich.git synced 2026-03-01 11:20:12 +03:00
Code Issues 783 Packages Projects Releases 12 Wiki Activity

[PR #23615] [MERGED] fix(web): fix timezone dropdown for timestamps lacking milliseconds #17575

New Issue
Closed
opened 2026-02-05 16:24:12 +03:00 by OVERLORD · 0 comments
OVERLORD commented 2026-02-05 16:24:12 +03:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/immich-app/immich/pull/23615
Author: @skatsubo
Created: 11/5/2025
Status: ✅ Merged
Merged: 11/5/2025
Merged by: @alextran1502

Base: main ← Head: fix/edit-date-timezone-modal

📝 Commits (1)

  • 740b803 Fix timezone selector for timestamps without milliseconds

📊 Changes

1 file changed (+8 additions, -3 deletions)

View changed files

📝 web/src/lib/modals/timezone-utils.ts (+8 -3)

📄 Description

Description

Fix bug in timezone dropdown in "Edit date and time" modal that affects assets without milliseconds in their timestamp.
Internally, browsers may strip .000 in datetime input, therefore breaking string-based comparison of datetimes in timezone-utils that assumes format with milliseconds .SSS.

This PR switches to second precision in the affected comparison - instead of milliseconds as they are not used in TZ calculations anyway.
Apart from dropping milliseconds, existing logic is preserved. So this is a point solution in the place where the bug occurs.

Fixes:

I considered other options, but stick to the point solution outlined above. Alternatives:

  • Keep existing logic in timezone-utils, append missing ".000" at datetime picker level
  • Keep existing logic in timezone-utils, append missing ".000" at modal level
  • Switch to DateTime comparison in timezone-utils instead of string comparison

How Has This Been Tested?

Tested manually

  • Without milliseconds -> works
  • With milliseconds -> works, milliseconds are preserved
  • With DST-gap times such as 2025-03-30 02:30:00 -> affected timezones disappear from the dropdown (there is no Europe/Berlin and so on)

Screenshot

Changing date/time for an asset having 0 milliseconds: successful.
Dev console shows additional debug output in zoneOptionForDate() (not included in PR).

image

Out of scope

Fix UX regression: give feedback for "obviously" wrong values that have date.isValid false, such as 30th of Feb.

  • Current behavior upon entering invalid date: TZ switches to UTC, I still can click "Confirm", then modal closes silently.
  • Previously there was feedback via disabled={!date.isValid}.
Some other low priority observations/thoughts regarding the datetime + timezone validation.
  1. Tweak implementation. Does it make sense to check isValid for every timezone? Only if isValid depends on timezone. Otherwise, move isValid check out of the loop over 400+ timezones.
  2. Tweak implementation. Handle "not isValid" earlier: skip other validation checks (e.g. DST gaps) when isValid === false. (Low priority / mostly aesthetic because false isValid is rare, so for 99.9% invocations this does not matter).
  3. Consider a UX change (probably too radical): defer the expensive datetime+timezone validation to a later phase instead of doing it on every change. Still do "date.isValid" check on every change. This follows the Pareto principle: lightweight realtime "isValid" check will catch most probable mistakes (Nov 31, Feb 29 on non-leap year). While more expensive datetime+tz check will later catch rare cases when datetime falls into a DST gap.
    To be clear: I like the current UX and how it filters out impossible timezones. Current implementation works fine. This suggestion only applies if performance matters, e.g. when iterating over 400+ timezones on some outdated potato hardware. (Negligible difference in Firefox on M1: 30ms vs 20ms with or without the "expensive" check)

Checklist:

  • I have performed a self-review of my own code
  • I have no unrelated changes in the PR.
  • I have followed naming conventions/patterns in the surrounding code

Please describe to which degree, if any, an LLM was used in creating this pull request.

To explore existing code and suggest fixes at different levels: datetime input, modal, utils.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/immich-app/immich/pull/23615 **Author:** [@skatsubo](https://github.com/skatsubo) **Created:** 11/5/2025 **Status:** ✅ Merged **Merged:** 11/5/2025 **Merged by:** [@alextran1502](https://github.com/alextran1502) **Base:** `main` ← **Head:** `fix/edit-date-timezone-modal` --- ### 📝 Commits (1) - [`740b803`](https://github.com/immich-app/immich/commit/740b803918028d1ecff904f39dce311758d1d915) Fix timezone selector for timestamps without milliseconds ### 📊 Changes **1 file changed** (+8 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `web/src/lib/modals/timezone-utils.ts` (+8 -3) </details> ### 📄 Description ## Description Fix bug in timezone dropdown in "Edit date and time" modal that affects assets _without_ milliseconds in their timestamp. Internally, browsers may strip `.000` in datetime input, therefore breaking string-based comparison of datetimes in timezone-utils that assumes format with milliseconds `.SSS`. This PR switches to second precision in the affected comparison - instead of milliseconds as they are not used in TZ calculations anyway. Apart from dropping milliseconds, existing logic is preserved. So this is a point solution in the place where the bug occurs. Fixes: - https://github.com/immich-app/immich/issues/23491#issuecomment-3480069256 (the originally reported Android issue was overshadowed by the frontend bug discussion) I considered other options, but stick to the point solution outlined above. Alternatives: - Keep existing logic in timezone-utils, append missing ".000" _at datetime picker level_ - Keep existing logic in timezone-utils, append missing ".000" _at modal level_ - Switch to DateTime comparison in timezone-utils instead of string comparison ## How Has This Been Tested? Tested manually - Without milliseconds -> works - With milliseconds -> works, milliseconds are preserved - With DST-gap times such as 2025-03-30 02:30:00 -> affected timezones disappear from the dropdown (there is no Europe/Berlin and so on) <details><summary><h2>Screenshot</h2></summary> Changing date/time for an asset having 0 milliseconds: successful. Dev console shows additional debug output in `zoneOptionForDate()` (not included in PR). <img width="1021" height="670" alt="image" src="https://github.com/user-attachments/assets/ad986bfe-d8dd-4d69-989b-9a07142d488a" /> </details> ## Out of scope Fix UX regression: give feedback for "obviously" wrong values that have `date.isValid` false, such as 30th of Feb. - Current behavior upon entering invalid date: TZ switches to UTC, I still can click "Confirm", then modal closes silently. - Previously there was feedback via `disabled={!date.isValid}`. <details><summary>Some other low priority observations/thoughts regarding the datetime + timezone validation.</summary> 1. Tweak implementation. Does it make sense to check `isValid` for every timezone? Only if `isValid` depends on timezone. Otherwise, move `isValid` check out of the loop over 400+ timezones. 2. Tweak implementation. Handle "not isValid" earlier: skip other validation checks (e.g. DST gaps) when isValid === false. (Low priority / mostly aesthetic because false isValid is rare, so for 99.9% invocations this does not matter). 3. Consider a UX change (probably too radical): defer the expensive datetime+timezone validation to a later phase instead of doing it on every change. Still do "date.isValid" check on every change. This follows the Pareto principle: lightweight realtime "isValid" check will catch most probable mistakes (Nov 31, Feb 29 on non-leap year). While more expensive datetime+tz check will later catch rare cases when datetime falls into a DST gap. To be clear: I like the current UX and how it filters out impossible timezones. Current implementation works fine. This suggestion only applies if performance matters, e.g. when iterating over 400+ timezones on some outdated potato hardware. (Negligible difference in Firefox on M1: 30ms vs 20ms with or without the "expensive" check) </details> ## Checklist: - [x] I have performed a self-review of my own code - [x] I have no unrelated changes in the PR. - [x] I have followed naming conventions/patterns in the surrounding code ## Please describe to which degree, if any, an LLM was used in creating this pull request. To explore existing code and suggest fixes at different levels: datetime input, modal, utils. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
OVERLORD added the pull-request label 2026-02-05 16:24:12 +03:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
accessibility
changelog:enhancement
changelog:security
changelog:skip
changelog:translation
cli
date-time
dependencies
documentation
external-library
format
good first issue
mobile-beta
mobile-beta
mobile-beta
needs-answer
nice to have
pull-request
Mirrored from GitHub Pull Request
 sharing
tech-debt
📱mobile
🖥️web
🗄️server
🧠machine-learning
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
OVERLORD
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: immich-app/immich#17575
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?