pkg_comments.php: Make comment timestamps link to the comment
diff mbox

Message ID 20181016002658.10771-1-archlinux@thecybershadow.net
State New
Headers show

Commit Message

Vladimir Panteleev Oct. 16, 2018, 12:26 a.m. UTC
As of today, there is no easy way to obtain a link to a specific
comment on a package page.

Many implementations of forums and comment systems today seem to
follow a convention where a comment's timestamp is an unobtrusive link
to the comment itself. Some examples are:

- phpBB (e.g. bbs.archlinux.org)
- GitHub
- Disqus
- Discourse

This patch adopts this convention as well, by making the comment
timestamp an unobtrusive link to the comment itself.
---
 web/html/css/aurweb.css       |  2 +-
 web/template/pkg_comments.php | 12 +++++++-----
 2 files changed, 8 insertions(+), 6 deletions(-)

Comments

Eli Schwartz Oct. 16, 2018, 7:23 p.m. UTC | #1
On 10/15/18 8:26 PM, Vladimir Panteleev wrote:
> As of today, there is no easy way to obtain a link to a specific
> comment on a package page.
> 
> Many implementations of forums and comment systems today seem to
> follow a convention where a comment's timestamp is an unobtrusive link
> to the comment itself. Some examples are:
> 
> - phpBB (e.g. bbs.archlinux.org)
> - GitHub
> - Disqus
> - Discourse
> 
> This patch adopts this convention as well, by making the comment
> timestamp an unobtrusive link to the comment itself.
> ---
>  web/html/css/aurweb.css       |  2 +-
>  web/template/pkg_comments.php | 12 +++++++-----
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/web/html/css/aurweb.css b/web/html/css/aurweb.css
> index 593c9ae..d587dc6 100644
> --- a/web/html/css/aurweb.css
> +++ b/web/html/css/aurweb.css
> @@ -122,7 +122,7 @@
>  	top: 4px;
>  }
>  
> -.flagged a {
> +.flagged a, a.date {
>  	color: inherit;
>  }

This causes the link color to be the same black as the rest of the
"commented on" text. Question is, do we actually want that? It makes it
a bit hard to tell the links are there...
Johannes Löthberg Oct. 20, 2018, 1:05 p.m. UTC | #2
Excerpts from Vladimir Panteleev's message of October 16, 2018 2:26:
> As of today, there is no easy way to obtain a link to a specific
> comment on a package page.
> 
> Many implementations of forums and comment systems today seem to
> follow a convention where a comment's timestamp is an unobtrusive link
> to the comment itself. Some examples are:
> 
> - phpBB (e.g. bbs.archlinux.org)

(Our BBS is FluxBB, not phpBB.)

> - GitHub
> - Disqus
> - Discourse
> 
> This patch adopts this convention as well, by making the comment
> timestamp an unobtrusive link to the comment itself.
> 

Most of what makes those URLs useful is that they link to the actual 
post though, while this is just an anchor to the page you're currently 
on, which means that as soon as it ends up at the next page, the URL is 
broken, so I'm not sure how useful this is in practice?
Eli Schwartz Oct. 20, 2018, 11:20 p.m. UTC | #3
On 10/20/18 9:05 AM, Johannes Löthberg wrote:
> Excerpts from Vladimir Panteleev's message of October 16, 2018 2:26:
>> As of today, there is no easy way to obtain a link to a specific
>> comment on a package page.
>>
>> Many implementations of forums and comment systems today seem to
>> follow a convention where a comment's timestamp is an unobtrusive link
>> to the comment itself. Some examples are:
>>
>> - phpBB (e.g. bbs.archlinux.org)
> 
> (Our BBS is FluxBB, not phpBB.)
> 
>> - GitHub
>> - Disqus
>> - Discourse
>>
>> This patch adopts this convention as well, by making the comment
>> timestamp an unobtrusive link to the comment itself.
>>
> 
> Most of what makes those URLs useful is that they link to the actual 
> post though, while this is just an anchor to the page you're currently 
> on, which means that as soon as it ends up at the next page, the URL is 
> broken, so I'm not sure how useful this is in practice?

Admittedly, my usual use case is with ?PP=250. Not too long ago, I
simply defaulted to using the "show all" link.

I guess there is a difference between forum posts, which start on the
first page and add new posts to a new page, vs. the AUR which shows the
newest comments first. Is there any way to make truly stable links like
that?
Johannes Löthberg Oct. 21, 2018, 11:53 a.m. UTC | #4
Excerpts from Eli Schwartz's message of October 21, 2018 1:20:
> On 10/20/18 9:05 AM, Johannes Löthberg wrote:
>> Excerpts from Vladimir Panteleev's message of October 16, 2018 2:26:
>>> As of today, there is no easy way to obtain a link to a specific
>>> comment on a package page.
>>>
>>> Many implementations of forums and comment systems today seem to
>>> follow a convention where a comment's timestamp is an unobtrusive link
>>> to the comment itself. Some examples are:
>>>
>>> - phpBB (e.g. bbs.archlinux.org)
>> 
>> (Our BBS is FluxBB, not phpBB.)
>> 
>>> - GitHub
>>> - Disqus
>>> - Discourse
>>>
>>> This patch adopts this convention as well, by making the comment
>>> timestamp an unobtrusive link to the comment itself.
>>>
>> 
>> Most of what makes those URLs useful is that they link to the actual 
>> post though, while this is just an anchor to the page you're currently 
>> on, which means that as soon as it ends up at the next page, the URL is 
>> broken, so I'm not sure how useful this is in practice?
> 
> Admittedly, my usual use case is with ?PP=250. Not too long ago, I
> simply defaulted to using the "show all" link.
> 
> I guess there is a difference between forum posts, which start on the
> first page and add new posts to a new page, vs. the AUR which shows the
> newest comments first. Is there any way to make truly stable links like
> that?
> 

Same way that phpBB and FluxBB does it, you link to a comment-specific 
URL, not an anchor on the destination page.

Something like /comment/<comment_id> -> look up which package the 
comment is on -> count how many visible comments are before it for that 
package -> divide by per-page value -> redirect to that page with the 
anchor.
Vladimir Panteleev Oct. 23, 2018, 2:15 p.m. UTC | #5
On Sat, Oct 20, 2018 at 1:05 PM Johannes Löthberg <johannes@kyriasis.com>
wrote:

> (Our BBS is FluxBB, not phpBB.)
>

Oops, right.


> Most of what makes those URLs useful is that they link to the actual
> post though, while this is just an anchor to the page you're currently
> on, which means that as soon as it ends up at the next page, the URL is
> broken, so I'm not sure how useful this is in practice?
>

True. Though, as far as I've seen, the vast majority of AUR packages have
low enough comment activity that the link will remain functional for quite
some time. You can also get a permanent link by capturing the anchor from
the "?comments=all" page. I think making the timestamp's href point to a
truly permanent link would be a good improvement on top of this change.

Patch
diff mbox

diff --git a/web/html/css/aurweb.css b/web/html/css/aurweb.css
index 593c9ae..d587dc6 100644
--- a/web/html/css/aurweb.css
+++ b/web/html/css/aurweb.css
@@ -122,7 +122,7 @@ 
 	top: 4px;
 }
 
