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

Jump to bottom

Fix usages of ARIA tabpanel #10628

Merged
merged 7 commits into from
Apr 25, 2023
Merged

Fix usages of ARIA tabpanel #10628

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e69c511
RovingTabIndex handle looping around start/end
t3chguy Apr 17, 2023
5bcf4d1
Make TabbedView expose aria tabpanel/tablist/tab roles
t3chguy Apr 17, 2023
78b6ed9
Fix right panel being wrongly specified as aria tabs
t3chguy Apr 17, 2023
93530df
tsc strict
t3chguy Apr 17, 2023
700960a
Update snapshots
t3chguy Apr 17, 2023
ad360cb
Fix ARIA AXE violation
t3chguy Apr 17, 2023
4c8abca
Update tests
t3chguy Apr 17, 2023
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
2 changes: 1 addition & 1 deletion cypress/e2e/integration-manager/get-openid-token.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ const INTEGRATION_MANAGER_HTML = `
`;

function openIntegrationManager() {
cy.findByRole("tab", { name: "Room info" }).click();
cy.findByRole("button", { name: "Room info" }).click();
Copy link
Contributor

@luixxiul luixxiul Apr 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It would be appreciated if you would let me know that this (and another one below) change meant <BaseCard> was not regarded as tabpanel (for #10495). Thanks in advance.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes they are no longer tabpanel due to tabpanel wanting a 1:1 relationship between tab and tabpanel but we have some right panel options which don't have a button in the header so it doesn't make much sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

I understood. Thank you for the explanation!

cy.findByRole("button", { name: "Add widgets, bridges & bots" }).click();
}

Expand Down
2 changes: 1 addition & 1 deletion cypress/e2e/integration-manager/kick.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ const INTEGRATION_MANAGER_HTML = `
`;

function openIntegrationManager() {
cy.findByRole("tab", { name: "Room info" }).click();
cy.findByRole("button", { name: "Room info" }).click();
cy.findByRole("button", { name: "Add widgets, bridges & bots" }).click();
}

Expand Down
15 changes: 12 additions & 3 deletions src/accessibility/RovingTabIndex.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ export const reducer: Reducer<IState, IAction> = (state: IState, action: IAction
};

interface IProps {
handleLoop?: boolean;
handleHomeEnd?: boolean;
handleUpDown?: boolean;
handleLeftRight?: boolean;
Expand All @@ -167,19 +168,26 @@ export const findSiblingElement = (
refs: RefObject<HTMLElement>[],
startIndex: number,
backwards = false,
loop = false,
): RefObject<HTMLElement> | undefined => {
if (backwards) {
for (let i = startIndex; i < refs.length && i >= 0; i--) {
if (refs[i].current?.offsetParent !== null) {
return refs[i];
}
}
if (loop) {
return findSiblingElement(refs.slice(startIndex + 1), refs.length - 1, true, false);
}
} else {
for (let i = startIndex; i < refs.length && i >= 0; i++) {
if (refs[i].current?.offsetParent !== null) {
return refs[i];
}
}
if (loop) {
return findSiblingElement(refs.slice(0, startIndex), 0, false, false);
}
}
};

Expand All @@ -188,6 +196,7 @@ export const RovingTabIndexProvider: React.FC<IProps> = ({
handleHomeEnd,
handleUpDown,
handleLeftRight,
handleLoop,
onKeyDown,
}) => {
const [state, dispatch] = useReducer<Reducer<IState, IAction>>(reducer, {
Expand Down Expand Up @@ -252,7 +261,7 @@ export const RovingTabIndexProvider: React.FC<IProps> = ({
handled = true;
if (context.state.refs.length > 0) {
const idx = context.state.refs.indexOf(context.state.activeRef!);
focusRef = findSiblingElement(context.state.refs, idx + 1);
focusRef = findSiblingElement(context.state.refs, idx + 1, false, handleLoop);
}
}
break;
Expand All @@ -266,7 +275,7 @@ export const RovingTabIndexProvider: React.FC<IProps> = ({
handled = true;
if (context.state.refs.length > 0) {
const idx = context.state.refs.indexOf(context.state.activeRef!);
focusRef = findSiblingElement(context.state.refs, idx - 1, true);
focusRef = findSiblingElement(context.state.refs, idx - 1, true, handleLoop);
}
}
break;
Expand All @@ -289,7 +298,7 @@ export const RovingTabIndexProvider: React.FC<IProps> = ({
});
}
},
[context, onKeyDown, handleHomeEnd, handleUpDown, handleLeftRight],
[context, onKeyDown, handleHomeEnd, handleUpDown, handleLeftRight, handleLoop],
);

return (
Expand Down
4 changes: 3 additions & 1 deletion src/components/structures/FilePanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,9 @@ class FilePanel extends React.Component<IProps, IState> {
withoutScrollContainer
ref={this.card}
>
<Measured sensor={this.card.current} onMeasurement={this.onMeasurement} />
{this.card.current && (
<Measured sensor={this.card.current} onMeasurement={this.onMeasurement} />
)}
<SearchWarning isRoomEncrypted={isRoomEncrypted} kind={WarningKind.Files} />
<TimelinePanel
manageReadReceipts={false}
Expand Down
46 changes: 37 additions & 9 deletions src/components/structures/TabbedView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ import { logger } from "matrix-js-sdk/src/logger";

import { _t } from "../../languageHandler";
import AutoHideScrollbar from "./AutoHideScrollbar";
import AccessibleButton from "../views/elements/AccessibleButton";
import { PosthogScreenTracker, ScreenName } from "../../PosthogTrackers";
import { NonEmptyArray } from "../../@types/common";
import { RovingAccessibleButton, RovingTabIndexProvider } from "../../accessibility/RovingTabIndex";

/**
* Represents a tab for the TabbedView.
Expand Down Expand Up @@ -98,34 +98,46 @@ export default class TabbedView extends React.Component<IProps, IState> {
}

private renderTabLabel(tab: Tab): JSX.Element {
let classes = "mx_TabbedView_tabLabel ";

if (this.state.activeTabId === tab.id) classes += "mx_TabbedView_tabLabel_active";
const isActive = this.state.activeTabId === tab.id;
const classes = classNames("mx_TabbedView_tabLabel", {
mx_TabbedView_tabLabel_active: isActive,
});

let tabIcon: JSX.Element | undefined;
if (tab.icon) {
tabIcon = <span className={`mx_TabbedView_maskedIcon ${tab.icon}`} />;
}

const onClickHandler = (): void => this.setActiveTab(tab);
const id = this.getTabId(tab);

const label = _t(tab.label);
return (
<AccessibleButton
<RovingAccessibleButton
className={classes}
key={"tab_label_" + tab.label}
onClick={onClickHandler}
data-testid={`settings-tab-${tab.id}`}
role="tab"
aria-selected={isActive}
aria-controls={id}
>
{tabIcon}
<span className="mx_TabbedView_tabLabel_text">{label}</span>
</AccessibleButton>
<span className="mx_TabbedView_tabLabel_text" id={`${id}_label`}>
{label}
</span>
</RovingAccessibleButton>
);
}

private getTabId(tab: Tab): string {
return `mx_tabpanel_${tab.id}`;
}

private renderTabPanel(tab: Tab): React.ReactNode {
const id = this.getTabId(tab);
return (
<div className="mx_TabbedView_tabPanel" key={"mx_tabpanel_" + tab.label}>
<div className="mx_TabbedView_tabPanel" key={id} id={id} aria-labelledby={`${id}_label`}>
<AutoHideScrollbar className="mx_TabbedView_tabPanelContent">{tab.body}</AutoHideScrollbar>
</div>
);
Expand All @@ -147,7 +159,23 @@ export default class TabbedView extends React.Component<IProps, IState> {
return (
<div className={tabbedViewClasses}>
{screenName && <PosthogScreenTracker screenName={screenName} />}
<div className="mx_TabbedView_tabLabels">{labels}</div>
<RovingTabIndexProvider
handleLoop
handleHomeEnd
handleLeftRight={this.props.tabLocation == TabLocation.TOP}
handleUpDown={this.props.tabLocation == TabLocation.LEFT}
>
{({ onKeyDownHandler }) => (
<div
className="mx_TabbedView_tabLabels"
role="tablist"
aria-orientation={this.props.tabLocation == TabLocation.LEFT ? "vertical" : "horizontal"}
onKeyDown={onKeyDownHandler}
>
{labels}
</div>
)}
</RovingTabIndexProvider>
{panel}
</div>
);
Expand Down
3 changes: 1 addition & 2 deletions src/components/views/right_panel/HeaderButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ export default class HeaderButton extends React.Component<IProps> {
return (
<AccessibleTooltipButton
{...props}
aria-selected={isHighlighted}
role="tab"
aria-current={isHighlighted ? "true" : "false"}
title={title}
alignment={Alignment.Bottom}
className={classes}
Expand Down
6 changes: 1 addition & 5 deletions src/components/views/right_panel/HeaderButtons.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,6 @@ export default abstract class HeaderButtons<P = {}> extends React.Component<IPro
public abstract renderButtons(): JSX.Element;

public render(): React.ReactNode {
return (
<div className="mx_HeaderButtons" role="tablist">
{this.renderButtons()}
</div>
);
return <div className="mx_HeaderButtons">{this.renderButtons()}</div>;
}
}
27 changes: 20 additions & 7 deletions test/components/structures/__snapshots__/TabbedView-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,56 +6,69 @@ exports[`<TabbedView /> renders tabs 1`] = `
class="mx_TabbedView mx_TabbedView_tabsOnLeft"
>
<div
aria-orientation="vertical"
class="mx_TabbedView_tabLabels"
role="tablist"
>
<div
aria-controls="mx_tabpanel_GENERAL"
aria-selected="true"
class="mx_AccessibleButton mx_TabbedView_tabLabel mx_TabbedView_tabLabel_active"
data-testid="settings-tab-GENERAL"
role="button"
role="tab"
tabindex="0"
>
<span
class="mx_TabbedView_maskedIcon general"
/>
<span
class="mx_TabbedView_tabLabel_text"
id="mx_tabpanel_GENERAL_label"
>
General
</span>
</div>
<div
class="mx_AccessibleButton mx_TabbedView_tabLabel "
aria-controls="mx_tabpanel_LABS"
aria-selected="false"
class="mx_AccessibleButton mx_TabbedView_tabLabel"
data-testid="settings-tab-LABS"
role="button"
tabindex="0"
role="tab"
tabindex="-1"
>
<span
class="mx_TabbedView_maskedIcon labs"
/>
<span
class="mx_TabbedView_tabLabel_text"
id="mx_tabpanel_LABS_label"
>
Labs
</span>
</div>
<div
class="mx_AccessibleButton mx_TabbedView_tabLabel "
aria-controls="mx_tabpanel_SECURITY"
aria-selected="false"
class="mx_AccessibleButton mx_TabbedView_tabLabel"
data-testid="settings-tab-SECURITY"
role="button"
tabindex="0"
role="tab"
tabindex="-1"
>
<span
class="mx_TabbedView_maskedIcon security"
/>
<span
class="mx_TabbedView_tabLabel_text"
id="mx_tabpanel_SECURITY_label"
>
Security
</span>
</div>
</div>
<div
aria-labelledby="mx_tabpanel_GENERAL_label"
class="mx_TabbedView_tabPanel"
id="mx_tabpanel_GENERAL"
>
<div
class="mx_AutoHideScrollbar mx_TabbedView_tabPanelContent"
Expand Down
41 changes: 28 additions & 13 deletions test/components/views/dialogs/__snapshots__/RoomSettingsDialog-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,76 +3,91 @@
exports[`<RoomSettingsDialog /> Settings tabs renders default tabs correctly 1`] = `
NodeList [
<div
aria-controls="mx_tabpanel_ROOM_GENERAL_TAB"
aria-selected="true"
class="mx_AccessibleButton mx_TabbedView_tabLabel mx_TabbedView_tabLabel_active"
data-testid="settings-tab-ROOM_GENERAL_TAB"
role="button"
role="tab"
tabindex="0"
>
<span
class="mx_TabbedView_maskedIcon mx_RoomSettingsDialog_settingsIcon"
/>
<span
class="mx_TabbedView_tabLabel_text"
id="mx_tabpanel_ROOM_GENERAL_TAB_label"
>
General
</span>
</div>,
<div
class="mx_AccessibleButton mx_TabbedView_tabLabel "
aria-controls="mx_tabpanel_ROOM_SECURITY_TAB"
aria-selected="false"
class="mx_AccessibleButton mx_TabbedView_tabLabel"
data-testid="settings-tab-ROOM_SECURITY_TAB"
role="button"
tabindex="0"
role="tab"
tabindex="-1"
>
<span
class="mx_TabbedView_maskedIcon mx_RoomSettingsDialog_securityIcon"
/>
<span
class="mx_TabbedView_tabLabel_text"
id="mx_tabpanel_ROOM_SECURITY_TAB_label"
>
Security & Privacy
</span>
</div>,
<div
class="mx_AccessibleButton mx_TabbedView_tabLabel "
aria-controls="mx_tabpanel_ROOM_ROLES_TAB"
aria-selected="false"
class="mx_AccessibleButton mx_TabbedView_tabLabel"
data-testid="settings-tab-ROOM_ROLES_TAB"
role="button"
tabindex="0"
role="tab"
tabindex="-1"
>
<span
class="mx_TabbedView_maskedIcon mx_RoomSettingsDialog_rolesIcon"
/>
<span
class="mx_TabbedView_tabLabel_text"
id="mx_tabpanel_ROOM_ROLES_TAB_label"
>
Roles & Permissions
</span>
</div>,
<div
class="mx_AccessibleButton mx_TabbedView_tabLabel "
aria-controls="mx_tabpanel_ROOM_NOTIFICATIONS_TAB"
aria-selected="false"
class="mx_AccessibleButton mx_TabbedView_tabLabel"
data-testid="settings-tab-ROOM_NOTIFICATIONS_TAB"
role="button"
tabindex="0"
role="tab"
tabindex="-1"
>
<span
class="mx_TabbedView_maskedIcon mx_RoomSettingsDialog_notificationsIcon"
/>
<span
class="mx_TabbedView_tabLabel_text"
id="mx_tabpanel_ROOM_NOTIFICATIONS_TAB_label"
>
Notifications
</span>
</div>,
<div
class="mx_AccessibleButton mx_TabbedView_tabLabel "
aria-controls="mx_tabpanel_ROOM_POLL_HISTORY_TAB"
aria-selected="false"
class="mx_AccessibleButton mx_TabbedView_tabLabel"
data-testid="settings-tab-ROOM_POLL_HISTORY_TAB"
role="button"
tabindex="0"
role="tab"
tabindex="-1"
>
<span
class="mx_TabbedView_maskedIcon mx_RoomSettingsDialog_pollsIcon"
/>
<span
class="mx_TabbedView_tabLabel_text"
id="mx_tabpanel_ROOM_POLL_HISTORY_TAB_label"
>
Poll history
</span>
Expand Down
Loading