AlmaLinux / almalinux.org

almalinux.org official web site sources.
https://almalinux.org
Creative Commons Attribution Share Alike 4.0 International
39 stars 49 forks source link

Added YouTube shortcode width and height functionality #568

Closed Noam-Alum closed 1 month ago

Noam-Alum commented 1 month ago

I added a modified version of the shortcode for YouTube embeds and updated all pages using iframes to use the shortcode.

These are the pages I modified: Link Changed?
https://almalinux.org/blog/2023-12-14-2023-highlights/ Yes
https://almalinux.org/blog/2024-01-16-video-contributions/ Yes
https://almalinux.org/blog/2024-05-06-announcing-94-stable/ Yes
https://almalinux.org/blog/2024-05-01-almalinux-day-germany-recap/ Yes
https://almalinux.org/blog/2024-02-29-feb-event-recap/ Yes
https://almalinux.org/blog/2024-02-01-aldt-recap/ Yes

The shortcode now can be used to set the height and width of an iframe, and with the help of @mattlasheboro I also added better support for mobile users.

This is the general syntax to use the shortcode:

{{< youtube id="AEA8losuKsA" width="45%" height="25%" autoplay="false" controls="true" mute="false" title="YouTube video player" >}}

This PR is to solve this issue.

codyro commented 1 month ago

Preview URLs of the pages changed to use the shortcode.

https://e8b754ef.almalinux-org.pages.dev/blog/2023-12-14-2023-highlights/ https://e8b754ef.almalinux-org.pages.dev/blog/2024-01-16-video-contributions/ https://e8b754ef.almalinux-org.pages.dev/blog/2024-05-06-announcing-94-stable/ https://e8b754ef.almalinux-org.pages.dev/blog/2024-05-01-almalinux-day-germany-recap/ https://e8b754ef.almalinux-org.pages.dev/blog/2024-02-29-feb-event-recap/ https://e8b754ef.almalinux-org.pages.dev/blog/2024-02-01-aldt-recap/

Noam-Alum commented 1 month ago

On this page: https://e8b754ef.almalinux-org.pages.dev/blog/2023-12-14-2023-highlights/ there is still a horizontal overflow issue caused by the "2023-12-10-almalinux_baseos_systems-timeseries-line-variant.png" image:

image

Original code:

<a href="https://jonathanspw.fedorapeople.org/alma/2023-12-10/2023-12-10-almalinux_baseos_systems-timeseries-line-variant.png"><img style="max-width:600px;" src="/blog-images/2023-12-10-almalinux_baseos_systems-timeseries-line-variant.png" alt="AlmaLinux adoption numbers from Jan 2022 to Dec 2023"></a>

We can change that to something like this:

<a href="https://jonathanspw.fedorapeople.org/alma/2023-12-10/2023-12-10-almalinux_baseos_systems-timeseries-line-variant.png"><img style="max-width:47%;" src="/blog-images/2023-12-10-almalinux_baseos_systems-timeseries-line-variant.png" alt="AlmaLinux adoption numbers from Jan 2022 to Dec 2023"></a>
codyro commented 1 month ago

On this page: https://e8b754ef.almalinux-org.pages.dev/blog/2023-12-14-2023-highlights/ there is still a horizontal overflow issue caused by the "2023-12-10-almalinux_baseos_systems-timeseries-line-variant.png" image:

Original code:

<a href="https://jonathanspw.fedorapeople.org/alma/2023-12-10/2023-12-10-almalinux_baseos_systems-timeseries-line-variant.png"><img style="max-width:600px;" src="/blog-images/2023-12-10-almalinux_baseos_systems-timeseries-line-variant.png" alt="AlmaLinux adoption numbers from Jan 2022 to Dec 2023"></a>

We can change that to something like this:

<a href="https://jonathanspw.fedorapeople.org/alma/2023-12-10/2023-12-10-almalinux_baseos_systems-timeseries-line-variant.png"><img style="max-width:47%;" src="/blog-images/2023-12-10-almalinux_baseos_systems-timeseries-line-variant.png" alt="AlmaLinux adoption numbers from Jan 2022 to Dec 2023"></a>

If it fixes the issue on mobile/everywhere else, I don't see why not. This isn't my forte, so I'll defer to you and @mattlasheboro here.

Noam-Alum commented 1 month ago

I think we should involve benny since this is her post, but I suggest we consider a better solution, something along the lines of a new shortcode for images so we can edit the height, width, link, and alt while still supporting phones or smaller screens.

codyro commented 1 month ago

Does Hugo have built in image processing that could handle this? Or any other helpers?

mattlasheboro commented 1 month ago

For the issue on: https://e8b754ef.almalinux-org.pages.dev/blog/2023-12-14-2023-highlights/

We have a CSS rule that seeks to prevent issues overflowing the page, it is:

.al-page-blog-post article .al-article-content img { max-width: 100%; }

In the future I would avoid using max-width inline on images. In order to fix this one I would change:

