DEV: Improve method of presenting link clicks (#29453)

Currently the tracking for clicked links are injected into the HTML in a span tag. This leads to the link counter value being highlighted when copying and pasting. Additionally, any means for using CSS to hide link counters result in a gap due to it occupying a specific width.

With this change, we make link counters appear in a data attribute on the link element and visually shown with CSS `::after` element.
This commit is contained in:
Keegan George 2024-11-01 04:44:08 +09:00 committed by GitHub
parent 927054b01e
commit 71f808dea9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 82 additions and 59 deletions

View File

@ -1,5 +1,6 @@
import Component from "@glimmer/component";
import replaceEmoji from "discourse/helpers/replace-emoji";
import i18n from "discourse-common/helpers/i18n";
import and from "truth-helpers/helpers/and";
const TRUNCATE_LENGTH_LIMIT = 85;
@ -27,6 +28,8 @@ export default class TopicMapLink extends Component {
data-ignore-post-id="true"
target="_blank"
rel="nofollow ugc noopener noreferrer"
data-clicks={{@clickCount}}
aria-label={{i18n "topic_map.clicks" count=@clickCount}}
>
{{#if @title}}
{{replaceEmoji this.truncatedContent}}

View File

@ -344,19 +344,13 @@ export default class TopicMapSummary extends Component {
<ul class="topic-links">
{{#each this.linksToShow as |link|}}
<li>
<span
class="badge badge-notification clicks"
title={{i18n "topic_map.clicks" count=link.clicks}}
>
{{link.clicks}}
</span>
<TopicMapLink
@attachment={{link.attachment}}
@title={{link.title}}
@rootDomain={{link.root_domain}}
@url={{link.url}}
@userId={{link.user_id}}
@clickCount={{link.clicks}}
/>
</li>
{{/each}}

View File

@ -194,18 +194,12 @@
'nofollow ugc'
}}"
target="_blank"
data-clicks={{link.clicks}}
aria-label={{i18n "topic_map.clicks" count=link.clicks}}
>
{{shorten-url link.url}}
</a>
{{! template-lint-enable link-rel-noopener }}
<span
class="badge badge-notification clicks"
title={{i18n "topic_map.clicks" count=link.clicks}}
>
{{number link.clicks}}
</span>
<br />
<a href={{link.post_url}}>

View File

@ -9,7 +9,6 @@ import {
destroyUserStatusOnMentions,
updateUserStatusOnMention,
} from "discourse/lib/update-user-status-on-mention";
import domFromString from "discourse-common/lib/dom-from-string";
import escape from "discourse-common/lib/escape";
import { getOwnerWithFallback } from "discourse-common/lib/get-owner";
import getURL from "discourse-common/lib/get-url";
@ -162,16 +161,14 @@ export default class PostCooked {
!bestElements.has(onebox) ||
bestElements.get(onebox) === link
) {
const title = I18n.t("topic_map.clicks", { count: lc.clicks });
link.appendChild(document.createTextNode(" "));
link.appendChild(
domFromString(
`<span class='badge badge-notification clicks' title='${title}'>${number(
lc.clicks
)}</span>`
)[0]
);
link.setAttribute("data-clicks", number(lc.clicks));
const ariaLabel = `${link.textContent.trim()} ${I18n.t(
"post.link_clicked",
{
count: lc.clicks,
}
)}`;
link.setAttribute("aria-label", ariaLabel);
}
}
});

View File

@ -13,10 +13,12 @@ export default createWidget("post-links", {
linkHtml(link) {
const linkBody = replaceEmoji(link.title);
const attributes = {
href: link.url,
};
if (link.clicks) {
linkBody.push(
h("span.badge.badge-notification.clicks", link.clicks.toString())
);
attributes["data-clicks"] = link.clicks.toString();
}
return h(
@ -25,7 +27,7 @@ export default createWidget("post-links", {
"a.track-link",
{
className: "inbound",
attributes: { href: link.url },
attributes,
},
[iconNode("link"), linkBody]
)

View File

@ -408,7 +408,7 @@ acceptance("Composer", function (needs) {
assert
.dom(".topic-post:last-of-type .cooked p")
.hasText(
"If you use gettext format you could leverage Launchpad 13 translations and the community behind it."
"If you use gettext format you could leverage Launchpad translations and the community behind it."
);
});

View File

@ -271,6 +271,11 @@ acceptance("Topic featured links", function (needs) {
display_name_on_posts: false,
prioritize_username_in_ux: true,
});
needs.pretender((server, helper) => {
server.get("/inline-onebox", () => {
return helper.response({ "inline-oneboxes": [] });
});
});
test("remove nofollow attribute", async function (assert) {
await visit("/t/-/299/1");
@ -433,7 +438,6 @@ acceptance("Topic featured links", function (needs) {
await visit("/t/internationalization-localization/280");
await selectText("#post_5 .cooked");
await click(".quote-button .insert-quote");
assert.ok(
query(".d-editor-input").value.includes(
'quote="pekka, post:5, topic:280, full:true"'

View File

@ -44,8 +44,14 @@ module("Integration | Component | Widget | post", function (hooks) {
hbs`<MountWidget @widget="post-contents" @args={{this.args}} />`
);
assert.strictEqual(queryAll(".badge.clicks")[0].innerText, "1");
assert.strictEqual(queryAll(".badge.clicks")[1].innerText, "2");
assert.strictEqual(
queryAll("a[data-clicks='1']")[0].getAttribute("data-clicks"),
"1"
);
assert.strictEqual(
queryAll("a[data-clicks='2']")[0].getAttribute("data-clicks"),
"2"
);
});
test("post - onebox links", async function (assert) {
@ -72,9 +78,16 @@ module("Integration | Component | Widget | post", function (hooks) {
hbs`<MountWidget @widget="post-contents" @args={{this.args}} />`
);
assert.strictEqual(count(".badge.clicks"), 2);
assert.strictEqual(queryAll(".badge.clicks")[0].innerText, "1");
assert.strictEqual(queryAll(".badge.clicks")[1].innerText, "2");
assert.strictEqual(
queryAll("a[data-clicks='1']")[0].getAttribute("data-clicks"),
"1",
"First link has correct data attribute and content"
);
assert.strictEqual(
queryAll("a[data-clicks='2']")[0].getAttribute("data-clicks"),
"2",
"Second link has correct data attribute and content"
);
});
test("wiki", async function (assert) {

View File

@ -388,10 +388,6 @@
a[href] {
color: var(--primary-med-or-secondary-med);
}
.clicks {
margin-left: 0.5em;
flex: 0 0 auto;
}
.d-icon {
font-size: var(--font-down-2);
margin: 0 0.5em 0 0;
@ -458,3 +454,7 @@ a.topic-featured-link {
text-transform: lowercase;
}
}
a[data-clicks]::after {
@include click-counter-badge;
}

View File

@ -112,17 +112,6 @@
width: 8px;
border-radius: 50%;
}
// Click count
&.clicks {
font-weight: normal;
background-color: var(--primary-low);
top: -1px;
color: var(--primary-medium);
position: relative;
border: none;
}
}
// Posts badge

View File

@ -393,14 +393,19 @@ body:not(.archetype-private_message) {
}
}
}
.badge {
grid-area: counter;
align-self: start;
top: 0.2em;
}
.topic-link {
grid-area: link;
}
.topic-link[data-clicks]::before {
@include click-counter-badge;
}
.topic-link[data-clicks]::after {
display: none;
}
.domain {
grid-area: domain;
font-size: var(--font-down-2);

View File

@ -292,3 +292,21 @@ $hpad: 0.65em;
background: var(--d-nav-color--active);
}
}
@mixin click-counter-badge {
content: attr(data-clicks);
font-weight: normal;
background-color: var(--primary-low);
color: var(--primary-medium);
position: relative;
top: -1px;
padding: 0.21em 0.42em;
min-width: 0.5em;
line-height: var(--line-height-small);
font-size: var(--font-down-2);
text-align: center;
border-radius: 10px;
white-space: nowrap;
display: inline-block;
margin: 0.15em;
}

View File

@ -32,7 +32,7 @@
#footer,
.alert-info,
.badge-category,
.badge-notification.clicks,
a[data-clicks]::after,
.crawler-nav,
.powered-by-link,
.timeline-container,

View File

@ -195,7 +195,8 @@ html {
color: var(--primary-medium);
}
.badge-notification.clicks {
a[data-clicks]::before,
a[data-clicks]::after {
color: var(--primary-high);
}

View File

@ -3766,6 +3766,9 @@ en:
one: "view %{count} hidden reply"
other: "view %{count} hidden replies"
sr_reply_to: "Reply to post #%{post_number} by @%{username}"
link_clicked:
one: "link clicked %{count} time"
other: "link clicked %{count} times"
notice:
new_user: "This is the first time %{user} has posted — lets welcome them to our community!"