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

feat(web): add keyboard shortcut to stack selected photos #5983

Merged
merged 8 commits into from
Apr 2, 2024

Conversation

Funk66
Copy link
Contributor

@Funk66 Funk66 commented Dec 26, 2023

This is my attempt at implementing #5143
I should say that I have zero experience with Svelte. I would be nice to reduce the duplication with

but I'm not familiar with the import hierarchy and event propagation conventions, so I would appreciate some guidance here.
As a side note, it may seem confusing to some that the help dialog shows action shortcuts that only work in the asset-viewer page, and now this new one that only works in the asset-grid page. Perhaps these should be shown under different categories.

@ThomasConrad
Copy link

When can this get merged?

@martabal
Copy link
Member

The logic to stack assets already exist in https://github.com/immich-app/immich/blob/main/web/src/lib/components/photos-page/actions/stack-action.svelte.

Can you use the same logic for the menu option and the shortcut action ?

@Funk66
Copy link
Contributor Author

Funk66 commented Jan 13, 2024

I tried deduplicating the code, but I don't know if my changes follow best practices.

@flightmansam
Copy link

This would be so handy! How close is a merge?

@isaacolsen94
Copy link

Any chance this can get merged? It would be crazy helpful!!

@jrasm91
Copy link
Contributor

jrasm91 commented Mar 29, 2024

@Funk66 can you resolve the merge conflicts and then we can look at merging it.

@isaacolsen94
Copy link

Thank you so much @Funk66, @jrasm91 is this good to go now? Sorry to pester! Just could really use this 😛

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

@jrasm91 jrasm91 enabled auto-merge (squash) April 2, 2024 15:00
@jrasm91 jrasm91 merged commit 28e8e53 into immich-app:main Apr 2, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants