Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

DEV: Use data attributes as identifier for sidebar category section links (#20384) 

Why this change?

Prior to this change, we placed the identifier for the category using
CSS classes like `sidebar-section-link-<category slug>`. However, we found that it wasn't obvious
that an identifier for the category exists since it is first buried in
the CSS classes and second there isn't a way to describe what the
identifier is. Using data attributes, it makes it more obvious that an
identifier exists and what the identifier represents.
This commit is contained in:
Alan Guo Xiang Tan 2023-02-21 10:29:44 +08:00 committed by GitHub
parent 359dc1c532
commit 53eb49de72
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 95 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/assets/javascripts/discourse/app/components/sidebar/anonymous/categories-section.hbs
View File

@ -6,7 +6,6 @@
{{#each this.sectionLinks as |sectionLink|}} {{#each this.sectionLinks as |sectionLink|}}
<Sidebar::SectionLink <Sidebar::SectionLink
@linkName={{sectionLink.name}}
@route={{sectionLink.route}} @route={{sectionLink.route}}
@title={{sectionLink.title}} @title={{sectionLink.title}}
@content={{sectionLink.text}} @content={{sectionLink.text}}
@ -16,6 +15,7 @@
@prefixValue={{sectionLink.prefixValue}} @prefixValue={{sectionLink.prefixValue}}
@prefixColor={{sectionLink.prefixColor}} @prefixColor={{sectionLink.prefixColor}}
@prefixElementColors={{sectionLink.prefixElementColors}} @prefixElementColors={{sectionLink.prefixElementColors}}
data-category-id={{sectionLink.category.id}}
/> />
{{/each}} {{/each}}

2
app/assets/javascripts/discourse/app/components/sidebar/section-link.hbs
View File

@ -10,6 +10,7 @@
target="_blank" target="_blank"
class={{this.classNames}} class={{this.classNames}}
title={{@title}} title={{@title}}
...attributes
> >
<Sidebar::SectionLinkPrefix <Sidebar::SectionLinkPrefix
@prefixType={{@prefixType}} @prefixType={{@prefixType}}
@ -32,6 +33,7 @@
@models={{this.models}} @models={{this.models}}
@current-when={{@currentWhen}} @current-when={{@currentWhen}}
title={{@title}} title={{@title}}
...attributes
> >
<Sidebar::SectionLinkPrefix <Sidebar::SectionLinkPrefix

10
app/assets/javascripts/discourse/app/components/sidebar/section-link.js
View File

@ -22,11 +22,11 @@ export default class SectionLink extends Component {
} }
get classNames() { get classNames() {
let classNames = [ let classNames = ["sidebar-section-link", "sidebar-row"];
"sidebar-section-link",
`sidebar-section-link-${this.args.linkName}`, if (this.args.linkName) {
"sidebar-row", classNames.push(`sidebar-section-link-${this.args.linkName}`);
]; }
if (this.args.class) { if (this.args.class) {
classNames.push(this.args.class); classNames.push(this.args.class);

2
app/assets/javascripts/discourse/app/components/sidebar/user/categories-section.hbs
View File

@ -15,7 +15,6 @@
{{#if (gt this.sectionLinks.length 0)}} {{#if (gt this.sectionLinks.length 0)}}
{{#each this.sectionLinks as |sectionLink|}} {{#each this.sectionLinks as |sectionLink|}}
<Sidebar::SectionLink <Sidebar::SectionLink
@linkName={{sectionLink.name}}
@route={{sectionLink.route}} @route={{sectionLink.route}}
@query={{sectionLink.query}} @query={{sectionLink.query}}
@title={{sectionLink.title}} @title={{sectionLink.title}}
@ -31,6 +30,7 @@
@suffixCSSClass={{sectionLink.suffixCSSClass}} @suffixCSSClass={{sectionLink.suffixCSSClass}}
@suffixValue={{sectionLink.suffixValue}} @suffixValue={{sectionLink.suffixValue}}
@suffixType={{sectionLink.suffixType}} @suffixType={{sectionLink.suffixType}}
data-category-id={{sectionLink.category.id}}
/> />
{{/each}} {{/each}}
{{else}} {{else}}

17
app/assets/javascripts/discourse/tests/acceptance/sidebar-plugin-api-test.js
View File

@ -684,20 +684,21 @@ acceptance("Sidebar - Plugin API", function (needs) {
assert.ok( assert.ok(
exists( exists(
`.sidebar-section-link-${category1.name} .sidebar-section-link-suffix.unread` `.sidebar-section-link[data-category-id="${category1.id}"] .sidebar-section-link-suffix.unread`
), ),
"the right suffix is displayed when custom countable is active" "the right suffix is displayed when custom countable is active"
); );
assert.strictEqual( assert.strictEqual(
query(`.sidebar-section-link-${category1.name}`).pathname, query(`.sidebar-section-link[data-category-id="${category1.id}"]`)
.pathname,
`/c/${category1.name}/${category1.id}`, `/c/${category1.name}/${category1.id}`,
"does not use route configured for custom countable when user has elected not to show any counts in sidebar" "does not use route configured for custom countable when user has elected not to show any counts in sidebar"
); );
assert.notOk( assert.notOk(
exists( exists(
`.sidebar-section-link-${category2.name} .sidebar-section-link-suffix.unread` `.sidebar-section-link[data-category-id="${category2.id}"] .sidebar-section-link-suffix.unread`
), ),
"does not display suffix when custom countable is not registered" "does not display suffix when custom countable is not registered"
); );
@ -708,7 +709,7 @@ acceptance("Sidebar - Plugin API", function (needs) {
assert.strictEqual( assert.strictEqual(
query( query(
`.sidebar-section-link-${category1.name} .sidebar-section-link-content-badge` `.sidebar-section-link[data-category-id="${category1.id}"] .sidebar-section-link-content-badge`
).innerText.trim(), ).innerText.trim(),
I18n.t("sidebar.unread_count", { count: 1 }), I18n.t("sidebar.unread_count", { count: 1 }),
"displays the right badge text in section link when unread is present and custom countable is not prioritised over unread" "displays the right badge text in section link when unread is present and custom countable is not prioritised over unread"
@ -722,20 +723,22 @@ acceptance("Sidebar - Plugin API", function (needs) {
assert.strictEqual( assert.strictEqual(
query( query(
`.sidebar-section-link-${category1.name} .sidebar-section-link-content-badge` `.sidebar-section-link[data-category-id="${category1.id}"] .sidebar-section-link-content-badge`
).innerText.trim(), ).innerText.trim(),
`some custom ${category1.topic_count}`, `some custom ${category1.topic_count}`,
"displays the right badge text in section link when unread is present but custom countable is prioritised over unread" "displays the right badge text in section link when unread is present but custom countable is prioritised over unread"
); );
assert.strictEqual( assert.strictEqual(
query(`.sidebar-section-link-${category1.name}`).pathname, query(`.sidebar-section-link[data-category-id="${category1.id}"]`)
.pathname,
`/c/${category1.name}/${category1.id}/l/latest`, `/c/${category1.name}/${category1.id}/l/latest`,
"has the right pathname for section link" "has the right pathname for section link"
); );
assert.strictEqual( assert.strictEqual(
query(`.sidebar-section-link-${category1.name}`).search, query(`.sidebar-section-link[data-category-id="${category1.id}"]`)
.search,
"?status=open", "?status=open",
"has the right query params for section link" "has the right query params for section link"
); );

86
app/assets/javascripts/discourse/tests/acceptance/sidebar-user-categories-section-test.js
View File

@ -50,7 +50,7 @@ acceptance(
); );
assert.ok( assert.ok(
exists(`.sidebar-section-link-${category1.slug}`), exists(`.sidebar-section-link[data-category-id="${category1.id}"]`),
`only the ${category1.slug} section link is shown` `only the ${category1.slug} section link is shown`
); );
}); });
@ -187,7 +187,9 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
await visit("/"); await visit("/");
assert.ok( assert.ok(
exists(`.sidebar-section-link-${uncategorizedCategory.slug}`), exists(
`.sidebar-section-link[data-category-id="${uncategorizedCategory.id}"]`
),
`displays the section link for ${uncategorizedCategory.slug} category` `displays the section link for ${uncategorizedCategory.slug} category`
); );
}); });
@ -397,18 +399,20 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
assert.ok( assert.ok(
exists( exists(
`.sidebar-section-link-${category1.slug} .sidebar-section-link-prefix .prefix-span[style="background: linear-gradient(90deg, #${category1.color} 50%, #${category1.color} 50%)"]` `.sidebar-section-link[data-category-id="${category1.id}"] .sidebar-section-link-prefix .prefix-span[style="background: linear-gradient(90deg, #${category1.color} 50%, #${category1.color} 50%)"]`
), ),
"category1 section link is rendered with solid prefix icon color" "category1 section link is rendered with solid prefix icon color"
); );
assert.strictEqual( assert.strictEqual(
query(`.sidebar-section-link-${category1.slug}`).textContent.trim(), query(
`.sidebar-section-link[data-category-id="${category1.id}"]`
).textContent.trim(),
category1.name, category1.name,
"displays category1's name for the link text" "displays category1's name for the link text"
); );
await click(`.sidebar-section-link-${category1.slug}`); await click(`.sidebar-section-link[data-category-id="${category1.id}"]`);
assert.strictEqual( assert.strictEqual(
currentURL(), currentURL(),
@ -423,11 +427,13 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
); );
assert.ok( assert.ok(
exists(`.sidebar-section-link-${category1.slug}.active`), exists(
`.sidebar-section-link[data-category-id="${category1.id}"].active`
),
"the category1 section link is marked as active" "the category1 section link is marked as active"
); );
await click(`.sidebar-section-link-${category2.slug}`); await click(`.sidebar-section-link[data-category-id="${category2.id}"]`);
assert.strictEqual( assert.strictEqual(
currentURL(), currentURL(),
@ -442,20 +448,22 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
); );
assert.ok( assert.ok(
exists(`.sidebar-section-link-${category2.slug}.active`), exists(
`.sidebar-section-link[data-category-id="${category2.id}"].active`
),
"the category2 section link is marked as active" "the category2 section link is marked as active"
); );
assert.ok( assert.ok(
exists( exists(
`.sidebar-section-link-${category3.slug} .sidebar-section-link-prefix .prefix-badge.d-icon-lock` `.sidebar-section-link[data-category-id="${category3.id}"] .sidebar-section-link-prefix .prefix-badge.d-icon-lock`
), ),
"category3 section link is rendered with lock prefix badge icon as it is read restricted" "category3 section link is rendered with lock prefix badge icon as it is read restricted"
); );
assert.ok( assert.ok(
exists( exists(
`.sidebar-section-link-${category4.slug} .sidebar-section-link-prefix .prefix-span[style="background: linear-gradient(90deg, #${category4.parentCategory.color} 50%, #${category4.color} 50%)"]` `.sidebar-section-link[data-category-id="${category4.id}"] .sidebar-section-link-prefix .prefix-span[style="background: linear-gradient(90deg, #${category4.parentCategory.color} 50%, #${category4.color} 50%)"]`
), ),
"sub category section link is rendered with double prefix color" "sub category section link is rendered with double prefix color"
); );
@ -469,7 +477,7 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
await visit("/"); await visit("/");
await click(`.sidebar-section-link-${category1.slug}`); await click(`.sidebar-section-link[data-category-id="${category1.id}"]`);
assert.strictEqual( assert.strictEqual(
currentURL(), currentURL(),
@ -484,7 +492,9 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
); );
assert.ok( assert.ok(
exists(`.sidebar-section-link-${category1.slug}.active`), exists(
`.sidebar-section-link[data-category-id="${category1.id}"].active`
),
"the category1 section link is marked as active" "the category1 section link is marked as active"
); );
}); });
@ -507,7 +517,7 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
await visit("/"); await visit("/");
await click(`.sidebar-section-link-${category1.slug}`); await click(`.sidebar-section-link[data-category-id="${category1.id}"]`);
assert.strictEqual( assert.strictEqual(
currentURL(), currentURL(),
@ -522,7 +532,9 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
); );
assert.ok( assert.ok(
exists(`.sidebar-section-link-${category1.slug}.active`), exists(
`.sidebar-section-link[data-category-id="${category1.id}"].active`
),
"the category1 section link is marked as active" "the category1 section link is marked as active"
); );
}); });
@ -553,7 +565,7 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
await visit("/"); await visit("/");
await click(`.sidebar-section-link-${category1.slug}`); await click(`.sidebar-section-link[data-category-id="${category1.id}"]`);
assert.strictEqual( assert.strictEqual(
currentURL(), currentURL(),
@ -568,7 +580,9 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
); );
assert.ok( assert.ok(
exists(`.sidebar-section-link-${category1.slug}.active`), exists(
`.sidebar-section-link[data-category-id="${category1.id}"].active`
),
"the category1 section link is marked as active" "the category1 section link is marked as active"
); );
}); });
@ -581,7 +595,7 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
assert.ok( assert.ok(
exists( exists(
`.sidebar-section-link-${category1.slug} .sidebar-section-link-prefix .prefix-span[style="background: linear-gradient(90deg, #888 50%, #888 50%)"]` `.sidebar-section-link[data-category-id="${category1.id}"] .sidebar-section-link-prefix .prefix-span[style="background: linear-gradient(90deg, #888 50%, #888 50%)"]`
), ),
"category1 section link is rendered with the right solid prefix icon color" "category1 section link is rendered with the right solid prefix icon color"
); );
@ -600,7 +614,7 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
await visit("/"); await visit("/");
assert.strictEqual( assert.strictEqual(
query(`.sidebar-section-link-${category.slug}`).title, query(`.sidebar-section-link[data-category-id="${category.id}"]`).title,
category.description_text, category.description_text,
"category description without HTML entity is used as the link's title" "category description without HTML entity is used as the link's title"
); );
@ -618,7 +632,9 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
); );
assert.ok( assert.ok(
exists(`.sidebar-section-link-${category1.slug}.active`), exists(
`.sidebar-section-link[data-category-id="${category1.id}"].active`
),
"the category1 section link is marked as active for the new route" "the category1 section link is marked as active for the new route"
); );
}); });
@ -635,7 +651,9 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
); );
assert.ok( assert.ok(
exists(`.sidebar-section-link-${category1.slug}.active`), exists(
`.sidebar-section-link[data-category-id="${category1.id}"].active`
),
"the category1 section link is marked as active for the unread route" "the category1 section link is marked as active for the unread route"
); );
}); });
@ -652,7 +670,9 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
); );
assert.ok( assert.ok(
exists(`.sidebar-section-link-${category1.slug}.active`), exists(
`.sidebar-section-link[data-category-id="${category1.id}"].active`
),
"the category1 section link is marked as active for the top route" "the category1 section link is marked as active for the top route"
); );
}); });
@ -669,7 +689,9 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
); );
assert.ok( assert.ok(
exists(`.sidebar-section-link-${category1.slug}.active`), exists(
`.sidebar-section-link[data-category-id="${category1.id}"].active`
),
"the category1 section link is marked as active for the none route" "the category1 section link is marked as active for the none route"
); );
}); });
@ -686,7 +708,9 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
); );
assert.ok( assert.ok(
exists(`.sidebar-section-link-${category1.slug}.active`), exists(
`.sidebar-section-link[data-category-id="${category1.id}"].active`
),
"the category1 section link is marked as active for the all route" "the category1 section link is marked as active for the all route"
); );
}); });
@ -725,7 +749,7 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
assert.ok( assert.ok(
exists( exists(
`.sidebar-section-link-${category1.slug} .sidebar-section-link-suffix` `.sidebar-section-link[data-category-id="${category1.id}"] .sidebar-section-link-suffix`
), ),
"shows suffix indicator for unread content on categories link" "shows suffix indicator for unread content on categories link"
); );
@ -741,7 +765,7 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
assert.ok( assert.ok(
exists( exists(
`.sidebar-section-link-${category1.slug} .sidebar-section-link-suffix` `.sidebar-section-link[data-category-id="${category1.id}"] .sidebar-section-link-suffix`
), ),
"shows suffix indicator for new topics on categories link" "shows suffix indicator for new topics on categories link"
); );
@ -757,7 +781,7 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
assert.ok( assert.ok(
!exists( !exists(
`.sidebar-section-link-${category1.slug} .sidebar-section-link-suffix` `.sidebar-section-link[data-category-id="${category1.id}"] .sidebar-section-link-suffix`
), ),
"hides suffix indicator when there's no new/unread content on category link" "hides suffix indicator when there's no new/unread content on category link"
); );
@ -817,7 +841,7 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
assert.strictEqual( assert.strictEqual(
query( query(
`.sidebar-section-link-${category1.slug} .sidebar-section-link-content-badge` `.sidebar-section-link[data-category-id="${category1.id}"] .sidebar-section-link-content-badge`
).textContent.trim(), ).textContent.trim(),
I18n.t("sidebar.unread_count", { count: 1 }), I18n.t("sidebar.unread_count", { count: 1 }),
`displays 1 unread count for ${category1.slug} section link` `displays 1 unread count for ${category1.slug} section link`
@ -825,7 +849,7 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
assert.strictEqual( assert.strictEqual(
query( query(
`.sidebar-section-link-${category2.slug} .sidebar-section-link-content-badge` `.sidebar-section-link[data-category-id="${category2.id}"] .sidebar-section-link-content-badge`
).textContent.trim(), ).textContent.trim(),
I18n.t("sidebar.unread_count", { count: 2 }), I18n.t("sidebar.unread_count", { count: 2 }),
`displays 2 unread count for ${category2.slug} section link` `displays 2 unread count for ${category2.slug} section link`
@ -842,7 +866,7 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
assert.strictEqual( assert.strictEqual(
query( query(
`.sidebar-section-link-${category1.slug} .sidebar-section-link-content-badge` `.sidebar-section-link[data-category-id="${category1.id}"] .sidebar-section-link-content-badge`
).textContent.trim(), ).textContent.trim(),
I18n.t("sidebar.new_count", { count: 1 }), I18n.t("sidebar.new_count", { count: 1 }),
`displays 1 new count for ${category1.slug} section link` `displays 1 new count for ${category1.slug} section link`
@ -859,7 +883,7 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
assert.ok( assert.ok(
!exists( !exists(
`.sidebar-section-link-${category1.slug} .sidebar-section-link-content-badge` `.sidebar-section-link[data-category-id="${category1.id}"] .sidebar-section-link-content-badge`
), ),
`does not display any badge ${category1.slug} section link` `does not display any badge ${category1.slug} section link`
); );
@ -875,7 +899,7 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
assert.strictEqual( assert.strictEqual(
query( query(
`.sidebar-section-link-${category2.slug} .sidebar-section-link-content-badge` `.sidebar-section-link[data-category-id="${category2.id}"] .sidebar-section-link-content-badge`
).textContent.trim(), ).textContent.trim(),
I18n.t("sidebar.unread_count", { count: 1 }), I18n.t("sidebar.unread_count", { count: 1 }),
`displays 1 unread count for ${category2.slug} section link` `displays 1 unread count for ${category2.slug} section link`

28
app/assets/javascripts/discourse/tests/acceptance/user-preferences-sidebar-test.js
View File

@ -66,7 +66,9 @@ acceptance("User Preferences - Sidebar", function (needs) {
await visit("/"); await visit("/");
assert.ok( assert.ok(
exists(".sidebar-section-categories .sidebar-section-link-support"), exists(
".sidebar-section-categories .sidebar-section-link[data-category-id=6]"
),
"support category is present in sidebar" "support category is present in sidebar"
); );
@ -90,12 +92,16 @@ acceptance("User Preferences - Sidebar", function (needs) {
await click(".dialog-footer .btn-primary"); await click(".dialog-footer .btn-primary");
assert.ok( assert.ok(
!exists(".sidebar-section-categories .sidebar-section-link-howto"), !exists(
".sidebar-section-categories .sidebar-section-link[data-category-id=10]"
),
"howto category is not displayed in sidebar" "howto category is not displayed in sidebar"
); );
assert.ok( assert.ok(
exists(".sidebar-section-categories .sidebar-section-link-support"), exists(
".sidebar-section-categories .sidebar-section-link[data-category-id=6]"
),
"support category is displayed in sidebar" "support category is displayed in sidebar"
); );
}); });
@ -113,12 +119,16 @@ acceptance("User Preferences - Sidebar", function (needs) {
await click(".save-changes"); await click(".save-changes");
assert.ok( assert.ok(
exists(".sidebar-section-categories .sidebar-section-link-support"), exists(
".sidebar-section-categories .sidebar-section-link[data-category-id=6]"
),
"support category has been added to sidebar" "support category has been added to sidebar"
); );
assert.ok( assert.ok(
exists(".sidebar-section-categories .sidebar-section-link-bug"), exists(
".sidebar-section-categories .sidebar-section-link[data-category-id=1]"
),
"bug category has been added to sidebar" "bug category has been added to sidebar"
); );
}); });
@ -137,12 +147,16 @@ acceptance("User Preferences - Sidebar", function (needs) {
await click(".save-changes"); await click(".save-changes");
assert.ok( assert.ok(
exists(".sidebar-section-categories .sidebar-section-link-support"), exists(
".sidebar-section-categories .sidebar-section-link[data-category-id=6]"
),
"support category has been added to sidebar" "support category has been added to sidebar"
); );
assert.ok( assert.ok(
exists(".sidebar-section-categories .sidebar-section-link-bug"), exists(
".sidebar-section-categories .sidebar-section-link[data-category-id=1]"
),
"bug category has been added to sidebar" "bug category has been added to sidebar"
); );