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

[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 Expand

<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 Expand

<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.

[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]

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.

[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

Code Select Expand


$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.

[Antechinus]

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

Code Select Expand

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]

Yes, but the actual data loading is handled by the Sources...

[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 Expand

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:

Code Select Expand

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 ?

[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...

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:

Code Select Expand

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:

Code Select Expand

<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

Code Select Expand


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

Code Select Expand


$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>';
}


[Antechinus]

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

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:

Code Select Expand

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:

Code Select Expand

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:

Code Select Expand

[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:

Code Select Expand

.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"

[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.

Code Select Expand

.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:

Code Select Expand

.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.

[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.

[Antechinus]

Alrighty. Giving this a bit more thought.

I can see why Teh Ranty One thinks this general approach is stupid. I don't understand the entire process in great detail, but in conceptual terms it's clear enough. Sources already hauls all the required crap out of the db. It just doesn't output all of it to the template in a format that is required for this particular use case. Trying to get that from the template means going back to the db, multiple times, to get stuff that has already been fetched from the db. This is roughly as intelligent as a cat trying to bury its droppings on a concrete floor.

TRO mentioned hooks and voodoo, by which I assume using hooks to create a nice little chunk in Sources, which can then be used by the template in much the same way that Elk does their board avs. This should, I think, be able to be made clean and bullet proof for a custom theme. It's the same general principle as buidling in support for any other mod.

In rough terms, you'd write a mod that hooks in a function to create ['handy_chunk'] form all sorts of horrible gobbledegook that is already there. Then templating could be done like:

Code Select Expand


if($do_not['wanna_see_avs_anyway'] || $no_can_do['handy_chunk']) {
    <td class="info">
}
else
{
    <td class="info mit_extra_shiz">';
    // Extra shiz goes here.
}

Then you can set up your CSS to deal with either case. If someone just wants the basic theme without board avs, they can have it. If someone wants board avs, just install one tiny mod that won't break anything. If they want avs but haven't installed the required mod, all that happens is they get told to install the mod, and theme works normally without avs in the meantime.

This could be very useful, because it allows any themer to code any theme and make sure it all works. All they have to do is call ['handy_chunk'] if it exists. Markup and CSS can be done anyway way they want it. No admin inputs, so very simple code, and beginner admins won't be able to mess things up big time unless they learn to play with CSS.  In which case they are now coders, so it's on them. 

I should write this little beastie. I need to learn about hooks anyway.

[Arantor]

Oh, the hook voodoo I had in mind is eldritch level since themes can't declare hook calls off the bat.

But you should be able to register a hook in the theme init, to hang off the board index and message index "where they define the buttons" and you should be able to trawl $context at that point for the member ids, to make a single call to loadMemberData to get avatar and group data, reinject that into $context and then display it however.

[Antechinus]

Hmm. I drew up a short list of what it would have to do:

1/ Preferably use hooks rather than Sources edits.

2/ Should be called at the most basic level possible.

3/ Only outputs required are group_id, as an integer, and member avatar href.

4/ Special case of group_id =3 will require a comparison.

   General format:
   a/ fetch board moderators array
   b/ check against member's user id
   c/ if a match => assign value of 3 to group_id output

5/ Output would need to be accessible to:

   a/ index.template.php - for avatars in theme header (Note: this can actually be done just with $user_info)
   b/ BoardIndex.template.php - for the obvious
   c/ MessageIndex.template.php - ditto
   d/ Recent.template.php - to match MessageIndex
   e/ Display.template.php - for the obvious
   f/ Memberlist.template.php - for custom presentation
   g/ Who.template.php - for custom presentation

The theme header is only calling the av of any user who is logged in, so just declaring $user_info as one of the globals for function template_body_above() does the trick, and may be as good as any other option (open to advice on that).

It's all the other ones that are trickier. The Mutant Curve theme uses basically default (very minor edits) tabular markup for the memberlist and buddies list, but re-styled to present like my old memberlist mod and be responsive. So they call avatars too, making a total of seven templates that require access to this stuff.

All ['handy_chunk'] has to do is output the correct group_id as an integer, but the case of board moderators will require special handling. That will need to check user id against the array of moderators for that board, and force an id value of 3 if user id matches one of the moderator id's. Without that check the moderator group will always be ignored, and the code will fall back to post count group.

So is this still the sort of thing that could be comfortably handled by a theme hook up in init? Or would it be saner as a small mod to create the required chunk in Sources?

[Antechinus]

Oh hang on a second. When you say "...themes can't declare hook calls off the bat. But you should be able to register a hook in the theme init..." you mean yes, would have to set the hook up in Sources first. I got a bit mixed up with the "themes can't" and "bung it in init".

So write standard mod, with hooks, to create ['handy_chunk']. Then register hook in init. Then call the thing where you need it. Which can be in any template, wherever handy. Have I got it now?

[Arantor]

No, you really haven't got what I was getting at.

So mods have install.php options to add hooks; themes don't. So you have to do something else.

Your theme init function can call register_integration_hook() to register what to call when you want to call it; the rest of the functions you call can live in index.template.php just fine (or somewhere else, doesn't matter) and be included as needed that way. You just make sure that unlike normally, you pass in a third parameter of false to register_integration_function so that it isn't saved to modSettings since you're adding the hook that page and not more broadly.

You primarily need to register the functions somewhere they will be called and where the profile data is already in $context - so for board index, message index, display, this is the hook at the end that the buttons can be edited with. The others, off hand I don't remember what hooks exist, but failing that you can always add a hook that hooks onto the thing in index.php where actions are declared (for any of the action= links), grab whatever value is used, store it somewhere, replace it with your own function so your action is called every time instead, then you just look up what the original value was.

For example SimpleDesk does this to hook into unread replies, a thing that at the time had no hook. It looks up what the normal call for action=unread would be, stores it, replaces the definition with itself, then it calls what would normally be called, and just adds itself after before returning.

It's ugly AF but it works.

[Antechinus]

No, you really haven't got what I was getting at.

This does not surprise me. 

The others, off hand I don't remember what hooks exist, but failing that you can always add a hook that hooks onto the thing in index.php where actions are declared (for any of the action= links), grab whatever value is used, store it somewhere, replace it with your own function so your action is called every time instead, then you just look up what the original value was.

I'd expect Recent.template.php would have a hook for its buttons, since it's mostly a copy of MessageIndex.template.php. That should be ok. The other bit (action=recent) is more or less Display.template.php but doesn't have any buttonlist, so I might need to throw one in there. Will go look.

It's ugly AF but it works.

Sounds like my kind of code. I'll try setting up a basic "Woof!" implementation and see how I go.
(I find making templates randomly go "Woof!" is more fun than "Hello world". Also saves on typing.)

[Antechinus]

Meh. There seems to be a deplorable paucity of default hooks.
The only instances of call_integration_hook in Mutant Curve's templates (or default's) are these ones:

Code (Display.template.php ) Select Expand

call_integration_hook('integrate_display_buttons', array(&$normal_buttons));
call_integration_hook('integrate_mod_buttons', array(&$mod_buttons));

Code (MessageIndex.template.php) Select Expand

call_integration_hook('integrate_messageindex_buttons', array(&$normal_buttons));

Looks like I'll have to get inventive.

[Antechinus]

Have been doing more digging and pondering. Turns out I want to apply this to 9 templates.

a/ index.template.php

   b/ BoardIndex.template.php
   c/ MessageIndex.template.php
   d/ Recent.template.php

   e/ Display.template.php
   f/ PersonalMessage.template.php

   g/ Memberlist.template.php
   h/ Profile.template.ph
   i/ Who.template.php

index.template.php doesn't need any special handling - it can be done just with $user_info. Display and PersonalMessage don't need any special handling - they already call avs, and have group_id available. Memberlist, Profile and Who don't need any special handling either - they don't call avs by default, but turn out to have them available anyway, and they have group_id available too.

So that leaves only Board, Message and Recent as problems. Board and Recent have no call_integration_hook anywhere. Message has call_integration_hook before the topic table, but not for the child boards (if there are any).

TBH I can't help thinking the most sensible way of doing this is to add avatar href and group_id to the main db queries in Subs-Board Index, MessageIndex.php and Recent.php. Then they can just be chucked into the member arrays there, and Bob's your uncle. But I do realise there will be some concerns about hacking the main db queries, even though it's not going to be at all difficult. It'd certainly be the least convoluted code once the new queries were in place.

ETA: Also, I'm not sure how paranoid about performance it's necessary to get. In theory more performant is always good, but OTOH calling more stuff is always bad anyway. I'm planning on setting these up so they can all be knocked out by the default admin setting ('id' => 'show_user_images',) so even if they do start loading the server the admin can just flick a switch and solve the problem.

[Antechinus]

Just thinking about this:

...as far as simplifying this part

Code Select Expand


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

Code Select Expand


$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>';
}


Not so sure it's a good idea. It takes up fewer lines in the template, but under some circumstances it means more processing. Take a situation where a member has an avatar and no primary group. Setting it up this way means processing for post_group unnecessarily, before going and fetching the avatar anyway, and you're still calling $memberContext just as many times. It doesn't really save anything in terms of performance.

Having it all in bracketed chunks looks longer, but none of the stuff inside the brackets is processed unless the if condition is a match. In practice, only the content of one pair of brackets will ever be processed, so the repetition of some content isn't really an issue AFAICT.

[SychO]

umm, there really is almost no performance difference there, calling $memberContext has no effect because it has already been filled, so it doesn't hurt at all.

but if you really want to go THAT far into optimization then ok ¯\_(ツ)_/¯

Simplifying that in my perspective and because it really doesn't make any it less performant, amounts to not repeating the same block of HTML code multiple times because of one variable.

EDIT: Also about using hooks in the theme, I actually did that in my potato 2.1 theme, I prefer that a lot more to having your theme relying on a mod, and it works very nicely. (except for where template_init isn't called, like popup content)

[Antechinus]

I'm not really getting how this is supposed to work with hooks. The docs are a bit lacking, and looking at the code in mods that use them for this sort of thing (Like Simple Desk) means far too much irrelevant stuff to dig through to find the bits I want, and even then they'll be doing something different to what I want to do. OTOH, adding an extra db query and an extra line to the array is something my brain easily gets around.

But sure, I'll take a look at how you did it in Potato. That might make sense to me.

[Bloc]

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.

I would say.

Round up the member ids and query the database once in message/boardindex/other and done. Presently in my Zine theme if you are curious.

[SychO]

Yup, that's what I did.

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

Code Select Expand


$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.

[Antechinus]

I would say.

Round up the member ids and query the database once in message/boardindex/other and done. Presently in my Zine theme if you are curious.

Aha. That makes sense. I like that. Nice and clean, and won't get tangled up with other conditionals in the relevant templates.

I did notice though that you've left the dreaded span class="ie6_header floatleft" all over the place. Them's is evil. I only invented those when I was possessed by Satan. Exorcising them would be a good idea.

Also, funny thing about the HTML5 header, main and footer tags: they are only visible to screen readers if they are direct children of the body tag. Put them inside anything else, and screen readers skip straight over them, so there's no semantic benefit at all. See: https://webaim.org/techniques/semanticstructure/#regions

We recommend every page have one <main> element (or <div role="main">). Most pages also have only one <header> and <footer>. <header>, <main>, and <footer> must be direct children of <body> to be exposed to screen readers. They cannot be nested within other container elements.

[Antechinus]

Code Select Expand

$request = $smcFunc['db_query']('','
SELECT mem.id_member,
IFNULL(a.id_attach, 0) AS id_attach, a.filename, a.attachment_type, mem.avatar AS avatar
FROM {db_prefix}members AS mem
LEFT JOIN {db_prefix}attachments AS a ON (a.id_member = mem.id_member)
WHERE mem.id_member IN (' . $i . ')'
);}

Alrighty then. El Blocco's code makes sense to me (draw from that what you will) but naturally leads to questions.

1/ Since it includes a db query in index.template.php (see code box) is this going to make the dreaded Purple Peeps have conniptions?

2/ Is using a specific db query, that only goes looking for the bits you really need, better or worse for performance than the alternative of calling loadMemberData and loadMemberContext? The latter two obviously haul up more stuff that isn't needed, but does this make much difference in practice?

3/ Are there any security implications from having a db query in the template?

[Bloc]

1) Most likely.

2) Not sure..but adding it as a (from the theme itself will require starting the process at one point) hook  will make it execute in all themes. As a mod its probably a little better since the changes will be just in the actal loadMember calls(if you change the Source file that is, not adding just a hook), but then again you have to install that mod. Doing it in the theme will only execute the call in that theme(albeit adding to the total db calls) and when using the exact same procedure(the code is after all lifted from Sources and trimmed down) the fucntion should not be any more hazardous than a mod.

3) If you have setup the server correctly none should have access to neither Sources or Themes folder, apart from the script. So no, IMO its not. Its more of an problem with dividing between whats core code(db calls essentially) and whats just templating(ideally, only HTML). BUT in SMF there is no abstraction layer within a theme, you can use pure PHP in any way you want there. In that regard you even build a portal with the theme if you like(...) but generally I limit things to more logic within what the theme receives of data from core files, and/or a single db call for stuff like avatars.

That is IMHO always been one of the perks of SMF themeing..but opinions vary.