-
Notifications
You must be signed in to change notification settings - Fork 87
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: Use resize_image
Built-in Function to Generate Image URLs
#46
base: master
Are you sure you want to change the base?
Conversation
By the way, I'm off-grid till the 17th of next month, so I won't be able to get to this till then. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, many thanks for all your hard work to get this fixed! 🥇
The solution is quite unexpected, but it seems to work just fine. I suggest also fixing the figure
shortcode after we got image
working as expected. I have left a few comments after some local testing.
{# ... then convert the page's colocated_path attribute to a string so it can be concatenated... #} | ||
{% set colocated_path = page.colocated_path | as_str %} | ||
{# ... and use `resize_image` to get image's URL for colocated blog posts and non-colocated blog posts. #} | ||
{% set image = resize_image(path=colocated_path ~ src, width=5000, height=5000, op="fit") %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're setting the image
variable which is a compound object.
{% set image = resize_image(path=colocated_path ~ src, width=5000, height=5000, op="fit") %} | |
{% set image = resize_image(path=colocated_path ~ src, width=5000, height=5000, op="fit") %} | |
{% set src = image.url %} |
templates/shortcodes/image.html
Outdated
{% endif %} | ||
<img src="{{ src | safe }}"{% if alt %} alt="{{ alt }}"{% endif %} class="{% if position %}{{ position }}{% else -%} center {%- endif %}" {%- if style %} style="{{ style | safe }}" {%- endif %} /> | ||
{% endif %} | ||
<img src="{{ image.url | safe }}"{% if alt %} alt="{{ alt }}"{% endif %} class="{% if position %}{{ position }}{% else -%} center {%- endif %}" {%- if style %} style="{{ style | safe }}" {%- endif %} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should keep using src
instead of image.url
here to make it work (at least that made it work properly in my limited testing, with both URLs and local file paths)
{# ... then convert the page's colocated_path attribute to a string so it can be concatenated... #} | ||
{% set colocated_path = page.colocated_path | as_str %} | ||
{# ... and use `resize_image` to get image's URL for colocated blog posts and non-colocated blog posts. #} | ||
{% set image = resize_image(path=colocated_path ~ src, width=5000, height=5000, op="fit") %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, unrelated to the above: I hope that this resizing won't decimate the image quality (e.g. in JPEG case, we'd really want to avoid re-encoding).
Can you confirm it's not going to re-encode JPEGs needlessly?
Hey @goingforbrooke a gentle ping here :) This PR is a good fix to merge, I wanted to clarify if you maybe had done some work on this? |
Hey @pawroman! Sorry for the long wait. It looks like we need to choose between Zola post structures. There might be a way to do both, but I haven't managed to figure it out. One post structure has images in Would you like to commit to one of these methods or maybe pursue a solution that works for both of them? |
Hey @goingforbrooke
I'd lean onto the solution that works for both, and in my limited testing (see prior comments), it should be possible. |
Updated branch from Testing... |
e77731f
to
4f3e58d
Compare
Using For testing, I'm looking at the sandwich image in this post. It's made with this: {{ image(src="raclette_sandwich.png",
alt="Gooey raclette cheese oozing out of a white bread sandwich",
style="border-radius: 8px;") }} It works with this ( <img src="{{ image.url | safe }}"{% if alt %} alt="{{ alt }}"{% endif %} class="{% if position %}{{ position }}{% else -%} center {%- endif %}" {%- if style %} style="{{ style | safe }}" {%- endif %} decoding="async" loading="lazy"/> But not this ( <img src="{{ src | safe }}"{% if alt %} alt="{{ alt }}"{% endif %} class="{% if position %}{{ position }}{% else -%} center {%- endif %}" {%- if style %} style="{{ style | safe }}" {%- endif %} decoding="async" loading="lazy"/> @pawroman what are you seeing in your tests? |
Fix Broken URLs Generated by the
image
Shortcode🪲 Issue
Zola doesn't seem to offer a universal way of getting an image's relative path. What works for a blog with colocated assets breaks in a blog with non-colocated assets. The root issue may have to do with dates not being stripped from permalink-generating built-in functions like
get_url
. See here for a deeper discussion.🔨 Solution
Using
resize_image
generates a valid image URL for colocated and non-colocated blog posts. We usefit
withwidth
andheight
set to 5000 so no resizing actually occurs. This is admittedly hacky, but it'll fix Terminimal sites while the Zola repo either adds a new built-in function, fixes(?)get_url
, or tells me that I've got it all wrong. 😉📚 Context
Fixes issue #42, which was created by (fix) PR #37 (source comment), which was troubleshooted and redesigned in draft PR #43.
🔮 Future
This fix should probably be preemptively applied to
figure
shortcodes as well.We may want to add image resizing args to the
image
shortcode so Terminimal blogs look nice on smaller screens.🙌🏻 Kudos
Special thanks to @ipetkov for notifying us of the change and for spectacular help with troubleshooting the introduced issue. Thanks to @pawroman for coordinating and for filing issue #42.