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

Add loading attribute #13276

Merged
merged 1 commit into from
Jan 17, 2025
Merged

Add loading attribute #13276

merged 1 commit into from
Jan 17, 2025

Conversation

nfriedli
Copy link
Contributor

All images shortcodes should have a loading attribute (for example for lazy).

All images shortcodes should have a loading attribute (for example for lazy).
@bep bep requested a review from jmooring January 17, 2025 15:06
@jmooring
Copy link
Member

This is fine, but the images are like 1k...

@nfriedli
Copy link
Contributor Author

I agree with you. In the vast majority of cases, the images are tiny.

But I think it's a good idea to add this option for 2 reasons:

  1. To be consistent with the figure.html shortcode, which features the loading attribute. This way, users really have control over image loading, with the shortcode provided by default.
  2. Because I can think of lots of other relevant uses, notably for creating deliberately large QR codes, with lots of text. I've just made a {{< qr level=“high” scale=16 >}}. And as I'll have several on the page, for poetry, it would be useful.

Personal note. I find the qr.html shortcode very, very well designed.

The variable harvesting part is great for overloading the default configuration. I wanted to create variables in the configuration, but it's so clear that I did it in my local shortcode, for example:

{{- $targetDir := or (.Get “targetDir”) “images/qr” }}
{{- $class := or (.Get “class”) “qr” }}
{{- $loading := or (.Get “loading”) “lazy” }}

Many thanks for your work, @jmooring & @bep.

@bep bep merged commit 8897113 into gohugoio:master Jan 17, 2025
6 checks passed
@nfriedli nfriedli deleted the patch-1 branch January 18, 2025 14:46
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.

3 participants