Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MA-9]: Fixed the keyboard interactivity of tablist and added appropriate ARIA properties #13

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[MA-9]: Fixed the keyboard interactivity of tablist and added appropriate ARIA properties #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a77c0d0
[MA-9]: Fixed the keyboard interactivity of tablist and added appropr…
ayush-chauhan233 Dec 5, 2024
51d4620
[MA-9]: Review Fixes: Added aria-level attribute to plugin preference…
ayush-chauhan233 Dec 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 54 additions & 44 deletions webapp/channels/src/components/settings_sidebar/settings_sidebar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@ export type Props = {

export default class SettingsSidebar extends React.PureComponent<Props> {
buttonRefs: Array<RefObject<HTMLButtonElement>>;
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) => {
Expand All @@ -41,23 +43,29 @@ export default class SettingsSidebar extends React.PureComponent<Props> {
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);
}
}
};

private renderTab(tab: Tab, index: number) {
const key = `${tab.name}_li`;
const isActive = this.props.activeTab === tab.name;
let className = '';
let className = 'nav-pills__tab';
if (isActive) {
className = 'active';
className += ' active';
}

let icon;
Expand All @@ -79,27 +87,22 @@ export default class SettingsSidebar extends React.PureComponent<Props> {
}

return (
<li
id={`${tab.name}Li`}
<button
key={key}
className={className}
role='presentation'
ref={this.buttonRefs[index]}
id={`${tab.name}Button`}
className={`cursor--pointer style--none ${className}`}
onClick={this.handleClick.bind(null, tab)}
onKeyUp={this.handleKeyUp.bind(null, index)}
aria-label={tab.uiName.toLowerCase()}
role='tab'
aria-selected={isActive}
tabIndex={!isActive && !this.props.isMobileView ? -1 : 0}
aria-controls={`${tab.name}Settings`}
>
<button
ref={this.buttonRefs[index]}
id={`${tab.name}Button`}
className='cursor--pointer style--none'
onClick={this.handleClick.bind(null, tab)}
onKeyUp={this.handleKeyUp.bind(null, index)}
aria-label={tab.uiName.toLowerCase()}
role='tab'
aria-selected={isActive}
tabIndex={!isActive && !this.props.isMobileView ? -1 : 0}
>
{icon}
{tab.uiName}
</button>
</li>
{icon}
{tab.uiName}
</button>
);
}

Expand All @@ -110,32 +113,39 @@ export default class SettingsSidebar extends React.PureComponent<Props> {
pluginTabList = (
<>
<hr/>
<li
key={'plugin preferences heading'}
role='heading'
className={'header'}
<div
role='group'
aria-labelledby='userSettingsModal.pluginPreferences.header'
>
<FormattedMessage
id={'userSettingsModal.pluginPreferences.header'}
defaultMessage={'PLUGIN PREFERENCES'}
/>
</li>
{this.props.pluginTabs.map((tab, index) => this.renderTab(tab, index))}
<div
key={'plugin preferences heading'}
role='heading'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't we need to add aria-level here ?

className={'header'}
aria-level={3}
id='userSettingsModal.pluginPreferences.header'
>
<FormattedMessage
id={'userSettingsModal.pluginPreferences.header'}
defaultMessage={'PLUGIN PREFERENCES'}
/>
</div>
{this.props.pluginTabs.map((tab, index) => this.renderTab(tab, index + this.props.tabs.length))}
</div>
</>
);
}

return (
<div>
<ul
id='tabList'
className='nav nav-pills nav-stacked'
role='tablist'
aria-orientation='vertical'
>
<div
id='tabList'
className='nav nav-pills nav-stacked'
role='tablist'
aria-orientation='vertical'
>
<div role='group'>
{tabList}
{pluginTabList}
</ul>
</div>
{pluginTabList}
</div>
);
}
Expand Down
7 changes: 6 additions & 1 deletion webapp/channels/src/components/team_settings/team_access_tab/team_access_tab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,12 @@ const AccessTab = ({closeModal, collapseModal, hasChangeTabError, hasChanges, se
<span>{formatMessage({id: 'team_settings_modal.title', defaultMessage: 'Team Settings'})}</span>
</h4>
</div>
<div className='modal-access-tab-content user-settings'>
<div
className='modal-access-tab-content user-settings'
id='accessSettings'
aria-labelledby='accessButton'
role='tabpanel'
>
{team.group_constrained ?
undefined :
<AllowedDomainsSelect
Expand Down
7 changes: 6 additions & 1 deletion webapp/channels/src/components/team_settings/team_info_tab/team_info_tab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,12 @@ const InfoTab = ({team, hasChanges, maxFileSize, closeModal, collapseModal, hasC
<span>{formatMessage({id: 'team_settings_modal.title', defaultMessage: 'Team Settings'})}</span>
</h4>
</div>
<div className='modal-info-tab-content user-settings' >
<div
className='modal-info-tab-content user-settings'
id='infoSettings'
aria-labelledby='infoButton'
role='tabpanel'
>
<div className='name-description-container' >
<TeamNameSection
name={name}
Expand Down
6 changes: 5 additions & 1 deletion webapp/channels/src/components/user_settings/advanced/user_settings_advanced.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -738,7 +738,11 @@ export default class AdvancedSettingsDisplay extends React.PureComponent<Props,
}

return (
<div>
<div
id='advancedSettings'
aria-labelledby='advancedButton'
role='tabpanel'
>
<SettingMobileHeader
closeModal={this.props.closeModal}
collapseModal={this.props.collapseModal}
Expand Down
28 changes: 28 additions & 0 deletions ...ls/src/components/user_settings/display/__snapshots__/user_settings_display.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

exports[`components/user_settings/display/UserSettingsDisplay should match snapshot, channel display mode section 1`] = `
<div
aria-labelledby="displayButton"
id="displaySettings"
role="tabpanel"
>
<SettingMobileHeader
closeModal={[MockFunction]}
Expand Down Expand Up @@ -355,7 +357,9 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

exports[`components/user_settings/display/UserSettingsDisplay should match snapshot, clickToReply section 1`] = `
<div
aria-labelledby="displayButton"
id="displaySettings"
role="tabpanel"
>
<SettingMobileHeader
closeModal={[MockFunction]}
Expand Down Expand Up @@ -708,7 +712,9 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

exports[`components/user_settings/display/UserSettingsDisplay should match snapshot, clock section 1`] = `
<div
aria-labelledby="displayButton"
id="displaySettings"
role="tabpanel"
>
<SettingMobileHeader
closeModal={[MockFunction]}
Expand Down Expand Up @@ -1061,7 +1067,9 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

exports[`components/user_settings/display/UserSettingsDisplay should match snapshot, collapse section 1`] = `
<div
aria-labelledby="displayButton"
id="displaySettings"
role="tabpanel"
>
<SettingMobileHeader
closeModal={[MockFunction]}
Expand Down Expand Up @@ -1414,7 +1422,9 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

exports[`components/user_settings/display/UserSettingsDisplay should match snapshot, languages section 1`] = `
<div
aria-labelledby="displayButton"
id="displaySettings"
role="tabpanel"
>
<SettingMobileHeader
closeModal={[MockFunction]}
Expand Down Expand Up @@ -1694,7 +1704,9 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

exports[`components/user_settings/display/UserSettingsDisplay should match snapshot, link preview section with EnableLinkPreviews is false 1`] = `
<div
aria-labelledby="displayButton"
id="displaySettings"
role="tabpanel"
>
<SettingMobileHeader
closeModal={[MockFunction]}
Expand Down Expand Up @@ -1956,7 +1968,9 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

exports[`components/user_settings/display/UserSettingsDisplay should match snapshot, link preview section with EnableLinkPreviews is true 1`] = `
<div
aria-labelledby="displayButton"
id="displaySettings"
role="tabpanel"
>
<SettingMobileHeader
closeModal={[MockFunction]}
Expand Down Expand Up @@ -2309,7 +2323,9 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

exports[`components/user_settings/display/UserSettingsDisplay should match snapshot, message display section 1`] = `
<div
aria-labelledby="displayButton"
id="displaySettings"
role="tabpanel"
>
<SettingMobileHeader
closeModal={[MockFunction]}
Expand Down Expand Up @@ -2680,7 +2696,9 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

exports[`components/user_settings/display/UserSettingsDisplay should match snapshot, no active section 1`] = `
<div
aria-labelledby="displayButton"
id="displaySettings"
role="tabpanel"
>
<SettingMobileHeader
closeModal={[MockFunction]}
Expand Down Expand Up @@ -2960,7 +2978,9 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

exports[`components/user_settings/display/UserSettingsDisplay should match snapshot, teammate name display section 1`] = `
<div
aria-labelledby="displayButton"
id="displaySettings"
role="tabpanel"
>
<SettingMobileHeader
closeModal={[MockFunction]}
Expand Down Expand Up @@ -3240,7 +3260,9 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

exports[`components/user_settings/display/UserSettingsDisplay should match snapshot, theme section with EnableThemeSelection is false 1`] = `
<div
aria-labelledby="displayButton"
id="displaySettings"
role="tabpanel"
>
<SettingMobileHeader
closeModal={[MockFunction]}
Expand Down Expand Up @@ -3520,7 +3542,9 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

exports[`components/user_settings/display/UserSettingsDisplay should match snapshot, theme section with EnableThemeSelection is true 1`] = `
<div
aria-labelledby="displayButton"
id="displaySettings"
role="tabpanel"
>
<SettingMobileHeader
closeModal={[MockFunction]}
Expand Down Expand Up @@ -3812,7 +3836,9 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

exports[`components/user_settings/display/UserSettingsDisplay should match snapshot, timezone section 1`] = `
<div
aria-labelledby="displayButton"
id="displaySettings"
role="tabpanel"
>
<SettingMobileHeader
closeModal={[MockFunction]}
Expand Down Expand Up @@ -4092,7 +4118,9 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

exports[`components/user_settings/display/UserSettingsDisplay should not show last active section 1`] = `
<div
aria-labelledby="displayButton"
id="displaySettings"
role="tabpanel"
>
<SettingMobileHeader
closeModal={[MockFunction]}
Expand Down
6 changes: 5 additions & 1 deletion webapp/channels/src/components/user_settings/display/user_settings_display.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1148,7 +1148,11 @@ export default class UserSettingsDisplay extends React.PureComponent<Props, Stat
}

return (
<div id='displaySettings'>
<div
id='displaySettings'
aria-labelledby='displayButton'
role='tabpanel'
>
<SettingMobileHeader
closeModal={this.props.closeModal}
collapseModal={this.props.collapseModal}
Expand Down
6 changes: 5 additions & 1 deletion webapp/channels/src/components/user_settings/general/user_settings_general.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1363,7 +1363,11 @@ export class UserSettingsGeneralTab extends PureComponent<Props, State> {
const pictureSection = this.createPictureSection();

return (
<div id='generalSettings'>
<div
id='profileSettings'
aria-labelledby='profileButton'
role='tabpanel'
>
<SettingMobileHeader
closeModal={this.props.closeModal}
collapseModal={this.props.collapseModal}
Expand Down
Loading
Loading