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

fix: update youtube placeholder default image url #235

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

serkodev
Copy link

@serkodev serkodev commented Sep 2, 2024

πŸ”— Linked issue

Fix #234

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Update the youtube placeholder url to https://i.ytimg.com/vi/${props.videoId}/hqdefault.jpg.
This thumbnail image url is guaranteed to exist. (Reference)

Copy link

vercel bot commented Sep 2, 2024

@serkodev is attempting to deploy a commit to the Nuxt Team on Vercel.

A member of the Team first needs to authorize it.

@harlan-zw
Copy link
Collaborator

harlan-zw commented Sep 2, 2024

Hey, thanks for the work!

I think this is a reasonable solution however we need to be mindful as this may break the look and feel of current users videos given the size difference, the current is 640x480 with this change it becomes 480x360.

Do you have any ideas on how to workaround this? I think the ideal case is probably two things:

  1. props to easily choose the right thumbnail size / webp
  2. a <img onerror fallback that serves the lower res safe size (if it's using the defaults). It would be nice to use picture or something for this but seems like it wouldn't work (<picture> and <source> elements: How should 404s with placeholder image data be handled?Β whatwg/html#8916)

@manniL
Copy link
Member

manniL commented Sep 2, 2024

Also I'd still go with the webp endpoint (smaller image size)

@serkodev
Copy link
Author

serkodev commented Sep 3, 2024

If we provide props to choose the thumbnail size or WebP format, I believe we don't have a perfect solution to detect if a YouTube video doesn't support these options.

  1. Use fetch to request the image and check the response code (we all know this isn't a good approach).
  2. Fetch the YouTube Data API to get a list of thumbnail URLs (this method requires an API key and is not very client-side rendering-friendly).

There's a somewhat hacky method that involves detecting the image size.
Since all 404 images will have a size of 120x90, we can use this characteristic to detect whether a thumbnail exists. If we detect that the image size is 120x90, we can then fallback to default.jpg.

The only drawback is that if the original image request (e.g., default.jpg, 1.jpg, 2.jpg, 3.jpg) already has a size of 120x90, we can't determine if it's actually a 404. It may be forcibly fallback to default.jpg, unless we calculate the image data (e.g., hash) to determine if it's a not-found image, but that would make things increasingly complicated.

However, if we only provide props for thumbnail size or WebP format and let users determine whether the image is supported, then we might not need to detect if it's a 404 at all.

That's all I can think of for now. 🀣

@harlan-zw
Copy link
Collaborator

harlan-zw commented Sep 4, 2024

I think the dimension heuristic is good enough and is free compared to doing extra requests manually.

<img src="https://i.ytimg.com/vi_webp/jNQXAC9IVRw/sddefault.webp" onload="if (this.naturalWidth == 120 && this.naturalHeight == 90) { (this.src = `https://i.ytimg.com/vi/jNQXAC9IVRw/hqdefault.jpg`) }" />

We should warn the developer in development that they should use the props when this fallback occurs.

@serkodev
Copy link
Author

serkodev commented Sep 4, 2024

Thanks for the feedback.

How about the behavior of the placeholder slot prop?

  • Should we keep passing sddefault.webp as placeholder ?
  • Or should we need to add a placeholderFallback (to pass hqdefault.jpg) slot prop?

As discussed previously, we can pass all formats of thumbnail links for the user to choose from. If we directly turn placeholder into a map containing all URLs, it might impact existing users.

  • Should we pass an additional placeholders prop including all URLs?

@serkodev serkodev marked this pull request as draft September 4, 2024 18:34
const placeholder = computed(() => `https://i.ytimg.com/vi_webp/${props.videoId}/sddefault.webp`)
const isFallbackPlaceHolder = ref(false)

Copy link
Author

Choose a reason for hiding this comment

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

Suggested change
type YouTubeThumbnailType =
// 120x90
'1' | '2' | '3' | 'default' |
// 320x180
'mq1' | 'mq2' | 'mq3' | 'mqdefault' |
// 480x360
'0' | 'hq1' | 'hq2' | 'hq3' | 'hqdefault' |
// 640x480
'sd1' | 'sd2' | 'sd3' | 'sddefault' |
// 1280x720
'hq720' |
// 1920x1080
'maxresdefault'
function getYouTubeThumbnail(type: YouTubeThumbnailType, webp = false) {
return `https://i.ytimg.com/${webp ? 'vi_webp' : 'vi'}/${props.videoId}/${type}.${webp ? 'webp' : 'jpg'}`
}

Just draft a function to get YouTube thumbnail here. I think we can use this for placeholder or provide for user to use?

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.

YouTube placeholder cannot show some videos
3 participants