Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(web): use natural asset order for navigation #3092

Merged
merged 1 commit into from
Jul 3, 2023
Merged

fix(web): use natural asset order for navigation #3092

merged 1 commit into from
Jul 3, 2023

Conversation

uhthomas
Copy link
Member

@uhthomas uhthomas commented Jul 3, 2023

Fixes: #3091

@vercel
Copy link

vercel bot commented Jul 3, 2023

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

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
immich ⬜️ Ignored (Inspect) Jul 3, 2023 0:59am

Copy link
Contributor

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@uhthomas uhthomas force-pushed the 3091 branch 2 times, most recently from b8d9e59 to 814dda5 Compare July 3, 2023 00:38
@alextran1502 alextran1502 merged commit 1a0a3aa into main Jul 3, 2023
19 checks passed
@alextran1502 alextran1502 deleted the 3091 branch July 3, 2023 02:02
@brighteyed
Copy link
Contributor

I haven't tested it myself yet, but looking into code it seems the issue may still exist.

_assetGridState.assets is a flattened list of all the assets for already loaded buckets. In general, buckets are loaded in "random" order if the user clicks on the scrollbar. In this case, we will find the next/prev index, but the asset for this index may not be from the adjacent bucket.

@brighteyed
Copy link
Contributor

Yes, I can reproduce the bug on main (2099b04057a161ec44b557abac17763b4e696657).

Steps to reproduce:

  1. Open the main timeline (the first N bucket(s) are loaded now)
  2. Click on the scrollbar somewhere near the bottom (some bucket(s) near that location are loaded. Note: _assetGridState.assets now contains assets from nonadjacent buckets)
  3. Click on any asset to open AssetViewer
  4. Click the Previous button multiple times. When we reach the end of the bucket(s) loaded in (2), we begin to show assets from buckets loaded in (1) thus missing a lot of assets in intermediate buckets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

asset view navigation does not match the timeline
4 participants