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

Allow loading BoosterYoutube image non-lazily #1055

Closed
amplitudesxd opened this issue Jul 15, 2024 · 15 comments
Closed

Allow loading BoosterYoutube image non-lazily #1055

amplitudesxd opened this issue Jul 15, 2024 · 15 comments

Comments

@amplitudesxd
Copy link

amplitudesxd commented Jul 15, 2024

Is your feature request related to a problem? Please describe.
By default, the placeholder image for BoosterYoutube is loaded lazily. This can slow down the LCP if it's in the main viewport.
image

Describe the solution you'd like
You should be able to decide if the BoosterYoutube image is loaded lazily or not.

Describe alternatives you've considered
N/A - looked through the code, couldn't find any workaround.

Additional context
N/A

@ThornWalli
Copy link
Contributor

Hello @amplitudesxd,

I think this can be solved.

Are you working with the critical attribute?

https://basics.github.io/nuxt-booster/guide/usage.html#critical-prop-for-critical-components

If the component recognizes that it is in a critical component, then the poster is delivered as a preload. This can solve the problem.

Call the useCritical() in the setup of the component where the video is integrated and give it a critical attribute. This is then inherited downwards.

@amplitudesxd
Copy link
Author

amplitudesxd commented Jul 17, 2024

Hey @ThornWalli,

Thanks for that information - I've tried adding the critical prop to the BoosterYoutube element, but the loading="lazy" still persists, and lighthouse continues to show that warning.

@ThornWalli
Copy link
Contributor

@amplitudesxd That should work.

components/ComponentA.vue

<template>
  <div>
    <booster-youtube :url="url" />
  </div>
</template>

<script setup>
import BoosterYoutube from '#booster/components/BoosterYoutube';
import { useBoosterCritical } from '#imports';
useBoosterCritical();

const url = ref('https://www.youtube.com/watch?v=cLKvbhfVBUU');
</script>

pages/index.vue

<template>
  <div>
    <ComponentA critical />
  </div>
</template>

image

@amplitudesxd
Copy link
Author

amplitudesxd commented Jul 21, 2024

Hi @ThornWalli, thanks for that example - I'm still not quite sure if I'm doing something wrong.

My code is as follows, but it still seems to load lazily:

<script setup lang="ts">
import BoosterYoutube from '#booster/components/BoosterYoutube';
import { useBoosterCritical } from '#imports';

useBoosterCritical();
</script>

<template>
  <div>
    <BoosterYoutube
      url="https://youtube.com/watch?v=JxS5E-kZc2s"
      title="Funny Cats Compilation (Most Popular) Part 1"
      critical
    />
  </div>
</template>

@ThornWalli
Copy link
Contributor

Hello @amplitudesxd,

you can try the [email protected] version.
I have added a useBoosterCritical to the SpeedkitYoutube and SpeedkitVimeo in the setup for your use case.

Now it is also possible to set a critical attribute on the SpeedkitYoutube and SpeedkitVimeo tag.

@amplitudesxd
Copy link
Author

amplitudesxd commented Jul 21, 2024

@ThornWalli Thank you! That seems to do the trick.

I think would be helpful to add a prop to customize the NuxtImage values, similar to the :options prop for the YouTube player. Another issue I'm facing is that images aren't loading because I'm using Cloudflare as the default nuxt-image provider. Cloudflare, like most providers, doesn't support external URLs, but IPX does. If we had this customization option, I could configure IPX for the BoosterYouTube images, while using Cloudflare everywhere else.

@ThornWalli
Copy link
Contributor

@amplitudesxd I see, what exactly are you missing?
Only the modifiers or are you also working with a preset?

https://image.nuxt.com/usage/nuxt-img#modifiers
https://image.nuxt.com/usage/nuxt-img#preset

Consider implementing them as a property.
For example posterSrcModifiers, posterSrcPreset.

In the Next-Branch you can already specify your own Src via posterSrc.

@amplitudesxd
Copy link
Author

@ThornWalli mainly, I need to be able to set the the provider prop on the NuxtImage component used inside of the BoosterYoutube component, though I feel as it could be useful to allow modifying all of the NuxtImage props.

@ThornWalli
Copy link
Contributor

ThornWalli commented Jul 23, 2024

