Open nagmat84 opened 2 years ago
Is this still in the roadmap? As the pages load default tags, but there's nothing in the content. The ability to set a site wide title/image/description would be a great placeholder if this is still pretty far down the road.
Current live page tags (anonymized).
It was on the road map, unfortunately, I have to rewrite the full front-end. See here: https://github.com/LycheeOrg/Lychee/discussions/2485
v6 is amazing so far, and thank you for digging into this, and working to resolve this issue!
Looks like it's not resolved on my end yet, but I do see it pulling in the title, description, and URL, but not the og:image or twitter:image
-- |
Checked the Docker compose and .env to make sure mine are current, but I'm not seeing anything else I'd be missing. Is there something I'd need to do or test to enable this feature?
Sorry, didn't realize raw code wouldn't post, screenshot attached.
You can post raw code by using ```
Thank you for your feedback. I will check to also add the header image in that.
@SSj-Saturn Can you check something for me?
That's the code.
if (session()->has('album')) {
/** @var AbstractAlbum $album */
$album = session()->get('album');
$this->pageTitle = $album->title;
if ($album instanceof BaseAlbum) {
$this->pageDescription = $album->description ?? Configs::getValueAsString('site_title');
}
$this->imageUrl = $this->getHeaderUrl($album) ?? '';
}
And then
protected function getHeaderUrl(AbstractAlbum $album): ?string
{
if (Configs::getValueAsBool('use_album_compact_header')) {
return null;
}
if ($album instanceof Album && $album->header_id === AlbumController::COMPACT_HEADER) {
return null;
}
if ($album->photos->isEmpty()) {
return null;
}
// TODO : already use the prefetched data for photos instead of 2 extra queries?
return $this->getByQuery($album);
}
private function getByQuery(AbstractAlbum $album): ?string
{
$headerSizeVariant = null;
if ($album instanceof Album && $album->header_id !== null) {
$headerSizeVariant = SizeVariant::query()
->where('photo_id', '=', $album->header_id)
->whereIn('type', [SizeVariantType::MEDIUM, SizeVariantType::SMALL2X, SizeVariantType::SMALL])
->orderBy('type', 'asc')
->first();
}
if ($headerSizeVariant !== null) {
return $headerSizeVariant->url;
}
$query_ratio = SizeVariant::query()
->select('photo_id')
->whereBelongsTo($album->photos)
->where('ratio', '>', 1) // ! we prefer landscape first.
->whereIn('type', [SizeVariantType::MEDIUM, SizeVariantType::SMALL2X, SizeVariantType::SMALL]);
$num = $query_ratio->count() - 1;
$photo = $query_ratio->skip(rand(0, $num))->first();
if ($photo === null) {
$query = SizeVariant::query()
->select('photo_id')
->whereBelongsTo($album->photos)
->whereIn('type', [SizeVariantType::MEDIUM, SizeVariantType::SMALL2X, SizeVariantType::SMALL]);
$num = $query->count() - 1;
$photo = $query->skip(rand(0, $num))->first();
}
return $photo === null ? null : SizeVariant::query()
->where('photo_id', '=', $photo->photo_id)
->where('type', '>', 1)
->orderBy('type', 'asc')
->first()?->url;
}
So can you check that:
It does work fine on my install see e.g. : https://photography.viguier.nl/gallery/GrR5EIpqKX2jX43-YhLM0XvA
<!--General Meta Data -->
<title>Lego</title>
<meta name="description" content="Benoit Viguier - Landscapes & Ballroom Photography">
<meta name="author" content="Benoît Viguier">
<meta name="publisher" content="Benoît Viguier">
<!-- Twitter Meta Data -->
<meta name="twitter:title" content="Lego">
<meta name="twitter:description" content="Benoit Viguier - Landscapes & Ballroom Photography">
<meta name="twitter:image" content="https://photography.viguier.nl/uploads/medium/fd/ea/00db78ff07751b5e69d35edeb0bb.jpg">
<!-- OpenGraph Meta Data (e.g. used by Facebook) -->
<meta property="og:title" content="Lego">
<meta property="og:description" content="Benoit Viguier - Landscapes & Ballroom Photography">
<meta property="og:image" content="https://photography.viguier.nl/uploads/medium/fd/ea/00db78ff07751b5e69d35edeb0bb.jpg">
<meta property="og:url" content="https://photography.viguier.nl/gallery/GrR5EIpqKX2jX43-YhLM0XvA">
Hey Thanks for the update, yeah it looks like it only works if you have a header image set to the album? As in only if there's an image inside of the Album, and set as the header image, and not sub folders?
Going from your site, I do see the one you provided works fine, but this other with only sub folders doesn't pull anything.
https://photography.viguier.nl/gallery/MCjsR1HBs5YkfcrM07aHHtLY
Terminiology wise I did have "Disable the header image in albums" turned on, which I think is the same feature as compact header? My main issues is that nearly every photo is a terrible crop lol.
So I think I understand the system now, but the native crop isn't good for my photography (mostly single portraits etc), maybe there's an option for adding in a header image to albums without an image inside (eg. Events / Conventions / NYCC / [MODEL NAME]. There's a lot of nesting there, but if I want to share the whole NYCC album, it'd be great if it had a photo to it vs. broken).
Or alternatively if the album doesn't have any images to reference, it can pull a default image? (eg. maybe the landing background image or something you can set once as a default?). Something is better than nothing haha.
And appreciate you looking into this!
I think default image should be square image of the Lychee logo and it could/should be configurable in the settings.
if the link is for album that is hidden, show default image. Do not leak the pics in meta tags!
Do not leak the pics in meta tags!
I'm pretty sure that "leaking" was a large part of the point.
if the link is for album that is hidden, show default image. Do not leak the pics in meta tags!
???
Wait so you want a share link available for hidden folders, but it not to have a share image? Wouldn't password protection do what you want? Otherwise it's still public with the link?
I mean that album sharing settings should be taken into account when the the meta pic is chosen.
I put it other way: Should public but hidden album have meta image from the album? I think not.
I put it other way: Should public but hidden album have meta image from the album? I think not.
To the contrary, I think yes, because you already have access to the url. And the data that the album is (if you are worried of scrapping) is also available as we return a 200. So you do have access to the image.
However I do agree there is a blind sight on password protected ones. I will fix that oversight.
I could make a configuration setting to disable photo integration in links.
And also I agree that we should display the landing image or default Lychee icon if none is provided.
I put it other way: Should public but hidden album have meta image from the album? I think not.
To the contrary, I think yes, because you already have access to the url. And the data that the album is (if you are worried of scrapping) is also available as we return a 200. So you do have access to the image.
I think that there should be some indication to the user that the album is public but hidden. meta desc would be nice place to it as in many cases people ask the album link again rather than bookmark the link because they do not understand the link is somehow special. There is no indication about this in the album either. Hiding the meta pic would be another indication of this.
I think that there should be some indication to the user that the album is public but hidden.
Indeed.
in many cases people ask the album link again rather than bookmark the link because they do not understand the link is somehow special. There is no indication about this in the album either. Hiding the meta pic would be another indication of this.
I disagree that hiding the meta pic is an indication of this as it is not obvious of the meaning, I think adding a banner on top would be a better solution for such message. 👍
To the contrary, I think yes, because you already have access to the url. And the data that the album is (if you are worried of scrapping) is also available as we return a 200. So you do have access to the image.
Agreed. If they have the link, they can see the image by clicking it, so hiding it doesn't achieve anything.
in many cases people ask the album link again rather than bookmark the link
I am firmly of the opinion that such people should not be allowed on the internet unsupervised. Whatever medium they received the link probably has a copy.
I disagree that hiding the meta pic is an indication of this as it is not obvious of the meaning,
Agreed.
Well anyways, that was my reasoning.
just to point out: If the landing image is used as "default" pic then it needs to indicated as such otherwise it is not obvious if it is not in use.
Currently, our web frontend is not very SEO friendly. It is entirely written in JS, which is fine for modern web crawlers, but it violates some best practices.
Enhancement: The frontend should set
<title>
and<meta>
tags programmatically for every loaded "page".See https://developers.google.com/search/docs/crawling-indexing/javascript/javascript-seo-basics#titles-and-snippets.