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): search albums #7322

Merged
merged 6 commits into from Feb 22, 2024
Merged

feat(web): search albums #7322

merged 6 commits into from Feb 22, 2024

Conversation

martabal
Copy link
Member

Search through albums

Screenshots

2024-02-21.21-56-54.mp4

Copy link

cloudflare-pages bot commented Feb 21, 2024

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: fb7a1e1
Status: ✅  Deploy successful!
Preview URL: https://cf6d6118.immich.pages.dev
Branch Preview URL: https://feat-search-albums.immich.pages.dev

View logs

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

Copy link
Contributor

@mertalev mertalev left a comment

Choose a reason for hiding this comment

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

Awesome work!

web/src/routes/(user)/albums/+page.svelte Outdated Show resolved Hide resolved
@jrasm91 jrasm91 merged commit 75947ab into main Feb 22, 2024
25 checks passed
@jrasm91 jrasm91 deleted the feat/search-albums branch February 22, 2024 14:04
@andrewgdunn
Copy link

Apologies for commenting on a closed PR, but... would it be possible to do this on shared albums? (or maybe this applies there already)?

@jrasm91
Copy link
Contributor

jrasm91 commented Feb 22, 2024

This change only impacts the albums page, but in general it would be a good idea to consolidate the albums/shared albums pages to work the same.

@martabal
Copy link
Member Author

It's on my To do list
image

@waclaw66
Copy link
Contributor

I really appreciate this functionality, however there is a space for speed optimization. The page freezes for 3-4 second while searching among ~300 albums.

@alextran1502
Copy link
Contributor

@waclaw66 @martabal We probably need some debouncing

@waclaw66
Copy link
Contributor

waclaw66 commented Feb 28, 2024

The strange thing is that the first letter search is quite fast (<1s) after fresh page reload. When search input is cleared and first letter inserted again, it takes >3s. Even clearing the input is that slow. The search time depends on the letter, "e" is significantly slowed than "x", obviously because of frequency of occurrence.

2024-02-28.07.21.31.mp4

The above described occurs on Win10 in Firefox (clear profile without addons), although it works normally fast (every search) in Chrome.

@martabal
Copy link
Member Author

Yes it's still an issue for large collections

@martabal
Copy link
Member Author

One easy fix would be to disable the animation when you have more than 100 albums

@waclaw66
Copy link
Contributor

Another glitch I've observed with this PR is that resolution of album thumbnails is lowered when search is performed.

@waclaw66
Copy link
Contributor

One easy fix would be to disable the animation when you have more than 100 albums

That's only workaround.

I just wonder why it's fast for the first search only. The flip animation will probably change the html code so that the next animation will be much more complex. The same occurs with album sorting as well.

@martabal
Copy link
Member Author

The best solution would be to have an infinite-scroll mechanism instead of loading all albums. But that would be a big and complex refactor

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