immich icon indicating copy to clipboard operation
immich copied to clipboard

feat(web): Scroll to asset

Open midzelis opened this issue 1 year ago • 5 comments

Fixes #3114

  • #hash in url is updated with the asset id as the asset-grid is scrolled.
  • Loading a url with a hash will auto-scroll to the asset.
  • If the asset-viewer was the URL that was opened, clicking close will add the currently viewed asset as the #hash.
  • Works in all asset-grids (album/archive/trash/etc), except for "gallery-viewer" used for search results. (By design)

midzelis avatar Jun 27 '24 02:06 midzelis

Hi @midzelis, thanks a lot for your work on this. The initial review round is very promising. Besides some questions, I noticed that grouping of assets by date of the same month no longer stays on the same flex-row; they are being split into individual rows.

Is that a requirement to make this mechanism work? Can we keep the previous grouping view?

alextran1502 avatar Jun 27 '24 04:06 alextran1502

Is that a requirement to make this mechanism work? Can we keep the previous grouping view?

No, not a requirement, yes we can keep. This was an oversight - reverted the css causing this issue.

midzelis avatar Jun 27 '24 23:06 midzelis

Can you help with a short write-up of how this mechanism is designed to work in the description of the PR? So that we can store it as documentation of your design?

alextran1502 avatar Jul 04 '24 02:07 alextran1502

Some initial observations:

  • Scrubbing via scrollbar seems noticeably slower
  • I dislike the at query parameter updating while scrolling, that will also clutter the browser history
  • Prefer using unknown or void instead of the any type
  • Instead of adding more functionality to the already large AssetGrid component, consider breaking it down into smaller, more manageable components or functions. Large components are hard to test and maintain.
  • Components are becoming overly interconnected by passing around references to DOM elements and then accessing properties like firstElementChild and offsetParent. It's better to keep those inside components.
  • It would be nice to keep the pending scroll logic outside of the assets store
  • There is a good amount of logic involved, adding some tests would help reduce the risk of things breaking in the future

Prefix this reply with I'm already working on v2 of this change. This PR can stand on its own and its not required for v2. In short, v2 optimizes some of the bucket rendering, which makes scrubbing very large timelines (tested with 111K assets) almost as fast as scrolling with the mouse wheel. It also has some bug fixes, and I'll work some tests and other improvements. That being said, I'll address your points one by one.

  1. The scrub speed will be fixed by the thing i'm workin on now
  2. the ?at= query param is something I'd like to hear more opinions on - maybe a vote on discord? It is true that it pollutes the browser history - there is nothing we can do about that. It does not add these entries into the browser tab's local history (not part of back/forward) but it does show up in the global 'recently visited' history.
    • The code that updates the nav is basically 1 line of code.
    • Should it stay or should it go?
    • Should it be a pref?
    • Should it be some menu action?
  3. Done (in latest commits)
  4. I agree - asset-grid could be refactored some more. However, I think that should be done in a different PR as a tech-debt item.
  5. I also dislike passing dom elements, but thats mostly due to <IntersectionObserver> - this could be improved, but the changes in this PR isn't really any worse than current behavior. The offsetParent stuff is contained to the component that owns those elements, so thats ok in my book.
  6. pendingScroll can be moved out of assets-store - but assets-store is intimately tied to asset-grid - its effectively the viewmodel for it. So, it can be either in assets-store, or its in the component directly. I'll go ahead and move it for now.
  7. Can't disagree about tests, can't disagree about splitting up components. However, those concerns seem like commentary that could have been made on the code even before this PR. I consider it general tech debt - and like I mentioned earlier, it could be done as a cleanup tech-debt PR in the future.

midzelis avatar Jul 05 '24 23:07 midzelis

Summary of changes

