From 5fbd6e113940a235a880b6519af1a1b7f4020d62 Mon Sep 17 00:00:00 2001 From: ayush-chauhan233 Date: Tue, 21 Jan 2025 02:55:11 +0530 Subject: [PATCH] [MM-61588]: Fixed the keyboard interactivity of tablist and added appropriate ARIA properties (#29592) * [MA-9]: Fixed the keyboard interactivity of tablist and added appropriate ARIA properties * [MA-9]: Review Fixes: Added aria-level attribute to plugin preferences heading * [MA-9]: Fixed failing e2e test case * [MA-9]: Fixed styling in mobile view and fixed failing e2e test cases * [MA-9]: Minor Refactoring --------- Co-authored-by: Mattermost Build --- .../accessibility_account_settings_spec.js | 2 +- .../channels/settings/settings_modal.ts | 2 +- .../settings_sidebar/settings_sidebar.tsx | 99 ++++++++++--------- .../team_access_tab/team_access_tab.tsx | 7 +- .../team_info_tab/team_info_tab.tsx | 7 +- .../advanced/user_settings_advanced.tsx | 6 +- .../user_settings_display.test.tsx.snap | 28 ++++++ .../display/user_settings_display.tsx | 6 +- .../general/user_settings_general.tsx | 6 +- .../user_settings_notifications.test.tsx.snap | 24 +++-- .../user_settings_notifications.tsx | 6 +- .../plugin/__snapshots__/index.test.tsx.snap | 6 +- .../components/user_settings/plugin/index.tsx | 6 +- .../user_settings_security.test.tsx.snap | 30 +++++- .../security/user_settings_security.tsx | 6 +- .../sidebar/user_settings_sidebar.tsx | 6 +- .../src/sass/components/_settings-modal.scss | 54 +++++----- .../channels/src/sass/responsive/_mobile.scss | 20 ++-- .../channels/src/sass/routes/_settings.scss | 51 ++++------ 19 files changed, 232 insertions(+), 140 deletions(-) diff --git a/e2e-tests/cypress/tests/integration/channels/accessibility/accessibility_account_settings_spec.js b/e2e-tests/cypress/tests/integration/channels/accessibility/accessibility_account_settings_spec.js index 6048a32c1e..ed16658359 100644 --- a/e2e-tests/cypress/tests/integration/channels/accessibility/accessibility_account_settings_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/accessibility/accessibility_account_settings_spec.js @@ -212,7 +212,7 @@ describe('Verify Accessibility Support in different sections in Settings and Pro // * Verify image alt in profile image cy.get('.profile-img').should('have.attr', 'alt', 'profile image'); - cy.get('#generalSettings').then((el) => { + cy.get('#profileSettings').then((el) => { if (el.find('.profile-img__remove').length > 0) { cy.findByTestId('removeSettingPicture').click(); cy.uiSave(); diff --git a/e2e-tests/playwright/support/ui/components/channels/settings/settings_modal.ts b/e2e-tests/playwright/support/ui/components/channels/settings/settings_modal.ts index 2b3e08cc76..61d3adb607 100644 --- a/e2e-tests/playwright/support/ui/components/channels/settings/settings_modal.ts +++ b/e2e-tests/playwright/support/ui/components/channels/settings/settings_modal.ts @@ -15,7 +15,7 @@ export default class SettingsModal { this.container = container; this.notificationsSettingsTab = container.locator('#notificationsButton'); - this.notificationsSettings = new NotificationsSettings(container.locator('#notificationSettings')); + this.notificationsSettings = new NotificationsSettings(container.locator('#notificationsSettings')); } async toBeVisible() { diff --git a/webapp/channels/src/components/settings_sidebar/settings_sidebar.tsx b/webapp/channels/src/components/settings_sidebar/settings_sidebar.tsx index e794272a92..39e346bfcc 100644 --- a/webapp/channels/src/components/settings_sidebar/settings_sidebar.tsx +++ b/webapp/channels/src/components/settings_sidebar/settings_sidebar.tsx @@ -1,6 +1,7 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. +import classNames from 'classnames'; import React from 'react'; import type {RefObject} from 'react'; import {FormattedMessage} from 'react-intl'; @@ -26,10 +27,12 @@ export type Props = { export default class SettingsSidebar extends React.PureComponent { buttonRefs: Array>; + totalTabs: Tab[]; constructor(props: Props) { super(props); - this.buttonRefs = this.props.tabs.map(() => React.createRef()); + this.totalTabs = [...this.props.tabs, ...this.props.pluginTabs || []]; + this.buttonRefs = this.totalTabs.map(() => React.createRef()); } public handleClick = (tab: Tab, e: React.MouseEvent) => { @@ -41,13 +44,19 @@ export default class SettingsSidebar extends React.PureComponent { public handleKeyUp = (index: number, e: React.KeyboardEvent) => { if (isKeyPressed(e, Constants.KeyCodes.UP)) { if (index > 0) { - this.props.updateTab(this.props.tabs[index - 1].name); + this.props.updateTab(this.totalTabs[index - 1].name); a11yFocus(this.buttonRefs[index - 1].current); + } else { + this.props.updateTab(this.totalTabs[this.totalTabs.length - 1].name); + a11yFocus(this.buttonRefs[this.buttonRefs.length - 1].current); } } else if (isKeyPressed(e, Constants.KeyCodes.DOWN)) { - if (index < this.props.tabs.length - 1) { - this.props.updateTab(this.props.tabs[index + 1].name); + if (index < this.totalTabs.length - 1) { + this.props.updateTab(this.totalTabs[index + 1].name); a11yFocus(this.buttonRefs[index + 1].current); + } else { + this.props.updateTab(this.totalTabs[0].name); + a11yFocus(this.buttonRefs[0].current); } } }; @@ -55,10 +64,6 @@ export default class SettingsSidebar extends React.PureComponent { private renderTab(tab: Tab, index: number) { const key = `${tab.name}_li`; const isActive = this.props.activeTab === tab.name; - let className = ''; - if (isActive) { - className = 'active'; - } let icon; if (typeof tab.icon === 'string') { @@ -79,27 +84,22 @@ export default class SettingsSidebar extends React.PureComponent { } return ( - + {icon} + {tab.uiName} + ); } @@ -110,32 +110,39 @@ export default class SettingsSidebar extends React.PureComponent { pluginTabList = ( <>
-
  • - -
    • - {this.props.pluginTabs.map((tab, index) => this.renderTab(tab, index))} +
    + +
    + {this.props.pluginTabs.map((tab, index) => this.renderTab(tab, index + this.props.tabs.length))} + ); } return ( -
    -
      +
      +
      {tabList} - {pluginTabList} -
    +
    + {pluginTabList} ); } diff --git a/webapp/channels/src/components/team_settings/team_access_tab/team_access_tab.tsx b/webapp/channels/src/components/team_settings/team_access_tab/team_access_tab.tsx index 1bc7d66bd0..bf04a6f5dc 100644 --- a/webapp/channels/src/components/team_settings/team_access_tab/team_access_tab.tsx +++ b/webapp/channels/src/components/team_settings/team_access_tab/team_access_tab.tsx @@ -123,7 +123,12 @@ const AccessTab = ({closeModal, collapseModal, hasChangeTabError, hasChanges, se {formatMessage({id: 'team_settings_modal.title', defaultMessage: 'Team Settings'})} -
    +
    {team.group_constrained ? undefined : {formatMessage({id: 'team_settings_modal.title', defaultMessage: 'Team Settings'})}
    -
    +
    +
    +
    { const pictureSection = this.createPictureSection(); return ( -
    +
    , "container":
    , "container":
    , "container":
    { const areAllSectionsInactive = this.props.activeSection === ''; return ( -
    +
    +
    +
    +
    +
    +
    +
    +
    { } return ( -
    +
    +
    li { + &__tab { + display: flex; + width: 100%; + height: 32px; + align-items: center; + justify-content: start; + padding: 0 12px; + border-radius: 4px; margin-bottom: 8px; + color: rgba(var(--center-channel-color-rgb), 0.75); + font-weight: 600; - &.header { - color: rgba(var(--center-channel-color-rgb), 0.75); - font-weight: 600; + i { + color: rgba(var(--center-channel-color-rgb), 0.64); } - button { - height: 32px; - padding: 0 12px; + &:hover { border-radius: 4px; - color: rgba(var(--center-channel-color-rgb), 0.75); - font-weight: 600; - - i { - color: rgba(var(--center-channel-color-rgb), 0.64); - } - - &:hover { - border-radius: 4px; - background-color: rgba(var(--center-channel-color-rgb), 0.04); - color: rgba(var(--center-channel-color-rgb), 0.8); - } + background-color: rgba(var(--center-channel-color-rgb), 0.04); + color: rgba(var(--center-channel-color-rgb), 0.8); } &.active { - button { - background: rgba(var(--button-bg-rgb), 0.08); - color: functions.v(button-bg); + background: rgba(var(--button-bg-rgb), 0.08); + color: functions.v(button-bg); - i { - color: functions.v(button-bg); - } + i { + color: functions.v(button-bg); } } } + + & .header { + margin-bottom: 8px; + color: rgba(var(--center-channel-color-rgb), 0.75); + font-weight: 600; + } } .settings-table { @@ -70,10 +70,6 @@ background: rgba(var(--center-channel-color-rgb), 0.04); } - .nav { - width: 200px; - } - .settings-content { .divider-dark { border-color: rgba(var(--center-channel-color-rgb), 0.12); diff --git a/webapp/channels/src/sass/responsive/_mobile.scss b/webapp/channels/src/sass/responsive/_mobile.scss index 7a6a2fffcd..cc6b7a85d5 100644 --- a/webapp/channels/src/sass/responsive/_mobile.scss +++ b/webapp/channels/src/sass/responsive/_mobile.scss @@ -167,7 +167,7 @@ } .nav-pills { - >li { + > &__tab { &:hover { a { background: transparent !important; @@ -276,7 +276,9 @@ width: 100%; margin: 0; - >li { + &-pills__tab { + height: 48px; + >a { font-size: 1.1em; line-height: 2.7; @@ -285,10 +287,6 @@ margin: 0 7px; } } - - > button { - height: 48px; - } } } @@ -319,14 +317,12 @@ } .nav-pills { - >li { + &__tab { &.active { - button { - background: transparent; + background: transparent; - &::before { - display: none; - } + &::before { + display: none; } } } diff --git a/webapp/channels/src/sass/routes/_settings.scss b/webapp/channels/src/sass/routes/_settings.scss index cc63bcb9cc..9e3f3833f2 100644 --- a/webapp/channels/src/sass/routes/_settings.scss +++ b/webapp/channels/src/sass/routes/_settings.scss @@ -569,30 +569,29 @@ background-color: rgba(0, 0, 0, 0.1); } - .nav-pills > li button { - color: rgba(var(--center-channel-color-rgb), 0.75); + .nav-pills { + &__tab { + color: rgba(var(--center-channel-color-rgb), 0.75); + } } } .nav-pills { - > li { + &__tab { + overflow: hidden; + width: 100%; + padding: 6px 15px; + border-radius: 4px; margin-bottom: 8px; + color: variables.$gray; + font-weight: 600; + text-align: left; + text-overflow: ellipsis; + white-space: nowrap; - button { - overflow: hidden; - width: 100%; - padding: 6px 15px; - border-radius: 4px; - color: variables.$gray; - font-weight: 600; - text-align: left; - text-overflow: ellipsis; - white-space: nowrap; - - &:hover { - background-color: rgba(var(--center-channel-color-rgb), 0.04); - color: rgba(var(--center-channel-color-rgb), 0.8); - } + &:hover { + background-color: rgba(var(--center-channel-color-rgb), 0.04); + color: rgba(var(--center-channel-color-rgb), 0.8); } img { @@ -604,7 +603,6 @@ .icon { position: relative; - top: 1px; width: 16px; margin-right: 8px; font-size: 18px; @@ -612,6 +610,10 @@ } &.active { + position: relative; + background: rgba(var(--button-bg-rgb), 0.08); + color: functions.v(button-bg); + div { background-color: #e1e1e1; color: #111; @@ -626,17 +628,6 @@ content: ''; } } - - button, - button:hover, - button:focus { - position: relative; - } - - button { - background: rgba(var(--button-bg-rgb), 0.08); - color: functions.v(button-bg); - } } } }