immich icon indicating copy to clipboard operation
immich copied to clipboard

basic support for permanent url in gallery view

Open szethh opened this issue 1 year ago • 14 comments

An attempt at persisting the picture ID in the url. Related: #2690 #2906

It's built around the assetViewingStore. When the presented asset changes, the app will redirect to /photos/{assetId}. It is a client-side redirect, so no state is lost. There is another listener that redirects back to /photos once the asset viewer is closed (clicking the 'x', pressing escape...)

When navigating to /photos/{assetId}, the path param is used to populate the asset viewer, if not set beforehand. This supports refreshing the page, and getting the same picture (and context!) back.

Navigation with the arrow keys / arrow buttons is unaffected.

The problem is that, to avoid loss of state when navigating to and from the pages, the main logic of /photos now resides on a layout. Essentially, renamed +page.svelte to +layout.svelte, and added a <slot /> at the end. Ditto with +page.server.ts. +page.svelte is now an empty file. The UI behavior is the same. I am not totally sure how we could go about changing the url AND keeping a +page.svelte intact.

For now this only works on /photos, since it's a proof of concept and I'm not sure if the layout thing is the best way to go. I'm completely new to this project so I might've missed something :D

szethh avatar Oct 31 '23 17:10 szethh

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
immich ⬜️ Ignored (Inspect) Visit Preview Nov 13, 2023 9:08pm

vercel[bot] avatar Oct 31 '23 17:10 vercel[bot]

@alextran1502 @jrasm91 hey, this is a super simple & rough proposal, but if yous think it's okay i can keep working on it

szethh avatar Nov 13 '23 20:11 szethh

Firstly, thank you for taking a look at this. It's is definitely a complicated problem.

I'm not a huge fan of using layouts myself. We have run into some weird issues with them in the past, especially when used with animations. Personally, I prefer components and slots instead.

I know that we use the asset viewer in several places (main timeline, archived assets, favorite assets, albums, shared albums, trash, etc.). It would be nice if the solution we came up with worked more or less on all those views without a lot of duplicated effort/code.

I had always assumed the urls would look like /photos/:id, etc, but that does indeed require a new route (additional photos/[id].svelte component) and that does seem to make everything more complicated.

Would you be willing to take a look at using query params instead? So something like /photos?id=:id? I think it might be possible to keep the /photos code in the same place and then auto-launch/change the asset-viewer if there is a query param (change). Or something like that. What do you think?

jrasm91 avatar Nov 13 '23 20:11 jrasm91

Maybe it would be possible to add code to the asset-grid (which is used in most pages) to:

  • sync current selected asset state via the query param
  • auto-launch the asset viewer on page refresh
  • auto-load / scroll-to the correct bucket (possibly by getting details for the asset id, calculating the asset bucket, and then scrolling to it)

jrasm91 avatar Nov 13 '23 20:11 jrasm91

yeah, using url params seems wayy easier, plus it works on places other than /photos.

i recreated the previous behavior without layouts - only some logic in asset-grid.svelte. this works for opening/closing/navigating between assets, as well as refreshing the page and getting the asset stored in the url.

do you think it looks good? ofc there's still the scroll logic to be implemented

szethh avatar Nov 13 '23 21:11 szethh

I think that looks a lot better! Minimal code changes and more or less the same functionality.

jrasm91 avatar Nov 13 '23 21:11 jrasm91

Deployment failed with the following error:

Resource is limited - try again in 1 hour (more than 100, code: "api-deployments-free-per-day").

vercel[bot] avatar Nov 13 '23 21:11 vercel[bot]

Deployment failed with the following error:

Resource is limited - try again in 57 minutes (more than 100, code: "api-deployments-free-per-day").

vercel[bot] avatar Nov 13 '23 22:11 vercel[bot]

Thank you for the nice work in getting this feature into the application.

I just spun up this PR to test and noticed that this mechanism doesn't work with the new feature of showing partners's assets on the timeline. If I select an asset that I own and refresh the page, I can then navigate. If I choose an asset that my partner shares with me and refresh the page, I cannot navigate.

Can you help take a look at this, thank you

alextran1502 avatar Nov 13 '23 22:11 alextran1502

yup i'll look into it. are there any examples of this feature on the public demo backend? currently connecting to that one for development.

also, my code fails on /albums/[albumId]. it seems there is a race condition on this line: https://github.com/immich-app/immich/blob/24670178dc85f8822f888b7cb88a611f4e6c0e58/web/src/routes/(user)/albums/%5BalbumId%5D/%2Bpage.svelte#L122

afterNavigate runs after onMount, so when attempting to open a picture inside an album my code will run first (setting the url params to the asset id), but immediately after the asset viewer will be closed in afterNavigate.

of course it can be worked around it with tick() or moving my own code to afterNavigate as well, but that seems very hacky... What is the purpose of that line? commenting it out allows the asset id to live in the url params as desired

szethh avatar Nov 13 '23 23:11 szethh

@szethh unfortunately there is no demo instance for that feature at the moment, the feature is merged in this branch though

alextran1502 avatar Nov 14 '23 02:11 alextran1502

yup i'll look into it. are there any examples of this feature on the public demo backend? currently connecting to that one for development.

also, my code fails on /albums/[albumId]. it seems there is a race condition on this line:

https://github.com/immich-app/immich/blob/24670178dc85f8822f888b7cb88a611f4e6c0e58/web/src/routes/(user)/albums/%5BalbumId%5D/%2Bpage.svelte#L122

afterNavigate runs after onMount, so when attempting to open a picture inside an album my code will run first (setting the url params to the asset id), but immediately after the asset viewer will be closed in afterNavigate.

of course it can be worked around it with tick() or moving my own code to afterNavigate as well, but that seems very hacky... What is the purpose of that line? commenting it out allows the asset id to live in the url params as desired

I think the use case is opening the asset viewer on a normal photo from the main timeline (or maybe an existing album?) and then using the overflow menu to create a new album. Basically if you make an album from a photo opened in the viewer the viewer needs to be closed.

jrasm91 avatar Nov 14 '23 03:11 jrasm91

@jrasm91 Looking into it more, I don't think tick() would do anything. To change the state in the url, we have to use goto, but that triggers afterNavigate. I don't see any option that would rewrite the url without triggering navigation (and therefore closing the viewer immediately).

Can I move this modal closing logic elsewhere? Maybe as a callback after the album gets created. I feel like closing the viewer after any navigation in /albums/[id] could conflict with more stuff in the future.

szethh avatar Nov 16 '23 15:11 szethh

url keeps state of the asset now in the album view as well. creating a new album from the navbar will now close the asset viewer.

I tried to make minimal changes, so I might've missed some edge case, I'm not super familiar with immich as a user myself :))

szethh avatar Nov 16 '23 16:11 szethh

Implemented in #8532

alextran1502 avatar Apr 24 '24 20:04 alextran1502