Got rid of svelte generates iframes that are created when using bind:clientWidth/bind:clientHeight - using resizeobserver instead

  • Fixed navigation on open/close asset-viewer - still not delegating to history.back - one for the future
  • Allowed intersection-observer to use non pixel margins, fix unseen bug with multiple entries (was looking at entries[0] even tho it may not have been the intersecting entry)
  • Dropped fallback from intersection observer - works on older modern browsers, except ie11 - https://caniuse.com/intersectionobserver
  • Reduce fade duration from 300 to 150, since the fade gives the perception of it being slower than it is.
  • Extend bucket-sections margins from +/- 750px to be +/- 200% of the #asset-grid rect - this should give more buffer to load next bucket when you get close to the edge, hopefully pre-loading and pre-rendering everything so you don’t see any fade in at all. - it can still happen if you scroll VERY fast, or if you scrub the timeline
  • Make thumbnails use the #asset-grid rect - when specifying nothing, the window viewport it used. And for overflow elements, they do not respect margin extensions on the window viewport (learned this by trial/error) - so pass in the #asset-grid rect instead - it is set to the same 200% margin extensions, so images will start to load/prerender when 2x off the scroll screen.
  • Asset-store has proper queuing behavior for laodBucket and init()
  • Asset-grid’s skeleton waits until scroll is done - no scroll flicker - if it’s slower than 300ms, you’ll see the skeleton.

  • Because of this impl - HMR causes the skeleton to show up, since a HMR runs onMount without running the navigation event. Added HMR specific code to perform the equivalent of a navigation event so that the scroll position is updated on a HMR and the skeleton disappears as appropriate.
  • Make all the scroll update/sync methods run in RAFs

How does this all work?

Beware - the timeline is a VERY complicated piece of code. Before we start, how does it behave?

When the asset-grid is first rendered, it doesn’t render every asset. This would be a bad idea if you have 111K assets in your timeline. Instead, it uses intersection observers to only load the ‘buckets’ that intersect with the viewport. Buckets are groupings of assets, by default, per month. Each bucket has the full asset record object, including all exif and other metadata. Buckets are further divided into Date-Groups. By default, the date groups are grouped by day. However, if a bucket is visible, it will render all the days within the bucket, even if the bucket has several thousand pictures (foreshadowing - being fixed in v2 of this feature),

Now, there is also a bucket metadata call, called getTimelineBuckets - this gives you ALL the buckets in the system, with a count of many assets are in that bucket, but without any asset details, or any other metadata. This is performed during asset-store initialization.

When the asset-grid is initialized, it initializes the asset-store, loads all the timeline bucket metadata, and then starts the load process of all the buckets that intersect the viewport to load them.

The asset-grid does not have ‘infinite-scroll’, since it does end, it is bounded to the number of assets in your library. But it is still virtual, in the sense that it tries to only load/show what is currently in the viewport of the screen, and unloads everything else.

It’s simple to figure out how big the timeline is - just load all the images, add up the heights, and you’re done. Oh - but that means you have to load the exif height/width information of the each image - we don’t want to do that for efficiency reasons. So, instead, we use the number of photos in each bucket, and ESTIMATE the size of the bucket. Later, when the bucket is actually fully rendered, we adjust the height of the bucket to reflect actual heights. Why do we need to adjust the height of the bucket? Because the scrubber’s height needs to reflect the actual height of the timeline, which was changed by the bucket height. There is one more use-case why this is important - if you jump to the bottom of a virtual timeline, and then you start to scroll upwards, as the buckets above the scroll position begin to render, they change the size of the bucket. This changes the scroll offset, which would cause a jump in your otherwise smooth scrolling as the buckets change these size. To counter act this, as buckets are loaded, the intersection observer notices if they are ABOVE or BELOW the current viewport - if they are above the viewport, the scroll position is updated with the delta-change of the bucket-size, to maintain the scroll position where it is.

There is a wrinkle to this. Within a given sub-bucket date-group, if there are just a few pictures (1,2 or maybe 3), and the next date-group also has a few, these are layed out using flex-cols. The images within a date-group are all layed out using a justified-layout algorithm, which yields absolutely positioned coordinates. However, you have to be careful to not be tempted to try to calculate the actual height of the bucket using these measurements - because of the flex-col layouts, 1 or more date-groups could be on the same row. Instead of re-implementing the browsers flow layout algorithm, it’s easier, faster, and more robust to just rely on measuring the positions from the clientHeight/width of the bucket itself. This has bit me more times than I will care to admit.

Ok, with that background out of the way, how do we ‘scroll to an asset’?

Well, ok, simple:

  1. Get the asset id
  2. Which bucket is this asset part of? Good question. We can use a trick - load the asset completely using the getAssetInfo call - this gives us the asset’s date/time. Round down this asset date time to the nearest month. That is the bucket we which to load.
  3. Load the bucket.
    1. Problem one - asset-grid notices that this bucket is offscreen and will cancel any loads of buckets that are offscreen
    2. Fix: set a flag (isPreventCancel) to force to load the bucket
  4. Now we need to find the HTML element for the asset.
  5. Set a property (pendingScrollAsset/pendingScrollBucket) to the asset/bucket we want to scroll to
  6. Have the thumbnail component check to see if the thumbnail asset matches pendingScrollToAsset
  7. If it does, calculate the relative offset of the image to the asset-grid component to get a scroll amount
  8. Perform the scroll.

