Skip to content

refactor: time buckets (2) - #3554

Merged
jrasm91 merged 1 commit into
mainfrom
refactor/time-buckets-2
Aug 5, 2023
Merged

jrasm91 merged 1 commit into
mainfrom
refactor/time-buckets-2

Conversation

@jrasm91

@jrasm91 jrasm91 commented Aug 4, 2023

Copy link
Copy Markdown
Member

Follow-up PR to address auto navigating unarchived assets, etc.

In this PR:

  • Add option to AssetGrid to auto advance the asset view for a specific action (archive on the timeline, unfavorite on favorites page, unarchive on archive page)
  • Navigate to previous asset if it is the last asset
  • Close the viewer if you trigger removing the last item via archive/unarchive/unfavorite.

Tested scenarios:

  • Archive on main timeline via asset viewer
  • Unfavorite on favorites page via asset viewer
  • Unarchive on archive page via asset viewer
  • All the same actions for the first image, last image of page

@vercel

vercel Bot commented Aug 4, 2023

Copy link
Copy Markdown

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

1 Ignored Deployment
Name Status Preview Updated (UTC)
immich ⬜️ Ignored (Inspect) Aug 4, 2023 9:45pm

assetViewingStore.setAssetId(previousAsset);
}

return !!previousAsset;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you help explain the logic here?

@jrasm91 jrasm91 Aug 5, 2023 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Just returns a boolean value to indicate if one was found or not. Makes it possible to fallback to navigating to the previous asset and again falling back to closing the viewer if there is not another asset to show.

Basically, when you archive an asset and it should advance, what if there is no next asset? Now we can know that and show the previous one instead. Double !! just converts it to a boolean.

@alextran1502

Copy link
Copy Markdown
Member

Looks good to me, besides that one question, feel free to merge

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.

2 participants