Hey, everybody!
Last month, I did the patch that added the "(OP)" thing to the forum (topic is
here), this month I thought I'd try my luck a second time and see if I can get theymos to merge a slightly bigger patch (fingers crossed).
So, ajaxtempest was the member that suggested the OP identification thing, and later in that same thread they had a second idea:
I have another idea!. Have a down arrow next to a person reply which states"jump to next post of the user". An up arrow can also be used to see the users previous post in the same thread.
The idea was merited (by vapourminer and LoyceV) and a few members made positive comments about it (LoyceV, Mr.right85, FatFork and nakamura12). After ajaxtempest noticed that their first idea actually got implemented, they came over to my thread to thank me (appreciated), cheekily point out that I hadn't properly listened to them (not so appreciated) and then reminded me about their second idea; unfortunately I didn't have good news for them:
also i had suggested another feature where you would click a down or up arrow in the post message to jump to next post of the specific user in that thread.
Yep, I've seen that one. I've only considered it superficially but knowing what I now know about SMF (which is not all that much, I'm still learning the codebase), implementing this feature would be technically quite "ugly" as it'll involve running a new SQL query for each post, each time it's rendered. I'm not sure if this suggestion is useful enough to warrant the additional DB load it'll cause. I guess if there are more than a few long-time members that chime in with support for this idea, I'll look into it more deeply and see if I can find a way to implement it elegantly.
After that, nobody else brought it up again, so I didn't think much more about the problem, but as time went on I was surprised to find that I kept wishing that the forum had that feature! Especially when you disagree with someone, it's only really possible to understand their position once you've read all of their posts in a given thread (and even then, maybe not). Trying to glean too much information from a single post is a recipe for miscommunication, but it's a real hassle at the moment (depending on the length of the thread) to go through each of someone's posts in a given topic. It's also very useful for skipping around megathreads (like WO) or just for when you're wondering if a specific poster had anything more to say in a specific thread.
So, once I saw the value in the feature, I started to think seriously about how to implement it without running into the problem that I had told ajaxtempest about: Right now, whenever a page of posts is returned by the server a small number of SQL queries are involved and there are no SQL queries that are issued
per-post; that's good design and I didn't want to add code to SMF that would change that. After thinking about the problem for a bit, I realized that the database lookups don't actually have to be done during the page render, but can be postponed until the next/previous button is actually clicked on by hiding the SQL query behind an HTTP redirect. That way, this new feature adds zero additional database load when it's not being used.
Unfortunately, this "deferred" approach has a small usability flaw, and that is that because the information about the next/previous post is unavailable at render time, the buttons are not "smart", and they appear whether there is a post to skip to or not. In cases where you're already on the first post (no previous post to skip to) or already on the last post (no next post to skip to), clicking on the respective button will simply take you to where you already are. I don't think this is a big deal, and like I said above, I don't think that the additional database load it would take to avoid it can be justified, so my position is that I'd much rather have this feature with this small flaw than not have it at all. If it turns out to be a deal-breaker and theymos thinks that additional database load
can be justified, then I'll rework the patch accordingly.
Anyway, now that I've bored you all to tears, here's what the final result looks like (buttons are next to "Report to moderator"):
If you hover over the buttons a tooltip appears that explains what they do, here's what each of them says:
Last time I did a patch, ajaxtempest wasn't entirely happy with my work:
Thanks for implementing my suggestion. However by making it in BOLD it would have been more easier when skimming through the thread just to see only OP posts.
So, this time around, in anticipation of them saying: "Thanks, but I wanted UP and DOWN arrows.", I'll just say: If you want things just so, then make them just so.
[I added some context to this sentiment
here]
I realize that people are likely to have many different opinions about where the buttons should go and what they should look like, and I experimented with lots of alternatives. The buttons are represented with Unicode symbols, and I've been careful to base my selection on characters that are likely to be widely available. Right now, the buttons are represented by ◁ and ▷ (U+25C1 and U+25B7). Another good choice might be the solid version of the same arrows: ◀ and ▶ (U+25C0 and U+25B6). Regarding positioning, I think the bottom-right is a good place for them to go (it's also quite a natural location for them to be in terms of source code, right before the signature gets rendered). I could also see them working in the top-right and maybe the bottom-left.
It's common for people to suggest that any given thing should "stand out" more, but this line of thinking tends to lead to noisy UIs with too many things competing for your visual attention, so I think the understated approach works fine for a feature that's intended to be used only once in a while.
Luckily, the way this patch is structured, messing with the "presentation layer" (piece 3, below) doesn't affect any of the logic, so theymos can pick whatever arrows he likes, and place them wherever he feels they should go.
Okay, so what follows is the actual patch (in three pieces).
I had some text, explaining what each piece does, but it felt silly to include, because theymos knows what's up and will be able to make sense of it all easily.
Piece 1<?php
// This file (Sources/Skip.php) implements the action (?action=skip) that's used when jumping to the next/previous post by the same author in a given topic.
// There's not much to it; after checking its request parameters, it queries the database for the appropriate destination message and then redirects there.
// Written by: PowerGlove [2022/10/10, 2022/10/21]
// Note: I'm intentionally not making use of the $topic global, because I don't know SMF well enough yet to feel comfortable reasoning out SQL injections by having to consider more than the code in this file.
if (!defined('SMF')) die('Hacking attempt...');
function Skip() {
global $db_prefix;
if (isset($_GET['topic']) && isset($_GET['u']) && isset($_GET['msg']) && isset($_GET['prev_next'])) {
$param_topic = (int)$_GET['topic'];
$param_user = (int)$_GET['u'];
$param_message = (int)$_GET['msg'];
$param_prev_next = $_GET['prev_next'];
if ($param_topic >= 1 && $param_user >= 1 && $param_message >= 1 && ($param_prev_next == 'prev' || $param_prev_next == 'next')) {
$ascending = $param_prev_next == 'next';
$gt_lt = $ascending ? '>' : '<';
$direction = $ascending ? '' : 'DESC';
$request = db_query("SELECT ID_MSG FROM {$db_prefix}messages WHERE ID_TOPIC = $param_topic AND ID_MEMBER = $param_user AND ID_MSG $gt_lt $param_message ORDER BY ID_MSG $direction LIMIT 1", __FILE__, __LINE__);
$row = mysql_fetch_row($request);
$destination_message = mysql_num_rows($request) == 1 ? $row[0] : $param_message;
mysql_free_result($request);
redirectexit('topic=' . $param_topic . '.msg' . $destination_message . '#msg' . $destination_message);
} else fatal_error('Skip: bad parameter(s).', false);
} else fatal_error('Skip: missing parameter(s).', false);
}
?>
--- /var/www/baseline/index.php 2013-10-21 15:01:11.000000000 -0400
+++ /var/www/modified/index.php 2022-10-12 04:48:08.000000000 -0400
@@ -311,6 +311,7 @@
'sendtopic' => array('SendTopic.php', 'SendTopic'),
'serversettings' => array('ManageServer.php', 'ModifySettings'),
'serversettings2' => array('ManageServer.php', 'ModifySettings2'),
+ 'skip' => array('Skip.php', 'Skip'),
'smileys' => array('ManageSmileys.php', 'ManageSmileys'),
'smstats' => array('Stats.php', 'SMStats'),
'spellcheck' => array('Subs-Post.php', 'SpellCheck'),
--- /var/www/baseline/Themes/default/Display.template.php 2010-10-21 21:38:35.000000000 -0400
+++ /var/www/modified/Themes/default/Display.template.php 2022-10-21 16:13:11.000000000 -0400
@@ -489,16 +489,24 @@
elseif (!$context['user']['is_guest'])
echo '
<a href="', $scripturl, '?action=helpadmin;help=see_member_ip" onclick="return reqWin(this.href);" class="help">', $txt[511], '</a>';
// Otherwise, you see NOTHING!
else
echo '
', $txt[511];
+ // Show the skip-by-author controls?
+ // - The first condition (not a guest post) is because although it could be made to work, skipping between guest posts doesn't make much sense, and so the "skip" action expects that u > 0.
+ // - The second condition (not in ";all" mode) is to prevent user confusion because (out of sympathy for the server) the "skip" action doesn't collect and propagate ";all" mode.
+ if (!$message['member']['is_guest'] && !isset($_REQUEST['all']))
+ echo '
+ <a href="', $scripturl, '?action=skip;topic=', $context['current_topic'], ';u=', $message['member']['id'], ';msg=', $message['id'], ';prev_next=prev', '" title="Skip to the previous post in this topic by the same author (if any)">◁</a>
+ <a href="', $scripturl, '?action=skip;topic=', $context['current_topic'], ';u=', $message['member']['id'], ';msg=', $message['id'], ';prev_next=next', '" title="Skip to the next post in this topic by the same author (if any)">▷</a>';
+
echo '
</td>
</tr></table>';
// Show the member's signature?
if (!empty($message['member']['signature']) && empty($options['show_no_signatures']))
echo '
<hr width="100%" size="1" class="hrcolor" />
