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 template tag for dynamic image url #2461

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@ychab
Copy link
Contributor

ychab commented Apr 7, 2016

Dynamic image view allow us to serve image rendition outside the main request (which is a real performance boost in some cases!! In my use case, it goes from ~400 to ~15 SQL queries!! ).

However, building rendition image url on Python side could be not enough. A common use-case could be do generate url in templates. I'm pretty sure I'm not the only one?

This PR basically copy/paste the example given in the docs : http://docs.wagtail.io/en/latest/advanced_topics/images/image_serve_view.html#generating-dynamic-image-urls-in-python

The util function is just wrapped into a simple template tag so it can still be used directly in Python code.

Hope it could be merge and thanks for review!


This change is Reviewable

@gasman gasman added this to the 1.5 milestone Apr 7, 2016

@kaedroho

This comment has been minimized.

Copy link
Member

kaedroho commented Apr 8, 2016

Thanks @ychab!

@@ -36,6 +37,15 @@ def verify_signature(signature, image_id, filter_spec, key=None):
return signature == generate_signature(image_id, filter_spec, key=key)


def generate_image_url(image, filter_spec, key=None):
signature = generate_signature(image.id, filter_spec, key)
url = reverse('wagtailimages_serve',

This comment has been minimized.

@kaedroho

kaedroho Apr 8, 2016

Member

Perhaps it would be useful to make the URL name configurable so a custom serve view can be used?

{% image_url 'wagtailimages_serve' myimage fill-100x100 %}

I think this also makes it more clearer that it's going to use the serve view rather than behaving like the {% image %} tag.

This comment has been minimized.

@ychab

ychab Apr 8, 2016

Contributor

Agree, done!

@kaedroho kaedroho self-assigned this May 4, 2016

@gasman gasman modified the milestones: 1.6, 1.5 May 11, 2016

@ychab ychab force-pushed the ychab:rendition_url_tag branch from 26c8c99 to eab4f94 May 18, 2016

@ychab

This comment has been minimized.

Copy link
Contributor

ychab commented May 18, 2016

@kaedroho , I have rebase the master branch to be more up-to-date. It could be practical to also have this PR merged for the 1.5 release since the PR #2459 was merged. Is that should be possible ? At least it needs doc. I was thinking about adding some notes at this place : https://github.com/torchbox/wagtail/blob/master/docs/topics/images.rst#images-embedded-in-rich-text. It sounds good to you?

EDIT: just after, of course

@gasman

This comment has been minimized.

Copy link
Collaborator

gasman commented May 18, 2016

@ychab 1.5 is in feature freeze now, sorry.

@gasman gasman modified the milestones: 1.7, 1.6 Aug 1, 2016

@gasman gasman modified the milestones: 1.8, 1.7 Oct 5, 2016

@gasman gasman modified the milestones: 1.9, 1.8 Dec 6, 2016

@gasman gasman modified the milestones: 1.10, 1.9 Feb 4, 2017

@gasman gasman modified the milestones: 1.11, 1.10 Apr 10, 2017

@gasman gasman modified the milestones: 1.12, 1.11 Jun 19, 2017

@gasman gasman modified the milestones: 1.13, 1.12 Aug 10, 2017

@gasman gasman modified the milestones: 1.13, 2.0 Oct 6, 2017

@gasman gasman removed this from the 2.0 milestone Jan 10, 2018

@zerolab

This comment has been minimized.

Copy link
Contributor

zerolab commented Oct 27, 2018

Discussed with @gasman and he agrees this can be added. Will submit a new PR with documentation updates. The rest LGTM

@m1kola

This comment has been minimized.

Copy link
Member

m1kola commented Oct 28, 2018

Finalised in #4856

@m1kola m1kola closed this Oct 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment