LycheeOrg / Lychee

A great looking and easy-to-use photo-management-system you can run on your server, to manage and share photos.
https://lycheeorg.github.io/
MIT License
3.2k stars 288 forks source link

New feature for setting specific album header image #2202 #2377

Closed aSouchereau closed 2 months ago

aSouchereau commented 2 months ago

Fixes #2202

Adds column in albums table to store id user specified header image. Adds api route for setting the album header image. Modifies the album livewire component to fetch specified image or random image if unset. Adds context menu option to set/unset header image. Adds properties menu option to set header image by title.

aSouchereau commented 2 months ago

So in /app/Livewire/Components/Pages/Gallery/Album.php, I changed the random query from the original because it restricted the selection of images to one variant type at a time. i.e. If an album has photos with medium variants, it only selects from those photos with those variants, even if there are suitable photos that only have small2x variants.

The difference now is that we can randomly select any photo in the album as long as they meet certain requirements, and then query for the largest variant of that photo.

I'd like your input on what those requirements should be. I'm assuming we shouldn't just use the original every time because it could potentially be too large. So maybe limit by variant type (Medium or smaller?), or by file-size/dimensions?

ildyria commented 2 months ago

Hi, first and foremost, very good PR. :)

I changed the random query from the original because it restricted the selection of images to one variant type at a time.

return SizeVariant::query()
                ->where('photo_id', '=', $photo->photo_id)
                ->orderBy('type', 'asc')
                ->first();

The problem with that query is that you will then by default always have the original selected (type = 0), that is why limited to medium and then small2x. That way we limit also the weight of the page at load. :)

I'd like your input on what those requirements should be. I'm assuming we shouldn't just use the original every time because it could potentially be too large. So maybe limit by variant type (Medium or smaller?), or by file-size/dimensions?

Sorry I read too fast, I would say between medium, small2x and small (So probably 2, 3, 4). I would avoid to use the 5 & 6 as they are corresponding to square thumbs.

ildyria commented 2 months ago

Option to manually unset header image

It could be an interesting addition to also add this possibility of selection in the properties of the album (when you press i). That way you can select the photo by title and have a small x to unset it. Something here: https://github.com/LycheeOrg/Lychee/blob/master/app/Livewire/Components/Forms/Album/Properties.php

ildyria commented 2 months ago

I would remove the foreign constraint on the column and simply make sure that when the header_id is set but pointing to a null image, it handles it gracefully (Or simply add some code in the delete() function of Photo to handle those case).

It is less elegant, I agree, but that is the cost to pay if we want to have the flexibility with SQLite.

aSouchereau commented 2 months ago

Foreign key constraint is removed, and I've changed the fetch function so it will use the random query if the header photo is missing, instead of just returning null.

However, that still leaves the header_id column set. Which I guess is fine for now, but just for consistency, it might be worth solving alongside another issue I've found.

I need to figure out a way to unset the header_id column when the header photo is moved to another album. I'm not too familiar with the code base just yet, my best guess is that it would have to be handled in both the PhotoController::setAlbum() method for the api and the Livewire/Photo/Move component for the UI.

Similarly, on deletion it could potentially be handled in the Photo/Delete action class

ildyria commented 2 months ago

Foreign key constraint is removed, and I've changed the fetch function so it will use the random query if the header photo is missing, instead of just returning null.

:heavy_check_mark:

ildyria commented 2 months ago
  • handling deletion and moving of header image
  • option to set header by title in album properties menu

:heavy_check_mark:
:heavy_check_mark:

ildyria commented 2 months ago

@aSouchereau I updated your branch, could you check if you are happy with those changes ? :)

aSouchereau commented 2 months ago

@aSouchereau I updated your branch, could you check if you are happy with those changes ? :)

Yes looks good, thank you

aSouchereau commented 2 months ago

Just an idea. The original issue mentions an option to set compact header setting per album rather than globally. Maybe this could be implemented here by adding an item at the top of the dropdown list to enable it. It could then be represented in the db by a string, something like "none" or "compact" since we cant use null in place of the photo id.