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

Remove args to see if gifs work #173

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Remove args to see if gifs work #173

wants to merge 2 commits into from

Conversation

carkod
Copy link
Contributor

@carkod carkod commented Mar 19, 2024

Test using this demo but we need a .gif yet

@bartaz
Copy link
Member

bartaz commented Mar 19, 2024

Blog is throwing 500 on anything.

This older article has a gif: https://ubuntu-com-13680.demos.haus/blog/lunar-lobster-has-landed (https://ubuntu.com/blog/lunar-lobster-has-landed)

I don't think that just removing params would work. They are probably mandatory (in image template) but need another value?

We should test it out without blog first. Just using image template with some gif. Then figure out what params we should use for gifs.

@bartaz
Copy link
Member

bartaz commented Mar 19, 2024

There are 2 gifs on asset server for noble numbat, so we could try with them directly first:
https://assets.ubuntu.com/manager?q=noble&type=gif

Asset server even gives you "image tag" snippet (I don't know if these are the same options that are used in blog module):

          {{ image (
            url="https://assets.ubuntu.com/v1/31ae60ab-noble-numbat-mascot-animated.gif",
            alt="",
            width="600",
            height="600",
            hi_def=True,
            loading="auto|lazy"
            ) | safe
          }}

@@ -252,9 +252,6 @@ def _apply_image_template(
alt="",
width=img_width or width,
height=img_height or height,
hi_def=True,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi_def is mandatory, so it needs to be set to False if we want to try opposite value.

e_sharpen is not documented at all, so I don't know what it's doing or what value it has, but likely it's not mandatory (as we don't use it in other places it seems)

https://github.com/canonical/canonicalwebteam.image-template?tab=readme-ov-file#parameters

@bartaz
Copy link
Member

bartaz commented Mar 19, 2024

On top of the params passed here from blog module, there is a bunch of params our image_template passes to cloudinary by default, without any way to override them:

image

https://github.com/canonical/canonicalwebteam.image-template/blob/1aa6941e4f61cc96dbcf4b7a92d2c069304c8c89/canonicalwebteam/image_template/__init__.py#L34-L38

f_auto can potentially mean that gif is changed to some other format, possibly affecting animation quality

q_auto allows cloudinary to automatically choose the quality of transformed image, which can have impact on the gif quality as well

see: https://cloudinary.com/documentation/image_transformations

We would need to test out those options, one by one, to figure out how they affect gifs, and test them in different browsers (as they may affect result as well, because cloudinary adjusts format to what browser accepts).

Then quite likely we need to make some of the options that are currently set by default optional, so they can be overridden if needed. Or maybe we need to define a separate set of defaults for animated gifs.

@carkod carkod marked this pull request as draft March 19, 2024 20:05
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.

2 participants