I've given it some more thought 😉
I will rework the poster on Youtube and Vimeo.

The plan now is that there is only one property to define the poster sources.

{
    posterSources: {
      type: Array,
      default() {
        return [
          {
            src: null,
            media: 'all',
            sizes: {
              default: '100vw'
            }
          }
        ];
      }
    }
}

With Preset:

{
    posterSources: {
      type: Array,
      default() {
        return [
          {
            src: null,
            preset: 'name',
            media: 'all',
            sizes: {
              default: '100vw'
            }
          }
        ];
      }
    }
}

With Modifiers

{
    posterSources: {
      type: Array,
      default() {
        return [
          {
            src: null,
            media: 'all',
            modifiers: {
              negate: true
            },
            sizes: {
              default: '100vw'
            }
          }
        ];
      }
    }
}

With custom src

{
    posterSources: {
      type: Array,
      default() {
        return [
          {
            src: '/img/image-text-b.jpg',
            media: 'all',
            sizes: {
              default: '100vw'
            }
          }
        ];
      }
    }
}

@amplitudesxd
Copy link
Author

amplitudesxd commented Jul 23, 2024

@ThornWalli that looks like an improvement - but from my look at the code, you still wouldn't be able to specify the provider property for the image, would you?

The overall idea would be that I can render the BoosterYoutube thumbnails with the IPX provider while using Cloudflare for everything else.

Something like:

<template>
  <div>
    <BoosterYoutube
      url="https://youtube.com/watch?v=JxS5E-kZc2s"
      title="Funny Cats Compilation (Most Popular) Part 1"
      :imageOptions="{ provider: 'ipx'}"
      critical
    />
  </div>
</template>

Which would then eventually render the NuxtImage component with the properties specified in imageOptions, something similar to <NuxtImage v-bind="imageOptions" />.

I could try to make a PR if that'd help, but I'm quite unfamiliar with this project.

@ThornWalli
Copy link
Contributor

ThornWalli commented Jul 24, 2024

Hello @amplitudesxd,

I already understood that 🙂

Just wanted to add a little more flexibility.

The sources that are defined are in theory individual NuxtImage representations. More about this: https://github.com/basics/nuxt-booster/blob/main/src/runtime/components/BoosterImage/classes/Source.js

The source in this case is the NuxtImage.

Your use case:

const videoYoutube = {
  title: 'Youtube',
  url: 'https://www.youtube.com/watch?v=cLKvbhfVBUU',
  text: 'Nunc odio nisl dapibus consequat recusandae doloremque nisi natus repudiandae do accusantium corrupti. Harum quisquam, maxime, perspiciatis lobortis earum iure.',
  posterSources: [
    {
      provider: 'ipx',
      sizes: {
        default: '100vw'
      }
    }
  ]
};

Another use case where the image is played out on different orientations:

const videoYoutube = {
  title: 'Youtube',
  url: 'https://www.youtube.com/watch?v=cLKvbhfVBUU',
  text: 'Nunc odio nisl dapibus consequat recusandae doloremque nisi natus repudiandae do accusantium corrupti. Harum quisquam, maxime, perspiciatis lobortis earum iure.',
  posterSources: [
    {
      provider: 'ipx',
      media: '(orientation: landscape)',
      sizes: {
        default: '100vw'
      }
    },
    {
      provider: 'otherProvider',
      media: '(orientation: portrait)',
      modifiers: {
        ratio: 1
      },
      sizes: {
        default: '100vw'
      }
    }
  ]
};

The next step would be to deploy the changes to next.

@amplitudesxd
Copy link
Author

amplitudesxd commented Jul 24, 2024

@ThornWalli, ah okay -- that makes sense, I wasn't aware you could specify a provider in sources like that 😊

The changes you've made look great and are a fantastic addition to the project. Thank you for making this!

@ThornWalli
Copy link
Contributor

@amplitudesxd, you can test the change now npm i [email protected].

@amplitudesxd
Copy link
Author

@ThornWalli seems to be working! Thank you very much :)

@ThornWalli
Copy link
Contributor

Perfect! 🎉 Will be officially included soon 😉

@ThornWalli ThornWalli mentioned this issue Aug 1, 2024
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

No branches or pull requests

2 participants