Fix: Use resize_image Built-in Function to Generate Image URLs
#46
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.
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.
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 %} |
| {% 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.
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.
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? |
Fix Broken URLs Generated by the
imageShortcode馃 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_imagegenerates a valid image URL for colocated and non-colocated blog posts. We usefitwithwidthandheightset 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
figureshortcodes as well.We may want to add image resizing args to the
imageshortcode 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.