Avatars on board and message index: pick holes in this implementation

Started by Antechinus, September 08, 2020, 05:24:24 AM

Previous topic - Next topic

Antechinus

WIP code. Original idea for coding nicked from SychO's Nightbreeze theme, but it has been extended to give fallback "avatars" in member group colours for members that don't have an image avatar. It also has a conditional to only display the fallbacks if someone has selected "Do not show other users' avatars" in their profile settings. It all works, and works without generating any errors.

Will get some more tweaking anyway, but I'm currently interested in ways in which this code might be able to be made cleaner and/or more performant. Happy to attach complete templates if that will help. Pick holes in it. :)

Code (BoardIndex.template.php) Select
<td class="info">';

$last_poster = $board['last_post']['member']['id'];
if($last_poster!=0)
{
// Get The Avatar the easy Way
loadMemberData($last_poster);
loadMemberContext($last_poster);

if(empty($options['show_no_avatars']) && !empty($memberContext[$last_poster]['avatar']['image'])) {
$board_avatar = '<img class="avatar" src="'. $memberContext[$last_poster]['avatar']['href'] . '" style="width: 48px;" alt="" />';
}

elseif (!empty($memberContext[$last_poster]['group'])) {
$board_avatar = '<span class="no_avatar '. $memberContext[$last_poster]['group']. '">'. (substr($board['last_post']['member']['name'], 0, 1)). '</span>';
}

elseif ($memberContext[$last_poster]['post_group'] != '') {
$board_avatar = '<span class="no_avatar '. $memberContext[$last_poster]['post_group']. '">'. (substr($board['last_post']['member']['name'], 0, 1)). '</span>';
}

else {
$board_avatar = '<span class="no_avatar">'. (substr($board['last_post']['member']['name'], 0, 1)). '</span>';
}
}

echo '
<a href="', $board['last_post']['member']['href'], '" title="',$txt['profile_of'], ' ', $board['last_post']['member']['name'], '" class="board_avatar_anchor">
', $board_avatar, '
</a>
<h4><a class="subject" href="', $board['href'], '" name="b', $board['id'], '">', $board['name'], '</a></h4>';


Code (MessageIndex.template.php) Select
<td class="subject ', $alternate_class, '">';

$last_poster_topic = $topic['last_post']['member']['id'];
if($last_poster_topic!=0)
{
// Get The Avatar the easy Way
loadMemberData($last_poster_topic);
loadMemberContext($last_poster_topic);

if(empty($options['show_no_avatars']) && !empty($memberContext[$last_poster_topic]['avatar']['image'])) {
$topic_avatar = '<img class="avatar" src="'. $memberContext[$last_poster_topic]['avatar']['href'] . '" style="width: 48px;" alt="" />';
}

elseif (!empty($memberContext[$last_poster_topic]['group'])) {
$topic_avatar = '<span class="no_avatar '. $memberContext[$last_poster_topic]['group']. '">'. (substr($topic['last_post']['member']['name'], 0, 1)). '</span>';
}

elseif ($memberContext[$last_poster_topic]['post_group'] != '') {
$topic_avatar = '<span class="no_avatar '. $memberContext[$last_poster_topic]['post_group']. '">'. (substr($topic['last_post']['member']['name'], 0, 1)). '</span>';
}

else {
$topic_avatar = '<span class="no_avatar">'. (substr($topic['last_post']['member']['name'], 0, 1)). '</span>';
}
}

echo '
<a href="', $topic['last_post']['member']['href'], '" title="',$txt['profile_of'], ' ', $topic['last_post']['member']['name'], '"  class="topic_avatar_anchor">
', $topic_avatar, '
</a>


ETA: Oh yeah, please don't go suggesting 'orrible gnarly multiple option ternaries instead of nice brackety chunks unless said gnarly beats will result in a noticeable performance improvement. I really do prefer to keep formatting clear and obvious. ;)

SychO

looking back at this, calling loadMemberData every time is a bad idea 🤔

should instead grab all the last posts's poster ids and call the function once only.

Checkout My Themes:
-

Potato  •  Ackerman  •  SunRise  •  NightBreeze

Antechinus

Yes I had thought of dropping it all in a function. Apart from anything else it would make the template a bit tidier.

I was also thinking that giving admins the ability to turn it off could be handy for server loads sometimes.

Arantor

It's also stupidly expensive. Adding multiple database queries for every board or every topic is nuts-levels bad for performance.

SychO

Quote from: Arantor on September 08, 2020, 07:29:27 AM
It's also stupidly expensive. Adding multiple database queries for every board or every topic is nuts-levels bad for performance.

exactly, back when I wrote it I didn't know much about the smf codebase, I was just looking for a working solution.
Checkout My Themes:
-

Potato  •  Ackerman  •  SunRise  •  NightBreeze

Arantor

2.1 has the base support for doing this now.

It would also be possible to do the loading via hooks before the theme loads but that's voodoo.

Antechinus

Voodoo is fine, as long as it works, but I was thinking of this not requiring a mod as such. IOW, all in the templates, somehow.

