Twenty Thirteen: Comment author's name looks out of place when avatar display is turned off in discussion settings.

[SriniG]

Comment author's name looks out of place when 'Avatar Display' is unchecked in Settings->Discussion. The layout looks as if something is missing, this shouldn't be.

[cdog]

attachment:23558.diff​ uses the show_avatars option to check wheter to display avatar images or not. See attachment:23558.png​ for preview.

[obenland]

I like the approach of using a comment class. It would be cleaner though, to use either a 'comment_class' callback or the 'body_class' callback we already have in functions.php.
We should be able to just switch to the mobile-style display of comments then.

[cdog]

Replying to obenland:

I like the approach of using a comment class. It would be cleaner though, to use either a 'comment_class' callback or the 'body_class' callback we already have in functions.php.

Thanks for your feedback. I've updated the patch to use the comment_class filter hook. Please check: attachment:23558.2.diff​.

[SergeyBiryukov]

Fixed typo in line 388

[SriniG]

@cdog: I don't think we need to conditionally check get_option( 'show_avatar' ) in order to echo get_avatar(). It seems the function get_avatar() somehow handles the condition by itself. If you view source after disabling avatars, you'll find there is no empty img tag or anything like that.

I am also not in favour of introducing a middot, or changing anything inside the twentythirteen_comment function, unless absolutely necessary. All changes can be made via CSS making use of the no-avatar class added through the comment_class filter hook.

[cdog]

Replying to SriniG:

@cdog: I don't think we need to conditionally check get_option( 'show_avatar' ) in order to echo get_avatar(). It seems the function get_avatar() somehow handles the condition by itself. If you view source after disabling avatars, you'll find there is no empty img tag or anything like that.

Agreed. I've updated the patch to use only the get_avatar() function. Please check: attachment:23558.4.diff​.

I am also not in favour of introducing a middot, or changing anything inside the twentythirteen_comment function, unless absolutely necessary. All changes can be made via CSS making use of the no-avatar class added through the comment_class filter hook.

I'm not sure if we can get the same result as in attachment:23558.png​ (excluding the middot) only using CSS in a clean way.

[SriniG]

@cdog: Please check attachment:23558.5.diff​. Of course this just switches to a styling similar to the mobile view as per comment 3 by obenland. If you want to add a middot, you can have that too, you can use the after pseudo selector, like .no-avatars .comment-author:after { content: '\00b7'; }. Also modify the margins and floats if neeeded.

[cdog]

Replying to SriniG:

@cdog: Please check attachment:23558.5.diff​. Of course this just switches to a styling similar to the mobile view as per comment 3 by obenland.

It's clean and simple. I like it. :)

Personally I prefer the comment author link and comment time on the same line (similar to DISQUS). Please check: attachment:23558.6.diff​. What do you think about this approach?

[obenland]

Let's go with 23558.5.diff​. Joen approved the mobile styles already and I don't really see a reason to introduce a new styling here.

We might even be able to get rid of the .no-avatars .comment-author .fn selector (same goes for the equivalent in the mobile media query) once #23514 makes it in.

[MikeHansenMe]

Replying to obenland:

Let's go with 23558.5.diff​. Joen approved the mobile styles already and I don't really see a reason to introduce a new styling here.

I agree, when I tested .6 it removes the name below the avatar when they are enabled.

.5 does not seem to have any side effects.

[cdog]

Replying to MikeHansenMe:

I agree, when I tested .6 it removes the name below the avatar when they are enabled.

It was only a suggestion and that behavior was intended.

.5 does not seem to have any side effects.

Please check: attachment:23558.5.1.diff​. This patch is the same as attachment:23558.5.diff​ with the following changes/additions:

• replaced no-avatars with no-avatar for the comment_class callback as a comment contains only one avatar - add this if it's really necessary;
• added the no-avatars class for the body_class callback as it could be useful for pages containing avatars in other places than comments (e.g. author pages) - this should be sufficient for most cases.

Note: Again, this is just a suggestion. Please feel free to choose the most suitable solution.

[lancewillett]

Replying to obenland:

Let's go with 23558.5.diff​. Joen approved the mobile styles already and I don't really see a reason to introduce a new styling here.

Noted.

We might even be able to get rid of the .no-avatars .comment-author .fn selector (same goes for the equivalent in the mobile media query) once #23514 makes it in.

I'd like to wait for #23514, then, before we push this one. Then we can update the patch here with the simpler selectors.

[lancewillett]

In 23498:

Twenty Thirteen: style comments correctly when show_avatars option is turned off in Discussion settings. Props SriniG, fixes #23558.