fix(web): Show resulting date range when changing by offset#28103
fix(web): Show resulting date range when changing by offset#28103svsantos42 wants to merge 2 commits intoimmich-app:mainfrom
Conversation
…ng dates by an offset
|
Label error. Requires exactly 1 of: changelog:.*. Found: 🖥️web. A maintainer will add the required label. |
| let assetDates = $derived( | ||
| assets | ||
| .map((asset) => DateTime.fromObject(asset.localDateTime, { zone: asset.localOffsetHours ? 'local' : 'UTC' })) | ||
| .sort((a, b) => a.toMillis() - b.toMillis()), | ||
| ); | ||
|
|
||
| let firstDate = $derived.by(() => { | ||
| let base = assetDates.at(0); | ||
| let date = showRelative && base ? calcNewDate(base, selectedDuration, selectedOption?.value) : base; | ||
| return getFullDateFormat(date); | ||
| }); | ||
|
|
||
| let lastDate = $derived.by(() => { |
There was a problem hiding this comment.
I don't see why these three derivedes can't be const?
Also, sorting is technically less efficient than just searching the earliest and latest date in two iterations.
There was a problem hiding this comment.
I hadn't thought about that, I changed it to just loop through the array once instead of sorting the array. And I added const where it can be used.
| let updatedAssets = getOwnedAssetsObjects(assets, authManager.user); | ||
| for (let asset of updatedAssets) { | ||
| asset.localOffsetHours = selectedOption.offsetMinutes / 60; | ||
| } | ||
|
|
There was a problem hiding this comment.
This change is entirely unrelated, is it not?
There was a problem hiding this comment.
This is needed to update the timezone offset of the assets currently on the page. If you refresh the page after changing the dates, then it gets the latest information. But if you try changing the date again without refreshing the page, it still has the old timezone offset if it changed, so it would show the wrong date range.
| <Text size="small">{$t('first')}: {firstDate}</Text> | ||
| <Text size="small">{$t('last')}: {lastDate}</Text> |
There was a problem hiding this comment.
You should use a translation key that takes an argument. Not in every language is it actually "First: "
There was a problem hiding this comment.
What do you recommend here? I don't speak any other languages, so I'm not sure what would be better than first/last.
| export const getOwnedAssetsObjects = (assets: TimelineAsset[], user: UserResponseDto | null): TimelineAsset[] => { | ||
| return [...assets].filter((a) => user && a.ownerId === user.id); | ||
| }; | ||
|
|
There was a problem hiding this comment.
Like just use the filter in onSubmit instead of creating a function here? I figured it might be a useful function for future use in other places.
Description
When selecting multiple assets on the main timeline and clicking the menu to "Change Date", it brings up the date change dialog. If you select to "Change date by offset", it used to show the resulting date range that would be applied to the selected photos with the desired change settings. This stopped working after v2.2, so this change brings that functionality back, which is very helpful when trying to change the dates of many photos at once.
Fixes #23659
How Has This Been Tested?
This was tested by uploading photos to Immich and then selecting multiple and doing the change date by offset operation. It now shows the resulting date range and it matches the dates that get applied after clicking confirm. This was tested on photos with different types of dates. Ones that already had a timezone in the metadata and ones that don't (like screenshots) and both ways produce the expected result. It was also tested by doing multiple date change operations in a row without refreshing the page to make sure the changes take effect instantly on the page for future changes.
Screenshots (if appropriate)
Checklist:
src/services/uses repositories implementations for database calls, filesystem operations, etc.src/repositories/is pretty basic/simple and does not have any immich specific logic (that belongs insrc/services/)Please describe to which degree, if any, an LLM was used in creating this pull request.
No LLM usage, used the Immich docs to get an environment set up and to run the test commands once finished.