SychO

This is what I'm gonna do with NightBreeze, (it's in the messageindex and it's actually the first poster in that theme and not the last)

adding this before the first loop


$poster_ids = array();
foreach ($context['topics'] as $topic)
{
if (!empty($topic['first_post']['member']['id']))
$poster_ids[] = $topic['first_post']['member']['id'];
}

loadMemberData($poster_ids);


And then I can remove the call to loadMemberData when looping through each item.
Checkout My Themes:
-

Potato  •  Ackerman  •  SunRise  •  NightBreeze

Antechinus

Just for comparison, I had a look at how Elk does it. This is from the Elk 1.1 GenericBoards.template.php:

if (!empty($board['last_post']['id']))
{
echo '
<p class="board_lastpost">';

if (!empty($board['last_post']['member']['avatar']))
echo '
<span class="board_avatar"><a href="', $board['last_post']['member']['href'], '"><img class="avatar" src="', $board['last_post']['member']['avatar']['href'], '" alt="" /></a></span>';
else
echo '
<span class="board_avatar"><a href="#"></a></span>';
echo '
', $board['last_post']['last_post_message'], '
</p>';
}


Which is pretty simple, but then I want to grab member groups as well, so that's an extra layer of complexity.

Arantor


Antechinus

TBH the other thing that bugs me about the code I have is that I can get it to work by membergroup name on the board and message index, but I can't figure out how to get it to haul up the group id number.

Code (MessageIndex.template.php) Select
elseif (!empty($memberContext[$last_poster_topic]['group'])) {
$topic_avatar = '<span class="no_avatar '. $memberContext[$last_poster_topic]['group']. '">


That outputs the group name, but group names are messy for CSS. ID number would be much cleaner and simpler. Calling id number in the upper section of  index.template.php is a piece of cake:

elseif ($user_info['groups']['0'] != '') {
echo ' <span class="no_avatar group_', $user_info['groups']['0'], '">', (substr($context['user']['name'], 0, 1)), '</span></a>';
}


That will return class = group_1 for admin, group_2 for global mods, etc. But I've tried everything I can think of, and can't get the id number on board index or message index. Which seems weird, because obviously it must be accessible somehow.

SychO

just do a print_r($memberContext[$last_poster_topic]); and look at its contents, also why are you trying to use group ids in classes ?
Checkout My Themes:
-

Potato  •  Ackerman  •  SunRise  •  NightBreeze

Antechinus

So the fallback avatars can be coloured according to member group. See attachment in this post.

As soon as I sorted calling the fallbacks with the first letter of the user name, Lurkalot piped up with "Do they come in group colours too?" so... :D

Anyway, I ran the printr and it returns the correct [group_id] => 1 for admin and [group_id] => 2 for global mods, etc. Which I would expect, since I already double checked the db to get that tag anyway. Problem is, even if I throw [group_id] in the template code it doesn't output anything. I've tried it every way I can think of, but no joy.

For example this:
elseif (!empty($memberContext[$last_poster]['group'])) {
$board_avatar = '<span class="no_avatar '. $memberContext[$board['last_post']['member']['group_id']]. '">'. (substr($board['last_post']['member']['name'], 0, 1)). '</span>';
}


Returns this:
<a href="http://127.0.0.1/Vanilla_20x/index.php?action=profile;u=8" title="View the profile of Fred the Ferret" class="board_avatar_anchor">
<span class="no_avatar ">F</span>
</a>

SychO

Oh I see, I like that, you should be able to get the group id with $memberContext[$last_poster_topic]['group_id'], definitely much better than using group names.

and as far as simplifying this part


if(empty($options['show_no_avatars']) && !empty($memberContext[$last_poster_topic]['avatar']['image'])) {
$topic_avatar = '<img class="avatar" src="'. $memberContext[$last_poster_topic]['avatar']['href'] . '" style="width: 48px;" alt="" />';
}

elseif (!empty($memberContext[$last_poster_topic]['group'])) {
$topic_avatar = '<span class="no_avatar '. $memberContext[$last_poster_topic]['group']. '">'. (substr($topic['last_post']['member']['name'], 0, 1)). '</span>';
}

elseif ($memberContext[$last_poster_topic]['post_group'] != '') {
$topic_avatar = '<span class="no_avatar '. $memberContext[$last_poster_topic]['post_group']. '">'. (substr($topic['last_post']['member']['name'], 0, 1)). '</span>';
}

else {
$topic_avatar = '<span class="no_avatar">'. (substr($topic['last_post']['member']['name'], 0, 1)). '</span>';
}


I would do something like this


$class = '';

if (!empty($memberContext[$last_poster_topic]['group']))
$class = $memberContext[$last_poster_topic]['group'];
elseif (!empty($memberContext[$last_poster_topic]['post_group']))
$class = $memberContext[$last_poster_topic]['post_group'];

if (empty($options['show_no_avatars']) && !empty($memberContext[$last_poster_topic]['avatar']['image'])) {
$topic_avatar = '<img class="avatar" src="'. $memberContext[$last_poster_topic]['avatar']['href'] . '" style="width: 48px;" alt="" />';
}

else {
$topic_avatar = '<span class="no_avatar '. $class . '">'. (substr($topic['last_post']['member']['name'], 0, 1)). '</span>';
}
Checkout My Themes:
-

Potato  •  Ackerman  •  SunRise  •  NightBreeze

Antechinus

Well bugger me. That actually works.  :P I was sure I'd tried that before, but it must have been the only thing I didn't try. D'oh. :D

However, there is one small catch with it. It works for all groups except board moderators. Global mods are fine, but for board moderators it outputs their post group id rather than their primary group for that board.

IOW this will output post group id, even when viewing the message index of the board that member moderates:

elseif (!empty($memberContext[$last_poster_topic]['group'])) {
$topic_avatar = '<span class="no_avatar group_'. $memberContext[$last_poster_topic]['group_id']. '">'. (substr($topic['last_post']['member']['name'], 0, 1)). '</span>';
}

elseif ($memberContext[$last_poster_topic]['post_group'] != '') {
$topic_avatar = '<span class="no_avatar group_'. $memberContext[$last_poster_topic]['group_id']. '">'. (substr($topic['last_post']['member']['name'], 0, 1)). '</span>';
}


But this outputs the group name (Moderator) when viewing the message index of the board that member moderates:

elseif (!empty($memberContext[$last_poster_topic]['group'])) {
$topic_avatar = '<span class="no_avatar '. $memberContext[$last_poster_topic]['group']. '">'. (substr($topic['last_post']['member']['name'], 0, 1)). '</span>';
}

elseif ($memberContext[$last_poster_topic]['post_group'] != '') {
$topic_avatar = '<span class="no_avatar '. $memberContext[$last_poster_topic]['post_group']. '">'. (substr($topic['last_post']['member']['name'], 0, 1)). '</span>';
}


Why? No idea at the moment. :)
I ran a printr of it and I get this:
[group] => Moderator [group_color] => #80c800 [group_id] => 0 [post_group] => Newbie

