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

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