I think in the layout files we prefer or and and over || and &&.
And wouldn't it make more sense to do that in the JLayout file? Then it would be fixed in all instances where that is used.

Well, Bakual is right. May be this more correct - http://pastebin.com/bjJrJ9KS in /layouts/joomla/content/icons.php

Thomas, I think you are right. I will move the logic to the layout file, that makes sense.

Best regards,
Dmitry

----- Reply message -----
From: "Thomas Hunziker" notifications@github.com
To: "joomla/joomla-cms" joomla-cms@noreply.github.com
Subject: [joomla-cms] [#33945] Fix an empty icons div in single article view (#3897)
Date: Sun, Jul 13, 2014 22:10
I think in the layout files we prefer or and and over || and &&.

And wouldn't it make more sense to do that in the JLayout file? Then it would be fixed in all instances where that is used.

—
Reply to this email directly or view it on GitHub.

Shouldnt the layout have less logic not more

It already uses that conditional already anyway. We probably can just move it a bit :)

Having a good sleep today after the great football match I starting to think that moving this logic to the layout is not such a great idea as it sounds. When we have this logic in the main layout we control the call of JLayout and inclusion of layout file. This allow us not always to include this layout.

I think in the layout files we prefer or and and over || and &&.

May be, but currently default.php contains only || and &&, so I am using them for consistency.

Thanks. That's exactly my point. Wherever possible you shouldn't be overriding the logic just the format

I'm fine either way. It was just a thought
@brianteeman The logic is already there anyway if you look at https://github.com/joomla/joomla-cms/blob/staging/layouts/joomla/content/icons.php#L19.

I think @Bakual is right. We should adjust it in the source, not in the template files. If we decide to use this solution instead, every template file should be adjusted (e.g. featured articles), not only the article view.

As @Bakual already said, the logic is already there, we just have to move the div container a little bit!

Yes the logic is there, but it does not control the inclusion of the layout file. Currently we always include it, but do we need to?

@b2z No, so your suggestion is also okay. But the PR can only be accepted if you edit every view of the component.

But the PR can only be accepted if you edit every view of the component.

Yeah but before I will do it I need to know - should I ? :) Everyone is ok with it?

I'm fine either way. Feel free to adjust the other views as well.

Fixed also "Article Category Blog" and "Featured Articles".

@b2z Thanks, looks good now!

Waiting for testers now. Thanks.

@test: works fine for me and resolves the bug

@test: tested successfully - can be merged @Bakual

@b2z Thank you for your contribution!

Hooray!

I think in the layout files we prefer or and and over || and &&.

Just for information as it bites hardly: "and" and "or" don't have the same precedence/priority as "&&" and "||" over other operators. So the last 2 should be much preferred everywhere.

The difference plays mainly a role when you mix them or use it in an assignment, which would be not the case here. (See http://php.net/manual/en/language.operators.precedence.php)
I looked over the coding standard to see if I can find it but had no luck. So either it's not a valid rule anymore or it never was and I mistakenly believed it to be or I just am blind

Anyway, doesn't matter much.

My remark was "just for information", not a bug report :)

This different precedence depending on the operator form used a quite wierd behavior of PHP.

Thanks for the link. It shows that wierd different priority (and i got bitten by that some years ago).