DEV: Various A11Y improvements for the new user menu (#18288)

This commit includes various accessibility improvements for the new user menu:

* Add `title` attributes to the user menu tabs
* Properly label lists (by adding `aria-labelledby` to `<ul>` elements) for screen readers
* Change the user menu structure so that the tabs come before the content panel in the DOM, but use CSS to reverse them visually.
  Normally, changing the order of elements via CSS is bad for accessibility, but I believe this is one of the rare scenarios where it [makes sense](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Flexible_Box_Layout/Ordering_Flex_Items#use_cases_for_order). Prior to this change, if you want to reach the first notification item after you select a tab using the keyboard, you have to hit <kbd>ctrl</kbd>+<kbd>tab</kbd> because the notifications list is before the tabs list. However, with this change, <kbd>tab</kbd> will move you to the first item in the list after you select a tab using your keyboard.
* Aria-hide the unread notifications badge/count on the tabs because the `title` attribute on the tab indicates the unread count.
* Add some tests.

app/assets/javascripts/discourse/app/components/user-menu/items-list.hbs

@@ -3,7 +3,7 @@
     <div class="spinner"></div>
   </div>
 {{else if this.items.length}}
-  <ul>
+  <ul aria-labelledby={{@ariaLabelledby}}>
     {{#each this.items as |item|}}
       <UserMenu::MenuItem @item={{item}} @closeUserMenu={{@closeUserMenu}}/>
     {{/each}}

app/assets/javascripts/discourse/app/components/user-menu/menu-tab.hbs

@@ -4,6 +4,7 @@
   class={{this.classNames}}
   id={{this.id}}
   tabindex={{this.tabIndex}}
+  title={{@tab.title}}
   aria-selected={{if this.isActive "true" "false"}}
   aria-controls={{this.ariaControls}}
   data-tab-number={{@tab.position}}
@@ -11,5 +12,8 @@
   {{!-- template-lint-disable require-context-role --}}
 >
   {{d-icon @tab.icon}}
+  {{#if @tab.count}}
+    <span aria-hidden="true" class="badge-notification">{{@tab.count}}</span>
+  {{/if}}
   {{yield}}
 </button>

app/assets/javascripts/discourse/app/components/user-menu/menu.hbs

@@ -1,25 +1,14 @@
 <div class="user-menu revamped menu-panel drop-down" data-max-width="320" {{did-insert this.triggerRenderedAppEvent}}>
   <div class="panel-body">
     <div class="panel-body-contents">
-      <div
-        id={{concat "quick-access-" this.currentTabId}}
-        class="quick-access-panel"
-        tabindex="-1"
-        aria-labelledby={{concat "user-menu-button-" this.currentTabId}}>
-        {{component this.currentPanelComponent closeUserMenu=@closeUserMenu filterByTypes=this.currentNotificationTypes}}
-      </div>
       <div class="menu-tabs-container" role="tablist" aria-orientation="vertical" aria-label={{i18n "user_menu.sr_menu_tabs"}}>
-        <div class="top-tabs tabs-list">
+        <div class="top-tabs tabs-list" {{did-insert this.focusFirstTab}}>
           {{#each this.topTabs as |tab|}}
             <UserMenu::MenuTab
               @tab={{tab}}
               @currentTabId={{this.currentTabId}}
               @onTabClick={{fn this.handleTabClick tab}}
-            >
-              {{#if tab.count}}
-                <span class="badge-notification">{{tab.count}}</span>
-              {{/if}}
-            </UserMenu::MenuTab>
+            />
           {{/each}}
         </div>
         <div class="bottom-tabs tabs-list">
@@ -32,6 +21,17 @@
           {{/each}}
         </div>
       </div>
+      <div
+        id={{concat "quick-access-" this.currentTabId}}
+        class="quick-access-panel"
+        tabindex="-1">
+        {{component
+          this.currentPanelComponent
+          closeUserMenu=@closeUserMenu
+          filterByTypes=this.currentNotificationTypes
+          ariaLabelledby=(concat "user-menu-button-" this.currentTabId)
+        }}
+      </div>
     </div>
   </div>
 </div>

app/assets/javascripts/discourse/app/components/user-menu/menu.js

@@ -257,7 +257,7 @@ const CORE_OTHER_NOTIFICATIONS_TAB = class extends UserMenuTab {
   }
 
   get id() {
-    return "other";
+    return "other-notifications";
   }
 
   get icon() {
@@ -378,4 +378,9 @@ export default class UserMenu extends Component {
   triggerRenderedAppEvent() {
     this.appEvents.trigger("user-menu:rendered");
   }
+
+  @action
+  focusFirstTab(topTabsContainerElement) {
+    topTabsContainerElement.querySelector(".btn.active")?.focus();
+  }
 }

app/assets/javascripts/discourse/app/components/user-menu/profile-tab-content.hbs

@@ -1,4 +1,4 @@
-<ul>
+<ul aria-labelledby={{@ariaLabelledby}}>
   {{#if this.siteSettings.enable_user_status}}
     <li class="set-user-status">
       <DButton @class="btn-flat profile-tab-btn" @action={{this.setUserStatusClick}}>

app/assets/javascripts/discourse/app/lib/user-menu/notification-item.js

@@ -65,8 +65,9 @@ export default class UserMenuNotificationItem extends UserMenuBaseItem {
     if (!this.notification.read) {
       this.notification.set("read", true);
 
-      const groupedUnreadNotifications =
-        this.currentUser.grouped_unread_notifications;
+      const groupedUnreadNotifications = {
+        ...this.currentUser.grouped_unread_notifications,
+      };
       const unreadCount =
         groupedUnreadNotifications &&
         groupedUnreadNotifications[this.notification.notification_type];

app/assets/javascripts/discourse/app/lib/user-menu/tab.js

@@ -1,3 +1,5 @@
+import I18n from "I18n";
+
 /**
  * abstract class representing a tab in the user menu
  */
@@ -22,6 +24,26 @@ export default class UserMenuTab {
     return 0;
   }
 
+  /**
+   * @returns {string} title attribute for the tab element in the DOM
+   */
+  get title() {
+    const id = this.id.replaceAll(/-/g, "_");
+    const count = this.count;
+    let key;
+    if (this.count) {
+      key = `user_menu.tabs.${id}_with_unread`;
+    } else {
+      key = `user_menu.tabs.${id}`;
+    }
+    // the lookup method returns undefined if the key doesn't exist.
+    // this ensures that don't use the "missing translation" string as the
+    // title for tabs that don't define title in the yml files.
+    if (I18n.lookup(key)) {
+      return I18n.t(key, { count });
+    }
+  }
+
   /**
    * @returns {string} Dasherized version of the component name that should be rendered in the panel area when the tab is active.
    */

app/assets/javascripts/discourse/tests/acceptance/user-menu-test.js

@@ -51,6 +51,41 @@ acceptance("User menu", function (needs) {
     requestHeaders = {};
   });
 
+  test("notifications panel has a11y attributes", async function (assert) {
+    await visit("/");
+    await click(".d-header-icons .current-user");
+    const panel = query("#quick-access-all-notifications");
+    assert.strictEqual(panel.getAttribute("tabindex"), "-1");
+    assert.strictEqual(
+      panel.querySelector("ul").getAttribute("aria-labelledby"),
+      "user-menu-button-all-notifications"
+    );
+  });
+
+  test("mentions notifications panel has a11y attributes", async function (assert) {
+    await visit("/");
+    await click(".d-header-icons .current-user");
+    await click("#user-menu-button-mentions");
+    const panel = query("#quick-access-mentions");
+    assert.strictEqual(panel.getAttribute("tabindex"), "-1");
+    assert.strictEqual(
+      panel.querySelector("ul").getAttribute("aria-labelledby"),
+      "user-menu-button-mentions"
+    );
+  });
+
+  test("profile panel has a11y attributes", async function (assert) {
+    await visit("/");
+    await click(".d-header-icons .current-user");
+    await click("#user-menu-button-profile");
+    const panel = query("#quick-access-profile");
+    assert.strictEqual(panel.getAttribute("tabindex"), "-1");
+    assert.strictEqual(
+      panel.querySelector("ul").getAttribute("aria-labelledby"),
+      "user-menu-button-profile"
+    );
+  });
+
   test("clicking on an unread notification", async function (assert) {
     await visit("/");
     await click(".d-header-icons .current-user");
@@ -133,6 +168,72 @@ acceptance("User menu", function (needs) {
     );
   });
 
+  test("tabs have title attributes", async function (assert) {
+    withPluginApi("0.1", (api) => {
+      api.registerUserMenuTab((UserMenuTab) => {
+        return class extends UserMenuTab {
+          get id() {
+            return "tiny-tab-1";
+          }
+
+          get count() {
+            return this.currentUser.get("unread_high_priority_notifications");
+          }
+
+          get icon() {
+            return "wrench";
+          }
+
+          get panelComponent() {
+            return "d-button";
+          }
+
+          get title() {
+            return `Custom title: ${this.count}`;
+          }
+        };
+      });
+    });
+
+    const expectedTitles = {
+      "user-menu-button-all-notifications": I18n.t(
+        "user_menu.tabs.all_notifications"
+      ),
+      "user-menu-button-replies": I18n.t("user_menu.tabs.replies_with_unread", {
+        count: 2,
+      }),
+      "user-menu-button-mentions": I18n.t("user_menu.tabs.mentions"),
+      "user-menu-button-likes": I18n.t("user_menu.tabs.likes"),
+      "user-menu-button-watching": I18n.t("user_menu.tabs.watching"),
+      "user-menu-button-messages": I18n.t("user_menu.tabs.messages"),
+      "user-menu-button-bookmarks": I18n.t("user_menu.tabs.bookmarks"),
+      "user-menu-button-tiny-tab-1": "Custom title: 73",
+      "user-menu-button-review-queue": I18n.t("user_menu.tabs.review_queue"),
+      "user-menu-button-other-notifications": I18n.t(
+        "user_menu.tabs.other_notifications"
+      ),
+      "user-menu-button-profile": I18n.t("user_menu.tabs.profile"),
+    };
+    await visit("/");
+    await click(".d-header-icons .current-user");
+    for (const [key, title] of Object.entries(expectedTitles)) {
+      assert.strictEqual(
+        query(`#${key}`).title,
+        title,
+        `${key} tab has the right title`
+      );
+    }
+
+    await publishToMessageBus(`/notification/${loggedInUser().id}`, {
+      unread_high_priority_notifications: 22,
+    });
+    assert.strictEqual(
+      query("#user-menu-button-tiny-tab-1").title,
+      "Custom title: 22",
+      "tabs titles can update dynamically"
+    );
+  });
+
   test("tabs added via the plugin API", async function (assert) {
     withPluginApi("0.1", (api) => {
       api.registerUserMenuTab((UserMenuTab) => {
@@ -186,7 +287,7 @@ acceptance("User menu", function (needs) {
       "user-menu-button-custom-tab-1": "7",
       "user-menu-button-custom-tab-2": "8",
       "user-menu-button-review-queue": "9",
-      "user-menu-button-other": "10",
+      "user-menu-button-other-notifications": "10",
     };
 
     await visit("/");
@@ -855,19 +956,21 @@ acceptance("User menu - Dismiss button", function (needs) {
     await visit("/");
     await click(".d-header-icons .current-user");
 
-    await click("#user-menu-button-other");
-    let repliesBadgeNotification = query(
-      "#user-menu-button-other .badge-notification"
+    await click("#user-menu-button-other-notifications");
+    let othersBadgeNotification = query(
+      "#user-menu-button-other-notifications .badge-notification"
     );
     assert.strictEqual(
-      repliesBadgeNotification.textContent.trim(),
+      othersBadgeNotification.textContent.trim(),
       "4",
       "badge shows the right count"
     );
 
     await click(".user-menu .notifications-dismiss");
 
-    assert.ok(!exists("#user-menu-button-other .badge-notification"));
+    assert.ok(
+      !exists("#user-menu-button-other-notifications .badge-notification")
+    );
     assert.ok(
       markRead,
       "mark-read request is sent without a confirmation modal"

app/assets/javascripts/discourse/tests/integration/components/user-menu/menu-test.js

@@ -21,16 +21,6 @@ module("Integration | Component | user-menu", function (hooks) {
     assert.ok(notifications[2].classList.contains("liked-consolidated"));
   });
 
-  test("notifications panel has a11y attributes", async function (assert) {
-    await render(template);
-    const panel = query("#quick-access-all-notifications");
-    assert.strictEqual(panel.getAttribute("tabindex"), "-1");
-    assert.strictEqual(
-      panel.getAttribute("aria-labelledby"),
-      "user-menu-button-all-notifications"
-    );
-  });
-
   test("active tab has a11y attributes that indicate it's active", async function (assert) {
     await render(template);
     const activeTab = query(".top-tabs.tabs-list .btn.active");

app/assets/stylesheets/common/base/menu-panel.scss

@@ -142,7 +142,7 @@
 
   .panel-body-contents {
     display: flex;
-    flex-direction: row;
+    flex-direction: row-reverse;
   }
 
   .quick-access-panel {

config/locales/client.en.yml

@@ -2567,10 +2567,45 @@ en:
     view_all: "view all %{tab}"
     user_menu:
       generic_no_items: "There are no items in this list."
-      sr_menu_tabs: "Menu tabs"
+      sr_menu_tabs: "User menu tabs"
       view_all_notifications: "view all notifications"
       view_all_bookmarks: "view all bookmarks"
       view_all_messages: "view all personal messages"
+      tabs:
+        all_notifications: "All notifications"
+        replies: "Replies"
+        replies_with_unread:
+          one: "Replies - %{count} unread reply"
+          other: "Replies - %{count} unread replies"
+        mentions: "Mentions"
+        mentions_with_unread:
+          one: "Mentions - %{count} unread mention"
+          other: "Mentions - %{count} unread mentions"
+        likes: "Likes"
+        likes_with_unread:
+          one: "Likes - %{count} unread like"
+          other: "Likes - %{count} unread likes"
+        watching: "Watched topics"
+        watching_with_unread:
+          one: "Watched topics - %{count} unread watched topic"
+          other: "Watched topics - %{count} unread watched topics"
+        messages: "Personal messages"
+        messages_with_unread:
+          one: "Personal messages - %{count} unread message"
+          other: "Personal messages - %{count} unread messages"
+        bookmarks: "Bookmarks"
+        bookmarks_with_unread:
+          one: "Bookmarks - %{count} unread bookmark"
+          other: "Bookmarks - %{count} unread bookmarks"
+        review_queue: "Review queue"
+        review_queue_with_unread:
+          one: "Review queue - %{count} item needs review"
+          other: "Review queue - %{count} items need review"
+        other_notifications: "Other notifications"
+        other_notifications_with_unread:
+          one: "Other notifications - %{count} unread notification"
+          other: "Other notifications - %{count} unread notifications"
+        profile: "Profile"
       reviewable:
         view_all: "view all review items"
         queue: "Queue"