So for some reason SMF does not regard group_id => 3 as being the primary group inside the relevant board, and falls back to the post count group.

Which I suppose there are a couple of workarounds for. One would be to handle it inside the CSS. You could enter the user id number and board id number, and use those with chained classes or whatever to apply the colour you want inside whatever board. Easy enough in principle, but likely to be a bit of a nuisance in practice. Example:

.board_6 .no_avatar[href*="u=9"] {background: #80c800;}

SychO

are you sure ? I tested locally and when the member has a primary group, the value of group_id is correctly the primary group's even when they are the moderator, it's just the group name that's changed to "Moderator"
Checkout My Themes:
-

Potato  •  Ackerman  •  SunRise  •  NightBreeze

Antechinus

That's what I mean. It won't pick up group_id => 3 inside a board. In this test case there is no primary group set, and because the test site has low activity the moderator account in question is still in the Newbie post count group. The code only picks up group_id => 0 (ie: default Newbie group) regardless of which board the account posts in.

Antechinus

Come to think of it, this could still be dealt with without extra CSS. You could simply add an admin input box to take the name of the moderator group in whatever language you were using. That string could then be used to check group name per board, and if there's a match with the moderator group then it assigns group_3 to the span class.

This is still something of a nuisance though. It may be best to just go with the CSS workaround.

ETA: This works for CSS.

.board_6 .topic_avatar_anchor[href*="u=9"] .group_0 {
background: #80c800;
}


Where .board_6 was already assigned to the body tag anyway, and u=9 is the relevant account id. So using this setup I can get group_id output to the span class for every group except 3. Then if anyone really wants to they can set up a bunch of CSS overrides for their moderator/board combinations. Not perfect, but pretty good. You'd only have to copy/paste an example and edit two numbers:

.board_6 .topic_avatar_anchor[href*="u=9"] .group_0,
.board_9 .topic_avatar_anchor[href*="u=22"] .group_0,
.board_11 .topic_avatar_anchor[href*="u=32"] .group_0,
.board_12 .topic_avatar_anchor[href*="u=19"] .group_0,
.board_4 .topic_avatar_anchor[href*="u=9"] .group_0 {
background: #80c800;
}

SychO

Oh I see, so you actually want the ID to be that of the Moderator group, there should be a simple condition that can be used, I haven't checked yet though.
Checkout My Themes:
-

Potato  •  Ackerman  •  SunRise  •  NightBreeze

Antechinus

It's probably possible, but I haven't been able to think of one yet.

ETA: Well, apart from checking against the text string for the actual name in any language, but that feels a bit messy.

ETA2: Come to think of it, I should ditch the spans anyway. An anchor set to display: block; is perfectly adequate if I edit the PHP to suit. The .no_avatar and .group_0 classes can go on the anchor if there is no image available. Might even be able to get away with just the group_id (should work). Will mess around with it a bit more. Simpler markup and CSS is usually a good thing. :)

Advertisement: