django-cms / djangocms-frontend

django CMS frontend is a plugin bundle for django CMS providing several components for the frontend, currently implemented with the popular Bootstrap 5 framework.
Other
43 stars 20 forks source link

Wrong condition in `ImageMixin.get_size()` #117

Closed adrien-delhorme closed 1 year ago

adrien-delhorme commented 1 year ago

I wondered why my cropping format was not respected.

I found that the following condition is met when the width and height are not None OR when (width and height are None AND self.picture is None): https://github.com/django-cms/djangocms-frontend/blob/61b5b766fb0256a9e8b426f4d68b4891ccc0f815/djangocms_frontend/contrib/image/models.py#L44-L47

So when width and height are set, we override them with 640, 640 / PICTURE_RATIO...

I think we can fully remove the else condition. If this.picture is None, we do not need to worry about its size.

fsbraun commented 1 year ago

I just created a quick fix (#118). Really, this should be covered by tests and might need a decent rework... Also to include the ideas of https://github.com/django-cms/djangocms-picture/pull/114

@adrien-delhorme Can you test if it works for you?

fsbraun commented 1 year ago

Fixed by #118