<a href="https://jonathanspw.fedorapeople.org/alma/2023-12-10/2023-12-10-almalinux_baseos_systems-timeseries-line-variant.png"><img style="max-width:600px;" src="/blog-images/2023-12-10-almalinux_baseos_systems-timeseries-line-variant.png" alt="AlmaLinux adoption numbers from Jan 2022 to Dec 2023"></a>

TO:

<a href="https://jonathanspw.fedorapeople.org/alma/2023-12-10/2023-12-10-almalinux_baseos_systems-timeseries-line-variant.png"><img style="width:600px;" src="/blog-images/2023-12-10-almalinux_baseos_systems-timeseries-line-variant.png" alt="AlmaLinux adoption numbers from Jan 2022 to Dec 2023"></a>

I agree that we should use a shortcode for image rendering. I can look into adding this, but I think it would be outside of the scope of this PR.

Does Hugo have built in image processing that could handle this? Or any other helpers?

You can do all kinds of image manipulation like resizing, rotating, filtering in the newer versions of hugo. To my knowledge we would have to move the images to the assets folder to be able to access them for these features. I think you can use them as a remote resource outside of the assets folder but it adds a ton of overhead and build time. I'm also assuming that if we use the built in functions it would not produce an image as optimized as we get using 3rd party lossless compression tools.

bennyvasquez commented 1 month ago

This is freaking awesome. Thanks to you ALL for getting this done lined up. Just a couple things before we merge it in:

1) With the creation of the youtube shortcode file, does that mean we're completely overriding the default one? or does it expand the functionality of the existing Hugo shortcode? If it's the former, we need to document that somewhere that makes sense, so it doesn't get lost. Maybe in the README? "Deviations from Hugo's defaults" or something like that? If it's the latter, that'll get addressed in 2. :D 2) Can we get how to use the youtube shortcode and its potential added to the documentation as part of this PR? I don't want it to get forgotten or dropped. 3) I agree with @mattlasheboro that we should split the image conversation into a different PR/Issue, so we can address that one correctly, too. 4) I just realized I never changed the titles to what they should be, and had just used youtube's default "youtube player" title for most of those. If you're up to it, @Noam-Alum, those should be replaced with the titles of the videos. If not, let's open an issue to address it. That'd be a good issue for a new contributor to get their feet wet.

Noam-Alum commented 1 month ago

This is freaking awesome. Thanks to you ALL for getting this done lined up. Just a couple things before we merge it in:

  1. With the creation of the youtube shortcode file, does that mean we're completely overriding the default one? or does it expand the functionality of the existing Hugo shortcode? If it's the former, we need to document that somewhere that makes sense, so it doesn't get lost. Maybe in the README? "Deviations from Hugo's defaults" or something like that? If it's the latter, that'll get addressed in 2. :D
  2. Can we get how to use the youtube shortcode and its potential added to the documentation as part of this PR? I don't want it to get forgotten or dropped.
  3. I agree with @mattlasheboro that we should split the image conversation into a different PR/Issue, so we can address that one correctly, too.
  4. I just realized I never changed the titles to what they should be, and had just used youtube's default "youtube player" title for most of those. If you're up to it, @Noam-Alum, those should be replaced with the titles of the videos. If not, let's open an issue to address it. That'd be a good issue for a new contributor to get their feet wet.

It was fun tinkering with how I could fix this issue in the best way :smile:, hopefully I did well.

  1. The new shortcode overwrites the original Hugo shortcode with the existing shortcode as I stated here, with the added changes to create the new functionality.
  2. "Can we get how to use the youtube shortcode", should this be part of this PR?
  3. I think @mattlasheboro mentioned that he can look into it (he seems to have insight on this case) so I'll leave opening the issue for his sake.
  4. Should this also be part of this PR?

@bennyvasquez let me know what you think and I'll cover 1,2 and 4 as soon as I can.

bennyvasquez commented 1 month ago

It was fun tinkering with how I could fix this issue in the best way 😄, hopefully I did well.

  1. The new shortcode overwrites the original Hugo shortcode with the existing shortcode as I stated here, with the added changes to create the new functionality.

Sweet! Tyty. This was a limitation of my understanding, so this helps me a lot.

  1. "Can we get how to use the youtube shortcode", should this be part of this PR?

I think so, yeah. Documentation is important, and I think it should come along with the new "feature".

  1. Should this also be part of this PR?

Nah, you're right to call this one out: it should be part of another PR, just to keep it clean.

Noam-Alum commented 1 month ago

I think it should come along with the new "feature".

I didn't notice that :fearful:, but I think the placement is good since this would be part of the "meat of your post" :laughing:, I can change that if you want.

bennyvasquez commented 1 month ago

I think it should come along with the new "feature".

I didn't notice that 😨, but I think the placement is good since this would be part of the "meat of your post" 😆, I can change that if you want.

OH I meant - because we're adding the feature in this PR, the documentation should be included in this PR. Adding it where you did seems just fine!

Also, hilariously, it appears I've been using an alias (which I assume is only there because of folks who use typos like me) to test hugo sites for... well, as long as I've used hugo: https://discourse.gohugo.io/t/hugo-serve-vs-hugo-server/24872/2