immich
immich copied to clipboard
fix(web/mobile): sort order wrong in asset viewer
This fixes #2061 I don't really understand why it has to be sorted this way, but I went with the way it is implemented in the asset grid: https://github.com/immich-app/immich/blob/4e526dfaae577336ab5a5e4aea451e722eeabdd7/web/src/lib/components/photos-page/asset-date-group.svelte#L33-L37
The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Comments | Updated (UTC) |
|---|---|---|---|---|
| immich | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | Jun 19, 2023 4:47pm |
Hello, I have a set of photos taken on the same date, the navigation on the web seems to work correctly. Is there a set of photos that you can share to reproduce the issue that this PR trying to fix?
Hello, I was able to use some of these images to reproduce the issue (iirc they not only have to be taken on the same day but also in the same minute probably): immich.zip
test-images.zip I did some testing with these test images that show the same number as the filename. Everytime I reimport the images the order changes in the timeline on the web. The android app always shows them in the order 5, 2, 4, 3, 1
I can see that the order is incorrect comparing the mobile app vs the web, however, the PR doesn't seem to fix this?
I actually haven't looked at the differences between web and mobile before you mentioned it.
I added additional sorting by filename (originalPath should imply filename) because that seems like the only useful criteria that should work in every scenario when the date is completely identical. Without that the sorting seems to be very arbitrary, probably just based on the way the sorting algorithms are implemented, which is hard to track down.
I've put some more thoughts into this and decided that using the the id (remoteId and additionally localId on mobile) would probably be the more intuitive way to handle this. The result is essentially the same but it seems easier and less error-prone to me. Honestly, I don't know why I didn't use this already.
Thank you for working on this.
I do not see why sorting by
remoteIdmakes sense - it's a random UUID. Sorting by it is probably just like taking the algorithms way of handling ties.filenamemakes a lot more sense to me in this regard.
For me the names of most if not all of my files are pretty random anyways and since there is no way to rename them after uploading them it feels just as random to me as sorting by ID but I can definitely understand why you think so. I'll simply revert the latest commit as soon as I have access to my PC again. Thank you for the feedback!
Sorting by a unique value has the benefit of ensuring the same order across different platforms and when an unstable sorting algorithm is used. I agree that sorting by filename makes more sense than ID, but we could include the ID as a third field.
@michelheusschen does adding the id as a third field give any additional value? As far as I can see originalPath should also be unique and thus there should be no case where the id would be used for sorting or am I missing something?
You're right, originalPath is unique. Then reverting the last commit should be good👍
I believe this was fixed in #3092.
This PR attempts to fix the order of assets in the main timeline for assets with the same timestamp. So it was not fixed by #3092
This PR attempts to fix the order of assets in the main timeline for assets with the same timestamp. So it was not fixed by #3092
Hmm, okay. There have been some changes in this area so I wonder if this is still present? I also wonder if the server should just do a better job or sorting instead to avoid duplicating this logic across clients?