diff --git a/app/assets/javascripts/discourse/app/lib/plugin-api.js b/app/assets/javascripts/discourse/app/lib/plugin-api.js index b536e80dbba..ce4145cd5e8 100644 --- a/app/assets/javascripts/discourse/app/lib/plugin-api.js +++ b/app/assets/javascripts/discourse/app/lib/plugin-api.js @@ -737,10 +737,10 @@ class PluginApi { * example: * * api.addUserMenuGlyph({ - * label: 'awesome.label', + * title: 'awesome.label', * className: 'my-class', * icon: 'my-icon', - * href: `/some/path` + * data: { url: `/some/path` }, * }); * */ diff --git a/app/assets/javascripts/discourse/app/widgets/button.js b/app/assets/javascripts/discourse/app/widgets/button.js index bb93acdbb5a..fe5c6d93aeb 100644 --- a/app/assets/javascripts/discourse/app/widgets/button.js +++ b/app/assets/javascripts/discourse/app/widgets/button.js @@ -38,6 +38,17 @@ export const ButtonClass = { attributes.title = title; } + if (attrs.role) { + attributes["role"] = attrs.role; + } + + if (attrs.tabAttrs) { + const tab = attrs.tabAttrs; + attributes["aria-selected"] = tab["aria-selected"]; + attributes["tabindex"] = tab["tabindex"]; + attributes["aria-controls"] = tab["aria-controls"]; + } + if (attrs.disabled) { attributes.disabled = "true"; } @@ -51,11 +62,20 @@ export const ButtonClass = { return attributes; }, + _buildIcon(attrs) { + const icon = iconNode(attrs.icon, { class: attrs.iconClass }); + if (attrs["aria-label"]) { + icon.properties.attributes["role"] = "img"; + icon.properties.attributes["aria-hidden"] = false; + } + return icon; + }, + html(attrs) { const contents = []; const left = !attrs.iconRight; if (attrs.icon && left) { - contents.push(iconNode(attrs.icon, { class: attrs.iconClass })); + contents.push(this._buildIcon(attrs)); } if (attrs.label) { contents.push( @@ -75,7 +95,7 @@ export const ButtonClass = { contents.push(attrs.contents); } if (attrs.icon && !left) { - contents.push(iconNode(attrs.icon, { class: attrs.iconClass })); + contents.push(this._buildIcon(attrs)); } return contents; diff --git a/app/assets/javascripts/discourse/app/widgets/quick-access-panel.js b/app/assets/javascripts/discourse/app/widgets/quick-access-panel.js index 23145723b53..8c6f0996f2a 100644 --- a/app/assets/javascripts/discourse/app/widgets/quick-access-panel.js +++ b/app/assets/javascripts/discourse/app/widgets/quick-access-panel.js @@ -40,6 +40,15 @@ export default createWidget("quick-access-panel", { return Promise.resolve([]); }, + buildAttributes() { + const attributes = this.attrs; + attributes["aria-labelledby"] = this.key; + attributes["tabindex"] = "0"; + attributes["role"] = "tabpanel"; + + return attributes; + }, + newItemsLoaded() {}, itemHtml(item) {}, // eslint-disable-line no-unused-vars diff --git a/app/assets/javascripts/discourse/app/widgets/user-menu.js b/app/assets/javascripts/discourse/app/widgets/user-menu.js index e532b38a211..7c89f3da0c5 100644 --- a/app/assets/javascripts/discourse/app/widgets/user-menu.js +++ b/app/assets/javascripts/discourse/app/widgets/user-menu.js @@ -23,51 +23,82 @@ export function addUserMenuGlyph(glyph) { createWidget("user-menu-links", { tagName: "div.menu-links-header", + _tabAttrs(quickAccessType) { + return { + "aria-controls": `quick-access-${quickAccessType}`, + "aria-selected": "false", + tabindex: "-1", + }; + }, + + // TODO: Remove when 2.7 gets released. + _structureAsTab(extraGlyph) { + const glyph = extraGlyph; + // Assume glyph is a button if it has a data-url field. + if (!glyph.data || !glyph.data.url) { + glyph.title = glyph.label; + glyph.data = { url: glyph.href }; + + glyph.label = null; + glyph.href = null; + } + + glyph.role = "tab"; + glyph.tabAttrs = this._tabAttrs(glyph.actionParam); + + return glyph; + }, + profileGlyph() { return { - label: "user.preferences", + title: "user.preferences", className: "user-preferences-link", icon: "user", - href: `${this.attrs.path}/summary`, action: UserMenuAction.QUICK_ACCESS, actionParam: QuickAccess.PROFILE, - "aria-label": "user.preferences", + data: { url: `${this.attrs.path}/summary` }, + role: "tab", + tabAttrs: this._tabAttrs(QuickAccess.PROFILE), }; }, notificationsGlyph() { return { - label: "user.notifications", + title: "user.notifications", className: "user-notifications-link", icon: "bell", - href: `${this.attrs.path}/notifications`, action: UserMenuAction.QUICK_ACCESS, actionParam: QuickAccess.NOTIFICATIONS, - "aria-label": "user.notifications", + data: { url: `${this.attrs.path}/notifications` }, + role: "tab", + tabAttrs: this._tabAttrs(QuickAccess.NOTIFICATIONS), }; }, bookmarksGlyph() { return { + title: "user.bookmarks", action: UserMenuAction.QUICK_ACCESS, actionParam: QuickAccess.BOOKMARKS, - label: "user.bookmarks", className: "user-bookmarks-link", icon: "bookmark", - href: `${this.attrs.path}/activity/bookmarks`, + data: { url: `${this.attrs.path}/activity/bookmarks` }, "aria-label": "user.bookmarks", + role: "tab", + tabAttrs: this._tabAttrs(QuickAccess.BOOKMARKS), }; }, messagesGlyph() { return { + title: "user.private_messages", action: UserMenuAction.QUICK_ACCESS, actionParam: QuickAccess.MESSAGES, - label: "user.private_messages", className: "user-pms-link", icon: "envelope", - href: `${this.attrs.path}/messages`, - "aria-label": "user.private_messages", + data: { url: `${this.attrs.path}/messages` }, + role: "tab", + tabAttrs: this._tabAttrs(QuickAccess.MESSAGES), }; }, @@ -82,7 +113,7 @@ createWidget("user-menu-links", { if (this.isActive(glyph)) { glyph = this.markAsActive(glyph); } - return this.attach("link", $.extend(glyph, { hideLabel: true })); + return this.attach("flat-button", glyph); }, html() { @@ -94,7 +125,8 @@ createWidget("user-menu-links", { g = g(this); } if (g) { - glyphs.push(g); + const structuredGlyph = this._structureAsTab(g); + glyphs.push(structuredGlyph); } }); } @@ -108,9 +140,10 @@ createWidget("user-menu-links", { glyphs.push(this.profileGlyph()); - return h("ul.menu-links-row", [ + return h("div.menu-links-row", [ h( - "li.glyphs", + "div.glyphs", + { attributes: { "aria-label": "Menu links", role: "tablist" } }, glyphs.map((l) => this.glyphHtml(l)) ), ]); @@ -121,6 +154,7 @@ createWidget("user-menu-links", { // the full page. definition.action = null; definition.actionParam = null; + definition.url = definition.data.url; if (definition.className) { definition.className += " active"; @@ -128,6 +162,9 @@ createWidget("user-menu-links", { definition.className = "active"; } + definition.tabAttrs["tabindex"] = "0"; + definition.tabAttrs["aria-selected"] = "true"; + return definition; }, diff --git a/app/assets/javascripts/discourse/tests/integration/widgets/user-menu-test.js b/app/assets/javascripts/discourse/tests/integration/widgets/user-menu-test.js index 891c07101c7..afe1ad3bf68 100644 --- a/app/assets/javascripts/discourse/tests/integration/widgets/user-menu-test.js +++ b/app/assets/javascripts/discourse/tests/integration/widgets/user-menu-test.js @@ -79,7 +79,9 @@ discourseModule("Integration | Component | Widget | user-menu", function ( const routeToStub = sinon.stub(DiscourseURL, "routeTo"); await click(".user-notifications-link"); assert.ok( - routeToStub.calledWith(queryAll(".user-notifications-link")[0].href), + routeToStub.calledWith( + queryAll(".user-notifications-link").data("url") + ), "a second click should redirect to the full notifications page" ); }, @@ -120,7 +122,7 @@ discourseModule("Integration | Component | Widget | user-menu", function ( }, async test(assert) { - const userPmsLink = queryAll(".user-pms-link")[0]; + const userPmsLink = queryAll(".user-pms-link").data("url"); assert.ok(userPmsLink); await click(".user-pms-link"); @@ -143,7 +145,7 @@ discourseModule("Integration | Component | Widget | user-menu", function ( const routeToStub = sinon.stub(DiscourseURL, "routeTo"); await click(".user-pms-link"); assert.ok( - routeToStub.calledWith(userPmsLink.href), + routeToStub.calledWith(userPmsLink), "a second click should redirect to the full private messages page" ); }, @@ -171,7 +173,7 @@ discourseModule("Integration | Component | Widget | user-menu", function ( const routeToStub = sinon.stub(DiscourseURL, "routeTo"); await click(".user-bookmarks-link"); assert.ok( - routeToStub.calledWith(queryAll(".user-bookmarks-link")[0].href), + routeToStub.calledWith(queryAll(".user-bookmarks-link").data("url")), "a second click should redirect to the full bookmarks page" ); }, diff --git a/app/assets/stylesheets/common/base/menu-panel.scss b/app/assets/stylesheets/common/base/menu-panel.scss index 9d93eb26f86..2fde6944ae1 100644 --- a/app/assets/stylesheets/common/base/menu-panel.scss +++ b/app/assets/stylesheets/common/base/menu-panel.scss @@ -380,76 +380,73 @@ div.menu-links-header { display: flex; width: 100%; z-index: 2; + justify-content: space-between; - li { + .glyphs { display: inline-flex; align-items: center; - flex-wrap: wrap; + flex-wrap: nowrap; + width: 100%; + justify-content: space-between; + padding: 0; - &.glyphs { - flex-wrap: nowrap; - width: 100%; - justify-content: space-between; - padding: 0; - - a { - display: flex; - flex: 1 1 auto; - padding: 0.65em 0.25em 0.75em; - justify-content: center; - } - } - - a, button { - // This is to make sure active and inactive tab icons have the same - // size. `box-sizing` does not work and I have no idea why. - border: 1px solid transparent; - &:not(.active):hover { - border-bottom: 0; - margin-top: -1px; - } + display: flex; + flex: 1 1 auto; + padding: 0.65em 0.25em 0.75em; + justify-content: center; + } + } + + button { + // This is to make sure active and inactive tab icons have the same + // size. `box-sizing` does not work and I have no idea why. + border: 1px solid transparent; + &:not(.active):hover { + border-bottom: 0; + margin-top: -1px; + } + } + + button.active { + border: 1px solid var(--primary-low); + border-bottom: 1px solid var(--secondary); + position: relative; + + .d-icon { + color: var(--primary-high); } - a.active { - border: 1px solid var(--primary-low); - border-bottom: 1px solid var(--secondary); - position: relative; - - .d-icon { - color: var(--primary-high); - } - - &:focus, - &:hover { - background-color: inherit; - } + &:focus, + &:hover { + background-color: inherit; } } } - a:hover, - a:focus { + + button:hover, + button:focus { background-color: var(--highlight-medium); outline: none; } - a { + button { padding: 0.3em 0.5em; } - li { + .glyphs { display: table-cell; width: auto; text-align: center; } - li:first-child { + .glyphs:first-child { text-align: left; } - li:last-child { + .glyphs:last-child { text-align: right; } .fa, - a { + button { color: var(--primary-med-or-secondary-med); } }