Fix EZP-24535: Avoid empty width/height and allow to force them #1300
Conversation
Plopix
commented
Jun 11, 2015
Plopix
commented
- Avoid the width="" and the height=""
- Allow to force a height and a width manually (useful for handling the different screen densities)
| @@ -410,6 +410,7 @@ | |||
| <figure {{ block( 'field_attributes' ) }}> | |||
| {% set imageAlias = ez_image_alias( field, versionInfo, parameters.alias|default( 'original' ) ) %} | |||
| <img src="{% if imageAlias %}{{ asset( imageAlias.uri ) }}{% else %}//:0{% endif %}"{% if imageAlias.width is defined %} width="{{ imageAlias.width }}"{% endif %}{% if imageAlias.height is defined %} height="{{ imageAlias.height }}"{% endif %} alt="{{ field.value.alternativeText }}" /> | |||
| <img src="{% if imageAlias %}{{ asset( imageAlias.uri ) }}{% else %}//:0{% endif %}"{% if parameters.width is defined %} width="{{ parameters.width }}" {% endif %}{% if parameters.height is defined %} height="{{ parameters.height }}" {% endif %}{% if parameters.width is not defined and imageAlias.width is defined and imageAlias.width is not null %} width="{{ imageAlias.width }}"{% endif %}{% if parameters.height is not defined and imageAlias.height is defined and imageAlias.height is not null %} height="{{ imageAlias.height }}"{% endif %} alt="{{ field.value.alternativeText }}" /> | |||
There was a problem hiding this comment.
might be better to set a height and width variable before this line in own set, picking parameter first if set, then alias if not null, and last fall back being null, hence simplifying the logic on this line and improving readability overall.
and besides I guess you meant to remove the original line? :)
There was a problem hiding this comment.
OH! Yes.. I'll do that tonight ;)
And ok for the set statement.
There was a problem hiding this comment.
A {spaceless} block might make this more readable, maybe.
|
Ok guys I think it's better @andrerom it did create a var for the src too. |
| {% set src = imageAlias ? asset( imageAlias.uri ) : "//:0" %} | ||
| {% set width = parameters.width is defined ? parameters.width : imageAlias.width %} | ||
| {% set height = parameters.height is defined ? parameters.height : imageAlias.height %} | ||
| <img src="{{ src }}"{% if width is not null %} width="{{ width }}"{% endif %}{% if height is not null %} height="{{ height }}"{% endif %} alt="{{ field.value.alternativeText }}" /> |
There was a problem hiding this comment.
{% if width %} might suffice
There was a problem hiding this comment.
I think he did this on purpose, he is tackling several issues here actually. So should probably update description to clarify that.
@Plopix code is not checking imageAlias.width is defined anymore, guess it is always defined but might be null or?
There was a problem hiding this comment.
@lolautruche, yes you're right according to the twig documentation I agree it's not required.
I can remove the "is not null", I wanted to enforce that as you asked for readability and because I thought you would have asked for it ;)
@andrerom Yes it's always defined. That's what I figured out and that's why we have today width="" height="" in the rendered source. It's normal as the property is always defined.
Then my description was (for me) complete, we avoid the empty width and we allow the developer to force the width/height with parameters.
Let me know if you want me to remove the "is not null"
There was a problem hiding this comment.
Let me know if you want me to remove the "is not null"
you can do that, no need to be so explicit in templates unless you have to :) 👍
|
+1, bon :) |
|
+1 looks good |
|
Since it is core: yes |
|
ping @Plopix |
- Avoid the width="" and the height="" - Allow to force a height and a width manually (useful for handling the different screen densities)
Plopix
changed the title
|
Done @yannickroger. Hope I did everything right ;-) |
|
@Plopix Thanks |
Fix EZP-24535: Avoid empty width/height and allow to force them
|
Damn I just noticed that the comment above (line 404) has not been updated… |
|
@Plopix Do you have time to open a PR fixing the comment or should I do it quickly ? |
|
Check out #1320 |