Ok, hold up. We had a problem at step 6. Even tho we loaded the bucket, the intersection observer is still preventing that bucket from being seen. We adjust the #if block in the asset-grid to show the bucket if it is intersecting OR if it is pendingScrollBucket.

Well, that’s pretty much it. There are some tricks that we need to play with scrolling to prevent circular/infinite loops. That’s because sometimes the scroll position is updating because we want to scroll TO an asset. But other times, the scroll position is being updated because an asset just entered the viewport and the URL query/param needs to be updated. (That’s what internalScroll property is for)

To reduce flicker that would otherwise happen while loading the first bucket, then finding/loading the target bucket, and finally scrolling, a skeleton is used. I also found the skeleton distracting for very fast navigations (<.3s), so I delay showing the skeleton while that is happening. To keep the grid heights and intersection observers working properly, they are marked with css property visibility: none - so even without the skeleton there (for the first .3s) you just see the background. For longer loads, you’ll see the skeleton and then later it will be replaced with the grid, pre-scrolled to the exact location it needs to be.

Now, for that to work, we need to delay setting the skeleton to ‘off’ until after the navigation completes. It needs to be a ‘navigation’ event, because that is how you update the location in svelte. That means the component assumes that there is an active navigation happening when it is created, and installs a afterNavigate() listener to start the scrolling process (if there is a query parameter) - or to set the skeleton to false if there is no query parameter. That means that the skeleton needs to be set to false after the ‘scroll to asset’ routine is finished, which is precisely where that happens.

Of course, there are some usages of the asset-grid that do not participate in routes, and are not tied to navigation at all. For example, when adding assets to an existing album. For that, a new property was added ‘participatesInRouting` which will disable all navigation - will not look at the url to scroll to asset, nor will it update the url on scroll.

Ok - other strangeness that you may experience/things just to be aware of.

As I said in the beginning/background - the viewer tries to load only things that are on-screen, and unloads things that are offscreen. What this means is that as you scroll (even a scroll triggered by the URL query parameter) - this may in turn load buckets, and those buckets may load/change bucket sizes while you are scrolling. If you did everything right - the scroll positions are appropriately compensated and you scroll to exactly where you wanted to go to.

The height of buckets is also dependent on the width of the screen. That is because date-groups that have small amount of photos can be combined into a single row!! Don’t forget that. Ever!

Actually, the bucket system is really great. The date-groups within the buckets work really well. One thing that doesn’t happen is combining date-groups that span buckets, but would otherwise fit on a single row. One might be tempted to allow that. I’d caution to avoid that. The code that ‘maintains/compensates’ the scroll position relies on the height of buckets as anchor points. If you combined date-groups across buckets, there would be no quick/easy way to determine exactly how much the scroll-delta should be. Especially since that row could be partially loaded and change as one or the other bucket come into the viewport. It is a very good idea to have well known anchor points, and those will be based on the monthly time bucket.

I wrote this in one go, didn’t reread, maybe I’ll edit for clarity in the future. I could also turn this into docs in the architecture section of the docs site.

midzelis avatar Jul 06 '24 00:07 midzelis

Some notes on the scroll bar offset issue

So, the way the scrollbar works now, to keep the ticks in fixed spots, is that it scales each bucket into a fraction of the scrollbar location on screen. This usually works perfectly when using mouse scrolling, since the scroll indicator can be placed with sub-pixel accuracy on the scrollbar. Howevever, when CLICKING on the scrollbar, we are limited to the mouse-click X/Y coordinates, which are FULL pixels. That means, if you have a large bucket that's been compressed into say 8 pixels, the you only get to position the viewer with 1/8 % positions. So, there are a bunch of positions that are not 'navigatible' via scrub/click on the scroll bar. But scrolling the viewer by mouse will accurately update the indicator on the scrollbar. The months being off that you are witnessing may be due to you click on/near a boundary condition. That might be able to fixed a little, so that at least the months don't shift - might be OBO error somewhere there. But fundamentally, the above is a limitation. My mitigation would be to 'snap' the scrub or mouse indicators while dragging or clicking the scrollbar, so that it won't shift during the 'round-trip' - but it would still be limited in accuracy

alextran1502 avatar Aug 20 '24 13:08 alextran1502