-.flagged a {
+.flagged a, a.date {
 	color: inherit;
 }
 
diff --git a/web/template/pkg_comments.php b/web/template/pkg_comments.php
index 3001a34..3bcf1a3 100644
--- a/web/template/pkg_comments.php
+++ b/web/template/pkg_comments.php
@@ -53,17 +53,19 @@  if ($comment_section == "package") {
 			$pkgbase_name = $row["PackageBaseName"];
 		}
 
+		$anchor = (isset($pinned) ? "pinned-" : "comment-") . $row['ID'];
 		$date_fmtd = date('Y-m-d H:i', $row['CommentTS']);
+		$date_link = '<a href="#' . $anchor . '" class="date">' . $date_fmtd . '</a>';
 		if ($comment_section == "package") {
 			if ($row['UserName']) {
 				$user_fmtd = html_format_username($row['UserName']);
-				$heading = __('%s commented on %s', $user_fmtd, $date_fmtd);
+				$heading = __('%s commented on %s', $user_fmtd, $date_link);
 			} else {
-				$heading = __('Anonymous comment on %s', $date_fmtd);
+				$heading = __('Anonymous comment on %s', $date_link);
 			}
 		} elseif ($comment_section == "account") {
 			$pkg_uri = '<a href=' . htmlspecialchars(get_pkg_uri($row['PackageBaseName']), ENT_QUOTES) . '>' . htmlspecialchars($row['PackageBaseName']) . '</a></td>';
-			$heading = __('Commented on package %s on %s', $pkg_uri, $date_fmtd);
+			$heading = __('Commented on package %s on %s', $pkg_uri, $date_link);
 		}
 
 		$is_deleted = $row['DelTS'];
@@ -97,7 +99,7 @@  if ($comment_section == "package") {
 			$comment_classes .= " comment-deleted";
 		}
 		?>
-		<h4 id="<?= isset($pinned) ? "pinned-" : "comment-" ?><?= $row['ID'] ?>" class="<?= $comment_classes ?>">
+		<h4 id="<?= $anchor ?>" class="<?= $comment_classes ?>">
 			<?= $heading ?>
 			<?php if ($is_deleted && has_credential(CRED_COMMENT_UNDELETE)): ?>
 				<form class="undelete-comment-form" method="post" action="<?= htmlspecialchars(get_pkgbase_uri($pkgbase_name), ENT_QUOTES); ?>">
@@ -152,7 +154,7 @@  if ($comment_section == "package") {
 				</form>
 			<?php endif; ?>
 		</h4>
-		<div id="<?= isset($pinned) ? "pinned-" : "comment-" ?><?= $row['ID'] ?>-content" class="article-content<?php if ($is_deleted): ?> comment-deleted<?php endif; ?>">
+		<div id="<?= $anchor ?>-content" class="article-content<?php if ($is_deleted): ?> comment-deleted<?php endif; ?>">
 			<div>
 				<?php if (!empty($row['RenderedComment'])): ?>
 				<?= $row['RenderedComment'] ?>