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

margin filter / give Thumbnail file a margin property #57

Closed
hvdklauw opened this issue Nov 30, 2010 · 4 comments
Closed

margin filter / give Thumbnail file a margin property #57

hvdklauw opened this issue Nov 30, 2010 · 4 comments
Labels

Comments

@hvdklauw
Copy link

the new sorl thumbnail has a margin filter, which is all very nice but requires me to duplicate the size specification.

Would be nice if the ThumbnailFile also knew the requested size so it knew what the margin had to be to make the image centered on the requested size.

Something like:

{% thumbnail model.image "100x100" as im %}
<img src="{{ im.url }}" style="margin:{{ im.margin }}" alt="Image" width="{{ im.width }}" height="{{ im.height }}" />
@SmileyChris
Copy link
Owner

I guess that'd be ok... or alternatively, perhaps the ThumbnailFile should be built with knowledge of the options it was generated with, allowing for a filter with no required options.

@sorl
Copy link

sorl commented Nov 30, 2010

I just want to comment on my design decision to make it a filter. First of all I think im.margin is ugly, is that really an explicit attribute for an image abstraction? Having the image abstraction to know of its requested size as well as the actual size also leads to some ugly coupling. As a filter you get one downside if you must and that is to have to set the size for the bounding box. The pros are:

  1. Its doesn't clutter the image abstraction.
  2. The filter is not tied to a thumbnail, it could be any image.
  3. You can have more margin than just the minumum.

@SmileyChris
Copy link
Owner

Yep, i'm not sold on im.margin, but I think giving the ThumbnailFile information so that you could optionally use the filter without an argument would be a fair enough use-case.

Thanks for your comments, sorl.

@hvdklauw
Copy link
Author

hvdklauw commented Dec 1, 2010

I can certainly live with that solution. Less duplication = more power to us

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants