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

UX: Display tag's description as title in navigation menu (#22710) 

Why this change?

We're already displaying a category's description as the title attribute
on the category section link. We should do the same for tags as well.
This commit is contained in:
Alan Guo Xiang Tan 2023-07-24 08:07:37 +08:00 committed by GitHub
parent e19316aebf
commit 7a790a5f4c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 269 additions and 171 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
app/assets/javascripts/discourse/app/components/sidebar/anonymous/tags-section.hbs
View File

@ -9,6 +9,7 @@
<Sidebar::SectionLink
@route={{sectionLink.route}}
@content={{sectionLink.text}}
@title={{sectionLink.title}}
@currentWhen={{sectionLink.currentWhen}}
@prefixType={{sectionLink.prefixType}}
@prefixValue={{sectionLink.prefixValue}}

13
app/assets/javascripts/discourse/app/components/sidebar/anonymous/tags-section.js
View File

@ -1,10 +1,10 @@
import Component from "@glimmer/component";
import { cached } from "@glimmer/tracking";
import { inject as service } from "@ember/service";
import SidebarCommonTagsSection from "discourse/components/sidebar/common/tags-section";
import TagSectionLink from "discourse/lib/sidebar/user/tags-section/tag-section-link";
export default class SidebarAnonymousTagsSection extends SidebarCommonTagsSection {
export default class SidebarAnonymousTagsSection extends Component {
@service router;
@service topicTrackingState;
@service site;
@ -12,17 +12,18 @@ export default class SidebarAnonymousTagsSection extends SidebarCommonTagsSectio
get displaySection() {
return (
this.site.anonymous_default_navigation_menu_tags?.length > 0 ||
this.topSiteTags?.length > 0
this.site.navigation_menu_site_top_tags?.length > 0
);
}
@cached
get sectionLinks() {
return (
this.site.anonymous_default_navigation_menu_tags || this.topSiteTags
).map((tagName) => {
this.site.anonymous_default_navigation_menu_tags ||
this.site.navigation_menu_site_top_tags
).map((tag) => {
return new TagSectionLink({
tagName,
tag,
topicTrackingState: this.topicTrackingState,
});
});

20
app/assets/javascripts/discourse/app/components/sidebar/common/tags-section.js
View File

@ -1,20 +0,0 @@
import Component from "@glimmer/component";
import { inject as service } from "@ember/service";
export const TOP_SITE_TAGS_TO_SHOW = 5;
export default class SidebarCommonTagsSection extends Component {
@service site;
topSiteTags = [];
constructor() {
super(...arguments);
if (this.site.top_tags?.length > 0) {
this.site.top_tags.splice(0, TOP_SITE_TAGS_TO_SHOW).forEach((tagName) => {
this.topSiteTags.push(tagName);
});
}
}
}

16
app/assets/javascripts/discourse/app/components/sidebar/user/tags-section.js
View File

@ -1,18 +1,19 @@
import Component from "@glimmer/component";
import { action } from "@ember/object";
import { cached } from "@glimmer/tracking";
import { inject as service } from "@ember/service";
import SidebarCommonTagsSection from "discourse/components/sidebar/common/tags-section";
import TagSectionLink from "discourse/lib/sidebar/user/tags-section/tag-section-link";
import PMTagSectionLink from "discourse/lib/sidebar/user/tags-section/pm-tag-section-link";
import { hasDefaultSidebarTags } from "discourse/lib/sidebar/helpers";
import SidebarEditNavigationMenuTagsModal from "discourse/components/sidebar/edit-navigation-menu/tags-modal";
export default class SidebarUserTagsSection extends SidebarCommonTagsSection {
export default class SidebarUserTagsSection extends Component {
@service currentUser;
@service modal;
@service pmTopicTrackingState;
@service router;
@service site;
@service siteSettings;
@service topicTrackingState;
@ -41,26 +42,21 @@ export default class SidebarUserTagsSection extends SidebarCommonTagsSection {
if (this.currentUser.sidebarTags.length > 0) {
tags = this.currentUser.sidebarTags;
} else {
tags = this.topSiteTags.map((tagName) => {
return {
name: tagName,
pm_only: false,
};
});
tags = this.site.navigation_menu_site_top_tags || [];
}
for (const tag of tags) {
if (tag.pm_only) {
links.push(
new PMTagSectionLink({
tagName: tag.name,
tag,
currentUser: this.currentUser,
})
);
} else {
links.push(
new TagSectionLink({
tagName: tag.name,
tag,
topicTrackingState: this.topicTrackingState,
currentUser: this.currentUser,
})

11
app/assets/javascripts/discourse/app/lib/sidebar/user/tags-section/base-tag-section-link.js
View File

@ -1,3 +1,5 @@
import { escape } from "pretty-text/sanitizer";
let customTagSectionLinkPrefixIcons = {};
export function registerCustomTagSectionLinkPrefixIcon({
@ -20,8 +22,9 @@ export function resetCustomTagSectionLinkPrefixIcons() {
}
export default class BaseTagSectionLink {
constructor({ tagName, currentUser }) {
this.tagName = tagName;
constructor({ tag, currentUser }) {
this.tag = tag;
this.tagName = tag.name;
this.currentUser = currentUser;
}
@ -33,6 +36,10 @@ export default class BaseTagSectionLink {
return this.tagName;
}
get title() {
return escape(this.tag.description);
}
get prefixType() {
return "icon";
}

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

@ -7,35 +7,11 @@ import {
exists,
publishToMessageBus,
query,
queryAll,
updateCurrentUser,
} from "discourse/tests/helpers/qunit-helpers";
import discoveryFixture from "discourse/tests/fixtures/discovery-fixtures";
import { cloneJSON } from "discourse-common/lib/object";
import { NotificationLevels } from "discourse/lib/notification-levels";
import Site from "discourse/models/site";
import { TOP_SITE_TAGS_TO_SHOW } from "discourse/components/sidebar/common/tags-section";
acceptance(
"Sidebar - Logged on user - Tags section - tagging disabled",
function (needs) {
needs.settings({
tagging_enabled: false,
navigation_menu: "sidebar",
});
needs.user();
test("tags section is not shown", async function (assert) {
await visit("/");
assert.ok(
!exists(".sidebar-section[data-section-name='tags']"),
"does not display the tags section"
);
});
}
);
acceptance("Sidebar - Logged on user - Tags section", function (needs) {
needs.settings({
@ -93,72 +69,6 @@ acceptance("Sidebar - Logged on user - Tags section", function (needs) {
});
});
test("section is not displayed when display_sidebar_tags property is false", async function (assert) {
updateCurrentUser({ display_sidebar_tags: false });
await visit("/");
assert.notOk(
exists(".sidebar-section[data-section-name='tags']"),
"tags section is not displayed"
);
});
test("tags section is displayed with site's top tags when user has not added any tags and there are no default tags configured", async function (assert) {
updateCurrentUser({
sidebar_tags: [],
});
Site.current().top_tags = [
"test1",
"test2",
"test3",
"test4",
"test5",
"test6",
];
await visit("/");
assert.ok(
exists(".sidebar-section[data-section-name='tags']"),
"tags section is displayed"
);
assert.strictEqual(
count(
".sidebar-section[data-section-name='tags'] .sidebar-section-link-wrapper[data-tag-name]"
),
TOP_SITE_TAGS_TO_SHOW,
"right number of tag section links are displayed"
);
["test1", "test2", "test3", "test4", "test5"].forEach((tagName) => {
assert.ok(
exists(`.sidebar-section-link-wrapper[data-tag-name=${tagName}]`),
`${tagName} tag section link is displayed`
);
});
});
test("tag section links are sorted alphabetically by tag's name", async function (assert) {
await visit("/");
const tagSectionLinks = queryAll(
".sidebar-section[data-section-name='tags'] .sidebar-section-link:not(.sidebar-section-link[data-link-name='all-tags'])"
);
const tagNames = [...tagSectionLinks].map((tagSectionLink) =>
tagSectionLink.textContent.trim()
);
assert.deepEqual(
tagNames,
["tag1", "tag2", "tag3", "tag4"],
"tag section links are displayed in the right order"
);
});
test("tag section links for user", async function (assert) {
await visit("/");

12
app/serializers/concerns/navigation_menu_tags_mixin.rb Normal file
View File

@ -0,0 +1,12 @@
# frozen_string_literal: true
module NavigationMenuTagsMixin
def serialize_tags(tags)
topic_count_column = Tag.topic_count_column(scope)
tags
.select(:name, topic_count_column, :pm_topic_count, :description)
.order(topic_count_column => :desc)
.map { |tag| SidebarTagSerializer.new(tag, scope: scope, root: false).as_json }
end
end

11
app/serializers/concerns/user_sidebar_mixin.rb
View File

@ -1,15 +1,10 @@
# frozen_string_literal: true
module UserSidebarMixin
def sidebar_tags
topic_count_column = Tag.topic_count_column(scope)
include NavigationMenuTagsMixin
object
.visible_sidebar_tags(scope)
.pluck(:name, topic_count_column, :pm_topic_count)
.reduce([]) do |tags, sidebar_tag|
tags.push(name: sidebar_tag[0], pm_only: sidebar_tag[1] == 0 && sidebar_tag[2] > 0)
end
def sidebar_tags
serialize_tags(object.visible_sidebar_tags(scope))
end
def display_sidebar_tags

10
app/serializers/sidebar_tag_serializer.rb Normal file
View File

@ -0,0 +1,10 @@
# frozen_string_literal: true
class SidebarTagSerializer < ApplicationSerializer
attributes :name, :description, :pm_only
def pm_only
topic_count_column = Tag.topic_count_column(scope)
object.public_send(topic_count_column) == 0 && object.pm_topic_count > 0
end
end

31
app/serializers/site_serializer.rb
View File

@ -1,6 +1,8 @@
# frozen_string_literal: true
class SiteSerializer < ApplicationSerializer
include NavigationMenuTagsMixin
attributes(
:default_archetype,
:notification_types,
@ -21,6 +23,7 @@ class SiteSerializer < ApplicationSerializer
:can_tag_pms,
:tags_filter_regexp,
:top_tags,
:navigation_menu_site_top_tags,
:can_associate_groups,
:wizard_required,
:topic_featured_link_allowed_category_ids,
@ -187,7 +190,7 @@ class SiteSerializer < ApplicationSerializer
end
def top_tags
Tag.top_tags(guardian: scope)
@top_tags ||= Tag.top_tags(guardian: scope)
end
def wizard_required
@ -250,9 +253,33 @@ class SiteSerializer < ApplicationSerializer
About.displayed_plugin_stat_groups
end
SIDEBAR_TOP_TAGS_TO_SHOW = 5
def navigation_menu_site_top_tags
if top_tags.present?
tag_names = top_tags[0...SIDEBAR_TOP_TAGS_TO_SHOW]
serialized = serialize_tags(Tag.where(name: tag_names))
# Ensures order of top tags is preserved
serialized.sort_by { |tag| tag_names.index(tag[:name]) }
else
[]
end
end
def include_navigation_menu_site_top_tags?
!SiteSetting.legacy_navigation_menu? && SiteSetting.tagging_enabled
end
def anonymous_default_navigation_menu_tags
@anonymous_default_navigation_menu_tags ||=
SiteSetting.default_navigation_menu_tags.split("|") - DiscourseTagging.hidden_tag_names(scope)
begin
tag_names =
SiteSetting.default_navigation_menu_tags.split("|") -
DiscourseTagging.hidden_tag_names(scope)
serialize_tags(Tag.where(name: tag_names))
end
end
def include_anonymous_default_navigation_menu_tags?

3
spec/requests/api/schemas/json/site_response.json
View File

@ -800,6 +800,9 @@
},
"denied_emojis" : {
"type": "array"
},
"navigation_menu_site_top_tags": {
"type": "array"
}
},
"required": [

69
spec/serializers/site_serializer_spec.rb
View File

@ -133,9 +133,10 @@ RSpec.describe SiteSerializer do
describe "#anonymous_default_navigation_menu_tags" do
fab!(:user) { Fabricate(:user) }
fab!(:tag) { Fabricate(:tag, name: "dev") }
fab!(:tag) { Fabricate(:tag, name: "dev", description: "some description") }
fab!(:tag2) { Fabricate(:tag, name: "random") }
fab!(:hidden_tag) { Fabricate(:tag, name: "secret") }
fab!(:staff_tag_group) do
Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: [hidden_tag.name])
end
@ -177,7 +178,13 @@ RSpec.describe SiteSerializer do
it "includes only tags user can see in the serialised object when user is anonymous" do
serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json
expect(serialized[:anonymous_default_navigation_menu_tags]).to eq(%w[dev random])
expect(serialized[:anonymous_default_navigation_menu_tags]).to eq(
[
{ name: "dev", description: "some description", pm_only: false },
{ name: "random", description: tag2.description, pm_only: false },
],
)
end
end
@ -299,6 +306,64 @@ RSpec.describe SiteSerializer do
end
end
describe "#navigation_menu_site_top_tags" do
fab!(:tag1) do
Fabricate(:tag, name: "tag 1").tap { |tag| Fabricate.times(2, :topic, tags: [tag]) }
end
fab!(:tag2) do
Fabricate(:tag, name: "tag 2").tap { |tag| Fabricate.times(1, :topic, tags: [tag]) }
end
fab!(:tag3) do
Fabricate(:tag, name: "tag 3").tap { |tag| Fabricate.times(3, :topic, tags: [tag]) }
end
fab!(:hidden_tag) do
Fabricate(:tag, name: "tag 4").tap { |tag| Fabricate.times(4, :topic, tags: [tag]) }
end
fab!(:staff_tag_group) do
Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: [hidden_tag.name])
end
it "should return the site's top tags as the default tags for sidebar" do
serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json
expect(serialized[:navigation_menu_site_top_tags]).to eq(
[
{ name: tag3.name, description: tag2.description, pm_only: false },
{ name: tag1.name, description: tag1.description, pm_only: false },
{ name: tag2.name, description: tag3.description, pm_only: false },
],
)
end
it "should not be serialized if `navigation_menu` site setting is set to `legacy`" do
SiteSetting.set(:navigation_menu, "legacy")
serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json
expect(serialized[:navigation_menu_site_top_tags]).to eq(nil)
end
it "should not be serialized if `tagging_enabled` site setting is set to false" do
SiteSetting.set(:tagging_enabled, false)
serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json
expect(serialized[:navigation_menu_site_top_tags]).to eq(nil)
end
it "should return an empty array if site has no top tags" do
Tag.delete_all
serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json
expect(serialized[:navigation_menu_site_top_tags]).to eq([])
end
end
describe "#whispers_allowed_groups_names" do
fab!(:admin) { Fabricate(:admin) }
fab!(:allowed_user) { Fabricate(:user) }

12
spec/support/user_sidebar_serializer_attributes.rb
View File

@ -46,7 +46,7 @@ RSpec.shared_examples "User Sidebar Serializer Attributes" do |serializer_klass|
end
describe "#sidebar_tags" do
fab!(:tag) { Fabricate(:tag, name: "foo") }
fab!(:tag) { Fabricate(:tag, name: "foo", description: "foo tag") }
fab!(:pm_tag) do
Fabricate(:tag, name: "bar", pm_topic_count: 5, staff_topic_count: 0, public_topic_count: 0)
end
@ -89,8 +89,8 @@ RSpec.shared_examples "User Sidebar Serializer Attributes" do |serializer_klass|
json = serializer.as_json
expect(json[:sidebar_tags]).to contain_exactly(
{ name: tag.name, pm_only: false },
{ name: pm_tag.name, pm_only: true },
{ name: tag.name, pm_only: false, description: tag.description },
{ name: pm_tag.name, pm_only: true, description: nil },
)
user.update!(admin: true)
@ -98,9 +98,9 @@ RSpec.shared_examples "User Sidebar Serializer Attributes" do |serializer_klass|
json = serializer.as_json
expect(json[:sidebar_tags]).to contain_exactly(
{ name: tag.name, pm_only: false },
{ name: pm_tag.name, pm_only: true },
{ name: hidden_tag.name, pm_only: false },
{ name: tag.name, pm_only: false, description: tag.description },
{ name: pm_tag.name, pm_only: true, description: nil },
{ name: hidden_tag.name, pm_only: false, description: nil },
)
end
end

10
spec/system/page_objects/components/navigation_menu/base.rb
View File

@ -25,6 +25,7 @@ module PageObjects
def has_no_section_link?(name, href: nil, active: false)
section_link_present?(name, href: href, active: active, present: false)
end
def has_section?(name)
has_css?(".sidebar-sections [data-section-name='#{name.parameterize}']")
end
@ -61,6 +62,15 @@ module PageObjects
expect(tag_section_links.map(&:text)).to eq(tag_names)
end
def has_tag_section_link_with_title?(tag, title)
section_link =
find(
".sidebar-section[data-section-name='tags'] .sidebar-section-link-wrapper[data-tag-name='#{tag.name}'] .sidebar-section-link",
)
expect(section_link["title"]).to eq(title)
end
def primary_section_links(slug)
all("[data-section-name='#{slug}'] .sidebar-section-link-wrapper").map(&:text)
end

55
spec/system/viewing_sidebar_as_anonymous_user_spec.rb
View File

@ -1,33 +1,37 @@
# frozen_string_literal: true
describe "Viewing sidebar as anonymous user", type: :system do
fab!(:tag1) do
Fabricate(:tag, name: "tag 1").tap { |tag| Fabricate.times(1, :topic, tags: [tag]) }
end
fab!(:tag2) do
Fabricate(:tag, name: "tag 2").tap { |tag| Fabricate.times(2, :topic, tags: [tag]) }
end
fab!(:tag3) do
Fabricate(:tag, name: "tag 3").tap { |tag| Fabricate.times(3, :topic, tags: [tag]) }
end
fab!(:tag4) do
Fabricate(:tag, name: "tag 4").tap { |tag| Fabricate.times(2, :topic, tags: [tag]) }
end
fab!(:tag5) do
Fabricate(:tag, name: "tag 5").tap { |tag| Fabricate.times(2, :topic, tags: [tag]) }
end
fab!(:tag6) do
Fabricate(:tag, name: "tag 6").tap { |tag| Fabricate.times(1, :topic, tags: [tag]) }
end
let(:sidebar) { PageObjects::Components::NavigationMenu::Sidebar.new }
describe "when viewing the tags section" do
fab!(:tag1) do
Fabricate(:tag, name: "tag 1", description: "tag 1 description <script>").tap do |tag|
Fabricate.times(1, :topic, tags: [tag])
end
end
fab!(:tag2) do
Fabricate(:tag, name: "tag 2").tap { |tag| Fabricate.times(2, :topic, tags: [tag]) }
end
fab!(:tag3) do
Fabricate(:tag, name: "tag 3", description: "tag 3 description").tap do |tag|
Fabricate.times(3, :topic, tags: [tag])
end
end
fab!(:tag4) do
Fabricate(:tag, name: "tag 4").tap { |tag| Fabricate.times(2, :topic, tags: [tag]) }
end
fab!(:tag5) do
Fabricate(:tag, name: "tag 5").tap { |tag| Fabricate.times(2, :topic, tags: [tag]) }
end
fab!(:tag6) do
Fabricate(:tag, name: "tag 6").tap { |tag| Fabricate.times(1, :topic, tags: [tag]) }
end
it "should not display the tags section when tagging has been disabled" do
SiteSetting.tagging_enabled = false
@ -50,6 +54,7 @@ describe "Viewing sidebar as anonymous user", type: :system do
expect(sidebar).to have_tags_section
expect(sidebar).to have_all_tags_section_link
expect(sidebar).to have_tag_section_links([tag3, tag2, tag4, tag5, tag1])
expect(sidebar).to have_tag_section_link_with_title(tag1, "tag 1 description &lt;script&gt;")
end
it "should display the site's top tags when `default_navigation_menu_tags` site setting has been set but the tags configured are hidden to the user" do
@ -61,6 +66,7 @@ describe "Viewing sidebar as anonymous user", type: :system do
expect(sidebar).to have_tags_section
expect(sidebar).to have_all_tags_section_link
expect(sidebar).to have_tag_section_links([tag3, tag2, tag4, tag1, tag6])
expect(sidebar).to have_tag_section_link_with_title(tag1, "tag 1 description &lt;script&gt;")
end
it "should display the tags configured in `default_navigation_menu_tags` site setting when it has been set" do
@ -71,6 +77,7 @@ describe "Viewing sidebar as anonymous user", type: :system do
expect(sidebar).to have_tags_section
expect(sidebar).to have_all_tags_section_link
expect(sidebar).to have_tag_section_links([tag3, tag4])
expect(sidebar).to have_tag_section_link_with_title(tag3, "tag 3 description")
end
end
end

76
spec/system/viewing_sidebar_spec.rb
View File

@ -1,6 +1,6 @@
# frozen_string_literal: true
describe "Viewing sidebar", type: :system do
describe "Viewing sidebar as logged in user", type: :system do
fab!(:admin) { Fabricate(:admin) }
fab!(:user) { Fabricate(:user) }
fab!(:category_sidebar_section_link) { Fabricate(:category_sidebar_section_link, user: user) }
@ -107,4 +107,78 @@ describe "Viewing sidebar", type: :system do
)
end
end
describe "when viewing the tags section" do
fab!(:tag1) do
Fabricate(:tag, name: "tag 1", description: "tag 1 description <script>").tap do |tag|
Fabricate.times(1, :topic, tags: [tag])
end
end
fab!(:tag2) do
Fabricate(:tag, name: "tag 2").tap { |tag| Fabricate.times(2, :topic, tags: [tag]) }
end
fab!(:tag3) do
Fabricate(:tag, name: "tag 3", description: "tag 3 description").tap do |tag|
Fabricate.times(3, :topic, tags: [tag])
end
end
fab!(:tag4) do
Fabricate(:tag, name: "tag 4").tap { |tag| Fabricate.times(2, :topic, tags: [tag]) }
end
fab!(:tag5) do
Fabricate(:tag, name: "tag 5").tap { |tag| Fabricate.times(2, :topic, tags: [tag]) }
end
fab!(:tag6) do
Fabricate(:tag, name: "tag 6").tap { |tag| Fabricate.times(1, :topic, tags: [tag]) }
end
it "should not display the tags section when tagging is disabled" do
SiteSetting.tagging_enabled = false
visit("/latest")
expect(sidebar).to be_visible
expect(sidebar).to have_no_tags_section
end
it "should not display the tags section when there are no tags that a user can see" do
Tag.delete_all
visit("/latest")
expect(sidebar).to be_visible
expect(sidebar).to have_no_tags_section
end
it "should display the site's top tags in the tags section when user has not configured any tags" do
visit("/latest")
expect(sidebar).to be_visible
expect(sidebar).to have_tags_section
expect(sidebar).to have_tag_section_links([tag3, tag2, tag4, tag5, tag1])
expect(sidebar).to have_tag_section_link_with_title(tag3, "tag 3 description")
expect(sidebar).to have_tag_section_link_with_title(tag1, "tag 1 description &lt;script&gt;")
expect(sidebar).to have_all_tags_section_link
end
it "should display the tags configured by the user in alphabetical order" do
Fabricate(:sidebar_section_link, linkable: tag3, user: user)
Fabricate(:sidebar_section_link, linkable: tag1, user: user)
Fabricate(:sidebar_section_link, linkable: tag2, user: user)
visit("/latest")
expect(sidebar).to be_visible
expect(sidebar).to have_tags_section
expect(sidebar).to have_tag_section_links([tag1, tag2, tag3])
expect(sidebar).to have_tag_section_link_with_title(tag3, "tag 3 description")
expect(sidebar).to have_tag_section_link_with_title(tag1, "tag 1 description &lt;script&gt;")
expect(sidebar).to have_all_tags_section_link
end
end
end