From d1d760ae7b43602a1c46fedcca3ae6e2dce45859 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Wed, 3 Aug 2022 14:47:03 +0800 Subject: [PATCH] UX: Move links in Sidebar footer under community section (#17774) Now that we have a "more..." links drawer, we can move some of the links in footer into the links drawer. The footer itself does not have much horizontal or vertical space for us to work with and hence limits the amount of links which we can add to it. --- .../components/sidebar/community-section.js | 19 ++- .../app/components/sidebar/section-link.js | 16 +++ .../community-section/about-section-link.js | 21 ++++ .../community-section/admin-section-link.js | 21 ++++ .../community-section/base-section-link.js | 14 ++- .../community-section/faq-section-link.js | 25 ++++ .../components/sidebar/community-section.hbs | 1 + .../templates/components/sidebar/footer.hbs | 12 -- .../components/sidebar/more-section-links.hbs | 1 + .../components/sidebar/section-link.hbs | 118 ++++++++++-------- .../sidebar-community-section-test.js | 73 ++++++++++- .../tests/acceptance/sidebar-test.js | 30 +---- 12 files changed, 252 insertions(+), 99 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/lib/sidebar/community-section/about-section-link.js create mode 100644 app/assets/javascripts/discourse/app/lib/sidebar/community-section/admin-section-link.js create mode 100644 app/assets/javascripts/discourse/app/lib/sidebar/community-section/faq-section-link.js diff --git a/app/assets/javascripts/discourse/app/components/sidebar/community-section.js b/app/assets/javascripts/discourse/app/components/sidebar/community-section.js index 1ec1ca63c0..16b68f9ef1 100644 --- a/app/assets/javascripts/discourse/app/components/sidebar/community-section.js +++ b/app/assets/javascripts/discourse/app/components/sidebar/community-section.js @@ -8,6 +8,9 @@ import TrackedSectionLink from "discourse/lib/sidebar/community-section/tracked- import MyPostsSectionLink from "discourse/lib/sidebar/community-section/my-posts-section-link"; import GroupsSectionLink from "discourse/lib/sidebar/community-section/groups-section-link"; import UsersSectionLink from "discourse/lib/sidebar/community-section/users-section-link"; +import AboutSectionLink from "discourse/lib/sidebar/community-section/about-section-link"; +import FAQSectionLink from "discourse/lib/sidebar/community-section/faq-section-link"; +import AdminSectionLink from "discourse/lib/sidebar/community-section/admin-section-link"; import { inject as service } from "@ember/service"; import { action } from "@ember/object"; @@ -19,7 +22,14 @@ const MAIN_SECTION_LINKS = [ MyPostsSectionLink, ]; -const MORE_SECTION_LINKS = [GroupsSectionLink, UsersSectionLink]; +const ADMIN_MAIN_SECTION_LINKS = [AdminSectionLink]; + +const MORE_SECTION_LINKS = [ + GroupsSectionLink, + UsersSectionLink, + AboutSectionLink, + FAQSectionLink, +]; export default class SidebarCommunitySection extends GlimmerComponent { @service router; @@ -31,11 +41,16 @@ export default class SidebarCommunitySection extends GlimmerComponent { currentUser: this.currentUser, appEvents: this.appEvents, router: this.router, + siteSettings: this.siteSettings, }); } ); - sectionLinks = MAIN_SECTION_LINKS.map((sectionLinkClass) => { + #mainSectionLinks = this.currentUser.staff + ? [...MAIN_SECTION_LINKS, ...ADMIN_MAIN_SECTION_LINKS] + : [...MAIN_SECTION_LINKS]; + + sectionLinks = this.#mainSectionLinks.map((sectionLinkClass) => { return new sectionLinkClass({ topicTrackingState: this.topicTrackingState, currentUser: this.currentUser, diff --git a/app/assets/javascripts/discourse/app/components/sidebar/section-link.js b/app/assets/javascripts/discourse/app/components/sidebar/section-link.js index 6e638f8a85..32dc9eae3b 100644 --- a/app/assets/javascripts/discourse/app/components/sidebar/section-link.js +++ b/app/assets/javascripts/discourse/app/components/sidebar/section-link.js @@ -8,6 +8,22 @@ export default class SectionLink extends GlimmerComponent { } } + get classNames() { + return `${this.args.class} sidebar-section-link sidebar-section-link-${this.args.linkName}`; + } + + get models() { + if (this.args.model) { + return [this.args.model]; + } + + if (this.args.models) { + return this.args.models; + } + + return []; + } + get prefixCSS() { const color = this.args.prefixColor; diff --git a/app/assets/javascripts/discourse/app/lib/sidebar/community-section/about-section-link.js b/app/assets/javascripts/discourse/app/lib/sidebar/community-section/about-section-link.js new file mode 100644 index 0000000000..0fb4bd3dfb --- /dev/null +++ b/app/assets/javascripts/discourse/app/lib/sidebar/community-section/about-section-link.js @@ -0,0 +1,21 @@ +import I18n from "I18n"; + +import BaseSectionLink from "discourse/lib/sidebar/community-section/base-section-link"; + +export default class AboutSectionLink extends BaseSectionLink { + get name() { + return "about"; + } + + get route() { + return "about"; + } + + get title() { + return I18n.t("about.simple_title"); + } + + get text() { + return I18n.t("about.simple_title"); + } +} diff --git a/app/assets/javascripts/discourse/app/lib/sidebar/community-section/admin-section-link.js b/app/assets/javascripts/discourse/app/lib/sidebar/community-section/admin-section-link.js new file mode 100644 index 0000000000..48eedc9445 --- /dev/null +++ b/app/assets/javascripts/discourse/app/lib/sidebar/community-section/admin-section-link.js @@ -0,0 +1,21 @@ +import I18n from "I18n"; + +import BaseSectionLink from "discourse/lib/sidebar/community-section/base-section-link"; + +export default class AdminSectionLink extends BaseSectionLink { + get name() { + return "admin"; + } + + get route() { + return "admin"; + } + + get title() { + return I18n.t("admin_title"); + } + + get text() { + return I18n.t("admin_title"); + } +} diff --git a/app/assets/javascripts/discourse/app/lib/sidebar/community-section/base-section-link.js b/app/assets/javascripts/discourse/app/lib/sidebar/community-section/base-section-link.js index 0d91a6959e..7bb26101dd 100644 --- a/app/assets/javascripts/discourse/app/lib/sidebar/community-section/base-section-link.js +++ b/app/assets/javascripts/discourse/app/lib/sidebar/community-section/base-section-link.js @@ -2,11 +2,18 @@ * Base class representing a sidebar topics section link interface. */ export default class BaseSectionLink { - constructor({ topicTrackingState, currentUser, appEvents, router } = {}) { + constructor({ + topicTrackingState, + currentUser, + appEvents, + router, + siteSettings, + } = {}) { this.router = router; this.topicTrackingState = topicTrackingState; this.currentUser = currentUser; this.appEvents = appEvents; + this.siteSettings = siteSettings; } /** @@ -28,6 +35,11 @@ export default class BaseSectionLink { this._notImplemented(); } + /** + * @returns {string} href attribute for the link. This property will take precedence over the `route` property when set. + */ + get href() {} + /** * @returns {Object} Model for component. See https://api.emberjs.com/ember/release/classes/Ember.Templates.components/methods/LinkTo?anchor=LinkTo */ diff --git a/app/assets/javascripts/discourse/app/lib/sidebar/community-section/faq-section-link.js b/app/assets/javascripts/discourse/app/lib/sidebar/community-section/faq-section-link.js new file mode 100644 index 0000000000..03078c4ff5 --- /dev/null +++ b/app/assets/javascripts/discourse/app/lib/sidebar/community-section/faq-section-link.js @@ -0,0 +1,25 @@ +import I18n from "I18n"; + +import BaseSectionLink from "discourse/lib/sidebar/community-section/base-section-link"; + +export default class FAQSectionLink extends BaseSectionLink { + get name() { + return "faq"; + } + + get route() { + return "faq"; + } + + get href() { + return this.siteSettings.faq_url; + } + + get title() { + return I18n.t("faq"); + } + + get text() { + return I18n.t("faq"); + } +} diff --git a/app/assets/javascripts/discourse/app/templates/components/sidebar/community-section.hbs b/app/assets/javascripts/discourse/app/templates/components/sidebar/community-section.hbs index 8042f0e4bd..405ce9729c 100644 --- a/app/assets/javascripts/discourse/app/templates/components/sidebar/community-section.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/sidebar/community-section.hbs @@ -11,6 +11,7 @@ {{#each this.sectionLinks as |sectionLink|}}
-
- - {{i18n "about.simple_title"}} - - - {{#if this.currentUser.staff}} - - {{i18n "admin_title"}} - - {{/if}} -
-
- - {{#if @prefixValue }} - - {{#if (eq @prefixType "image")}} - - {{/if}} + {{#if @href}} + + + {{@content}} + + + {{else}} + + {{#if @prefixValue }} + + {{#if (eq @prefixType "image")}} + + {{/if}} - {{#if (eq @prefixType "text")}} - {{@prefixValue}} - {{/if}} + {{#if (eq @prefixType "text")}} + {{@prefixValue}} + {{/if}} - {{#if (eq @prefixType "icon")}} - {{d-icon @prefixValue class="prefix-icon"}} - {{/if}} + {{#if (eq @prefixType "icon")}} + {{d-icon @prefixValue class="prefix-icon"}} + {{/if}} - {{#if @prefixBadge}} - {{d-icon @prefixBadge class="prefix-badge"}} - {{/if}} + {{#if @prefixBadge}} + {{d-icon @prefixBadge class="prefix-badge"}} + {{/if}} + + {{/if}} + + + {{@content}} + + + {{#if @badgeText}} + + {{@badgeText}} + + {{/if}} + + {{#if @suffixValue}} + + {{#if (eq @suffixType "icon")}} + {{d-icon @suffixValue}} + {{/if}} + + {{/if}} + + + {{#if @hoverValue}} + + {{/if}} - - - {{@content}} - - - {{#if @badgeText}} - - {{@badgeText}} - - {{/if}} - - {{#if @suffixValue}} - - {{#if (eq @suffixType "icon")}} - {{d-icon @suffixValue}} - {{/if}} - - {{/if}} - - - {{#if @hoverValue}} - - - {{/if}}
diff --git a/app/assets/javascripts/discourse/tests/acceptance/sidebar-community-section-test.js b/app/assets/javascripts/discourse/tests/acceptance/sidebar-community-section-test.js index f71392f0ab..45d1832003 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/sidebar-community-section-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/sidebar-community-section-test.js @@ -1,6 +1,11 @@ import I18n from "I18n"; import { test } from "qunit"; -import { click, currentURL, visit } from "@ember/test-helpers"; +import { + click, + currentRouteName, + currentURL, + visit, +} from "@ember/test-helpers"; import { acceptance, count, @@ -8,6 +13,7 @@ import { loggedInUser, publishToMessageBus, query, + updateCurrentUser, } from "discourse/tests/helpers/qunit-helpers"; import topicFixtures from "discourse/tests/fixtures/discovery-fixtures"; import { cloneJSON } from "discourse-common/lib/object"; @@ -293,6 +299,71 @@ acceptance("Sidebar - Community Section", function (needs) { ); }); + test("navigating to about from sidebar", async function (assert) { + await visit("/"); + + await click( + ".sidebar-section-community .sidebar-more-section-links-details-summary" + ); + + await click(".sidebar-section-community .sidebar-section-link-about"); + + assert.strictEqual( + currentURL(), + "/about", + "navigates to about route correctly" + ); + }); + + test("navigating to FAQ from sidebar", async function (assert) { + await visit("/"); + + await click( + ".sidebar-section-community .sidebar-more-section-links-details-summary" + ); + + await click(".sidebar-section-community .sidebar-section-link-faq"); + + assert.strictEqual( + currentURL(), + "/faq", + "navigates to faq route correctly" + ); + }); + + test("navigating to custom FAQ URL from sidebar", async function (assert) { + this.siteSettings.faq_url = "http://some.faq.url"; + + await visit("/"); + + await click( + ".sidebar-section-community .sidebar-more-section-links-details-summary" + ); + + assert.strictEqual( + query(".sidebar-section-community .sidebar-section-link-faq").href, + "http://some.faq.url/", + "href attribute is set to custom FAQ URL on the section link" + ); + }); + + test("navigating to admin from sidebar", async function (assert) { + await visit("/"); + await click(".sidebar-section-community .sidebar-section-link-admin"); + + assert.strictEqual(currentRouteName(), "admin.dashboard.general"); + }); + + test("admin section link is not shown to non-staff users", async function (assert) { + updateCurrentUser({ admin: false, moderator: false }); + + await visit("/"); + + assert.notOk( + exists(".sidebar-section-community .sidebar-section-link-admin") + ); + }); + test("clicking on my posts link", async function (assert) { await visit("/t/280"); await click(".sidebar-section-community .sidebar-section-link-my-posts"); diff --git a/app/assets/javascripts/discourse/tests/acceptance/sidebar-test.js b/app/assets/javascripts/discourse/tests/acceptance/sidebar-test.js index c6cd5c3919..3388ce6e07 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/sidebar-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/sidebar-test.js @@ -1,12 +1,8 @@ import I18n from "I18n"; import { test } from "qunit"; -import { click, currentRouteName, visit } from "@ember/test-helpers"; -import { - acceptance, - exists, - updateCurrentUser, -} from "discourse/tests/helpers/qunit-helpers"; +import { click, visit } from "@ember/test-helpers"; +import { acceptance, exists } from "discourse/tests/helpers/qunit-helpers"; acceptance("Sidebar - Anon User", function () { // Don't show sidebar for anon user until we know what we want to display @@ -98,13 +94,6 @@ acceptance( enable_sidebar: true, }); - test("navigating to about route using sidebar", async function (assert) { - await visit("/"); - await click(".sidebar-footer-link-about"); - - assert.strictEqual(currentRouteName(), "about"); - }); - test("viewing keyboard shortcuts using sidebar", async function (assert) { await visit("/"); await click( @@ -119,21 +108,6 @@ acceptance( ); }); - test("navigating to admin route using sidebar", async function (assert) { - await visit("/"); - await click(".sidebar-footer-link-admin"); - - assert.strictEqual(currentRouteName(), "admin.dashboard.general"); - }); - - test("admin link is not shown in sidebar for non-admin user", async function (assert) { - updateCurrentUser({ admin: false, moderator: false }); - - await visit("/"); - - assert.notOk(exists(".sidebar-footer-link-admin")); - }); - test("sidebar is disabled on wizard route", async function (assert) { await visit("/wizard");