Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Tooltip: improve accessibility of spaces #12497

Merged
merged 2 commits into from
May 3, 2024
Merged

Tooltip: improve accessibility of spaces #12497

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
24f551b
Migrate to `AccessibleButtons`
florianduros May 3, 2024
e036d48
Update snapshots
florianduros May 3, 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
7 changes: 3 additions & 4 deletions src/components/structures/LeftPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import { HEADER_HEIGHT } from "../views/rooms/RoomSublist";
import { Action } from "../../dispatcher/actions";
import RoomSearch from "./RoomSearch";
import ResizeNotifier from "../../utils/ResizeNotifier";
import AccessibleTooltipButton from "../views/elements/AccessibleTooltipButton";
import SpaceStore from "../../stores/spaces/SpaceStore";
import { MetaSpace, SpaceKey, UPDATE_SELECTED_SPACE } from "../../stores/spaces";
import { getKeyBindingsManager } from "../../KeyBindingsManager";
Expand All @@ -41,7 +40,7 @@ import RoomBreadcrumbs from "../views/rooms/RoomBreadcrumbs";
import { KeyBindingAction } from "../../accessibility/KeyboardShortcuts";
import { shouldShowComponent } from "../../customisations/helpers/UIComponents";
import { UIComponent } from "../../settings/UIFeature";
import { ButtonEvent } from "../views/elements/AccessibleButton";
import AccessibleButton, { ButtonEvent } from "../views/elements/AccessibleButton";
import PosthogTrackers from "../../PosthogTrackers";
import PageType from "../../PageTypes";
import { UserOnboardingButton } from "../views/user-onboarding/UserOnboardingButton";
Expand Down Expand Up @@ -333,7 +332,7 @@ export default class LeftPanel extends React.Component<IProps, IState> {
// to start a new call
if (LegacyCallHandler.instance.getSupportsPstnProtocol()) {
dialPadButton = (
<AccessibleTooltipButton
<AccessibleButton
className={classNames("mx_LeftPanel_dialPadButton", {})}
onClick={this.onDialPad}
title={_t("left_panel|open_dial_pad")}
Expand All @@ -344,7 +343,7 @@ export default class LeftPanel extends React.Component<IProps, IState> {
let rightButton: JSX.Element | undefined;
if (this.state.activeSpace === MetaSpace.Home && shouldShowComponent(UIComponent.ExploreRooms)) {
rightButton = (
<AccessibleTooltipButton
<AccessibleButton
className="mx_LeftPanel_exploreButton"
onClick={this.onExplore}
title={_t("action|explore_rooms")}
Expand Down
35 changes: 14 additions & 21 deletions src/components/structures/SpaceHierarchy.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ limitations under the License.
*/

import React, {
ComponentProps,
Dispatch,
KeyboardEvent,
KeyboardEventHandler,
Expand Down Expand Up @@ -62,7 +61,6 @@ import InfoTooltip from "../views/elements/InfoTooltip";
import TextWithTooltip from "../views/elements/TextWithTooltip";
import { useStateToggle } from "../../hooks/useStateToggle";
import { getChildOrder } from "../../stores/spaces/SpaceStore";
import AccessibleTooltipButton from "../views/elements/AccessibleTooltipButton";
import { Linkify, topicToHtml } from "../../HtmlUtils";
import { useDispatcher } from "../../hooks/useDispatcher";
import { Action } from "../../dispatcher/actions";
Expand All @@ -75,7 +73,6 @@ import { ViewRoomPayload } from "../../dispatcher/payloads/ViewRoomPayload";
import { JoinRoomReadyPayload } from "../../dispatcher/payloads/JoinRoomReadyPayload";
import { KeyBindingAction } from "../../accessibility/KeyboardShortcuts";
import { getKeyBindingsManager } from "../../KeyBindingsManager";
import { Alignment } from "../views/elements/Tooltip";
import { getTopic } from "../../hooks/room/useTopic";
import { SdkContextClass } from "../../contexts/SDKContext";
import { getDisplayAliasForAliasSet } from "../../Rooms";
Expand Down Expand Up @@ -148,7 +145,7 @@ const Tile: React.FC<ITileProps> = ({
let button: ReactElement;
if (busy) {
button = (
<AccessibleTooltipButton
<AccessibleButton
disabled={true}
onClick={onJoinClick}
kind="primary_outline"
Expand All @@ -157,7 +154,7 @@ const Tile: React.FC<ITileProps> = ({
title={_t("space|joining_space")}
>
<Spinner w={24} h={24} />
</AccessibleTooltipButton>
</AccessibleButton>
);
} else if (joinedRoom || room.join_rule === JoinRule.Knock) {
// If the room is knockable, show the "View" button even if we are not a member; that
Expand Down Expand Up @@ -670,25 +667,16 @@ const ManageButtons: React.FC<IManageButtonsProps> = ({ hierarchy, selected, set

const disabled = !selectedRelations.length || removing || saving;

let Button: React.ComponentType<React.ComponentProps<typeof AccessibleButton>> = AccessibleButton;
let props: Partial<ComponentProps<typeof AccessibleTooltipButton>> = {};
if (!selectedRelations.length) {
Button = AccessibleTooltipButton;
props = {
tooltip: _t("space|select_room_below"),
alignment: Alignment.Top,
};
}

let buttonText = _t("common|saving");
if (!saving) {
buttonText = selectionAllSuggested ? _t("space|unmark_suggested") : _t("space|mark_suggested");
}

const title = !selectedRelations.length ? _t("space|select_room_below") : undefined;

return (
<>
<Button
{...props}
<AccessibleButton
onClick={async (): Promise<void> => {
setRemoving(true);
try {
Expand Down Expand Up @@ -719,11 +707,13 @@ const ManageButtons: React.FC<IManageButtonsProps> = ({ hierarchy, selected, set
}}
kind="danger_outline"
disabled={disabled}
aria-label={removing ? _t("redact|ongoing") : _t("action|remove")}
t3chguy marked this conversation as resolved.
Show resolved Hide resolved
title={title}
placement="top"
>
{removing ? _t("redact|ongoing") : _t("action|remove")}
</Button>
<Button
{...props}
</AccessibleButton>
<AccessibleButton
onClick={async (): Promise<void> => {
setSaving(true);
try {
Expand All @@ -750,9 +740,12 @@ const ManageButtons: React.FC<IManageButtonsProps> = ({ hierarchy, selected, set
}}
kind="primary_outline"
disabled={disabled}
aria-label={buttonText}
Copy link
Member

Choose a reason for hiding this comment

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

The old behaviour would have had a label of the title value, not the text content

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't get it.
When I'm looking at the old snapshots, they hadn't any label.

Copy link
Member

Choose a reason for hiding this comment

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

Then why do they need them now?

Copy link
Contributor Author

@florianduros florianduros May 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

AccessibleButton put automatically an aria-label with by default the content of title. In theses cases, I forced the aria-label to have the same content than the button.

To be honest I don't have a strong opinion if the aria-label should have the content of the button or the tooltip...

title={title}
placement="top"
>
{buttonText}
</Button>
</AccessibleButton>
</>
);
};
Expand Down
3 changes: 1 addition & 2 deletions src/components/structures/SpaceRoomView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ import {
defaultRoomsRenderer,
} from "../views/dialogs/AddExistingToSpaceDialog";
import AccessibleButton, { ButtonEvent } from "../views/elements/AccessibleButton";
import AccessibleTooltipButton from "../views/elements/AccessibleTooltipButton";
import ErrorBoundary from "../views/elements/ErrorBoundary";
import Field from "../views/elements/Field";
import RoomFacePile from "../views/elements/RoomFacePile";
Expand Down Expand Up @@ -248,7 +247,7 @@ const SpaceLanding: React.FC<{ space: Room }> = ({ space }) => {
let settingsButton;
if (shouldShowSpaceSettings(space)) {
settingsButton = (
<AccessibleTooltipButton
<AccessibleButton
className="mx_SpaceRoomView_landing_settingsButton"
onClick={() => {
showSpaceSettings(space);
Expand Down
9 changes: 4 additions & 5 deletions src/components/views/spaces/QuickSettingsButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import React from "react";
import classNames from "classnames";

import { _t } from "../../../languageHandler";
import AccessibleTooltipButton from "../elements/AccessibleTooltipButton";
import ContextMenu, { alwaysAboveRightOf, ChevronFace, useContextMenu } from "../../structures/ContextMenu";
import AccessibleButton from "../elements/AccessibleButton";
import StyledCheckbox from "../elements/StyledCheckbox";
Expand Down Expand Up @@ -130,16 +129,16 @@ const QuickSettingsButton: React.FC<{

return (
<>
<AccessibleTooltipButton
<AccessibleButton
className={classNames("mx_QuickSettingsButton", { expanded: !isPanelCollapsed })}
onClick={openMenu}
title={_t("quick_settings|title")}
aria-label={_t("quick_settings|title")}
title={isPanelCollapsed ? _t("quick_settings|title") : undefined}
ref={handle}
forceHide={!isPanelCollapsed}
aria-expanded={!isPanelCollapsed}
>
{!isPanelCollapsed ? _t("common|settings") : null}
</AccessibleTooltipButton>
</AccessibleButton>

{contextMenu}
</>
Expand Down
3 changes: 1 addition & 2 deletions src/components/views/spaces/SpaceCreateMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import {
import { logger } from "matrix-js-sdk/src/logger";

import { _t } from "../../../languageHandler";
import AccessibleTooltipButton from "../elements/AccessibleTooltipButton";
import ContextMenu, { ChevronFace } from "../../structures/ContextMenu";
import createRoom, { IOpts as ICreateOpts } from "../../../createRoom";
import MatrixClientContext, { useMatrixClientContext } from "../../../contexts/MatrixClientContext";
Expand Down Expand Up @@ -310,7 +309,7 @@ const SpaceCreateMenu: React.FC<{
} else {
body = (
<React.Fragment>
<AccessibleTooltipButton
<AccessibleButton
className="mx_SpaceCreateMenu_back"
onClick={() => setVisibility(null)}
title={_t("action|go_back")}
Expand Down
26 changes: 10 additions & 16 deletions src/components/views/spaces/SpacePanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import { _t } from "../../../languageHandler";
import { useContextMenu } from "../../structures/ContextMenu";
import SpaceCreateMenu from "./SpaceCreateMenu";
import { SpaceButton, SpaceItem } from "./SpaceTreeLevel";
import AccessibleTooltipButton from "../elements/AccessibleTooltipButton";
import { useEventEmitter, useEventEmitterState } from "../../../hooks/useEventEmitter";
import SpaceStore from "../../../stores/spaces/SpaceStore";
import {
Expand Down Expand Up @@ -73,6 +72,7 @@ import { ALTERNATE_KEY_NAME } from "../../../accessibility/KeyboardShortcuts";
import { shouldShowComponent } from "../../../customisations/helpers/UIComponents";
import { UIComponent } from "../../../settings/UIFeature";
import { ThreadsActivityCentre } from "./threads-activity-centre/";
import AccessibleButton from "../elements/AccessibleButton";

const useSpaces = (): [Room[], MetaSpace[], Room[], SpaceKey] => {
const invites = useEventEmitterState<Room[]>(SpaceStore.instance, UPDATE_INVITED_SPACES, () => {
Expand Down Expand Up @@ -389,24 +389,18 @@ const SpacePanel: React.FC = () => {
aria-label={_t("common|spaces")}
>
<UserMenu isPanelCollapsed={isPanelCollapsed}>
<AccessibleTooltipButton
<AccessibleButton
className={classNames("mx_SpacePanel_toggleCollapse", { expanded: !isPanelCollapsed })}
onClick={() => setPanelCollapsed(!isPanelCollapsed)}
title={isPanelCollapsed ? _t("action|expand") : _t("action|collapse")}
tooltip={
<div>
<div className="mx_Tooltip_title">
{isPanelCollapsed ? _t("action|expand") : _t("action|collapse")}
</div>
<div className="mx_Tooltip_sub">
{IS_MAC
? "⌘ + ⇧ + D"
: _t(ALTERNATE_KEY_NAME[Key.CONTROL]) +
" + " +
_t(ALTERNATE_KEY_NAME[Key.SHIFT]) +
" + D"}
</div>
</div>
// TODO should use a kbd element for accessibility https://developer.mozilla.org/en-US/docs/Web/HTML/Element/kbd
caption={
IS_MAC
? "⌘ + ⇧ + D"
: _t(ALTERNATE_KEY_NAME[Key.CONTROL]) +
" + " +
_t(ALTERNATE_KEY_NAME[Key.SHIFT]) +
" + D"
}
/>
</UserMenu>
Expand Down
11 changes: 5 additions & 6 deletions src/components/views/spaces/SpaceTreeLevel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,11 @@ import { NotificationLevel } from "../../../stores/notifications/NotificationLev
import { getKeyBindingsManager } from "../../../KeyBindingsManager";
import { NotificationState } from "../../../stores/notifications/NotificationState";
import SpaceContextMenu from "../context_menus/SpaceContextMenu";
import AccessibleTooltipButton from "../elements/AccessibleTooltipButton";
import { useRovingTabIndex } from "../../../accessibility/RovingTabIndex";
import { KeyBindingAction } from "../../../accessibility/KeyboardShortcuts";

type ButtonProps<T extends keyof JSX.IntrinsicElements> = Omit<
ComponentProps<typeof AccessibleTooltipButton<T>>,
ComponentProps<typeof AccessibleButton<T>>,
"title" | "onClick" | "size" | "element"
> & {
space?: Room;
Expand Down Expand Up @@ -143,17 +142,17 @@ export const SpaceButton = <T extends keyof JSX.IntrinsicElements>({
const onClick = props.onClick ?? (selected && space ? viewSpaceHome : activateSpace);

return (
<AccessibleTooltipButton
<AccessibleButton
{...props}
className={classNames("mx_SpaceButton", className, {
mx_SpaceButton_active: selected,
mx_SpaceButton_hasMenuOpen: menuDisplayed,
mx_SpaceButton_narrow: isNarrow,
})}
title={label}
aria-label={label}
title={!isNarrow || menuDisplayed ? undefined : label}
onClick={onClick}
onContextMenu={openMenu}
forceHide={!isNarrow || menuDisplayed}
ref={handle}
tabIndex={tabIndex}
onFocus={onFocus}
Expand All @@ -177,7 +176,7 @@ export const SpaceButton = <T extends keyof JSX.IntrinsicElements>({

{contextMenu}
</div>
</AccessibleTooltipButton>
</AccessibleButton>
);
};

Expand Down
4 changes: 4 additions & 0 deletions test/components/structures/__snapshots__/SpaceHierarchy-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ exports[`SpaceHierarchy <SpaceHierarchy /> renders 1`] = `
>
<div
aria-disabled="true"
aria-label="Remove"
class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_danger_outline mx_AccessibleButton_disabled"
data-state="closed"
disabled=""
role="button"
tabindex="0"
Expand All @@ -41,7 +43,9 @@ exports[`SpaceHierarchy <SpaceHierarchy /> renders 1`] = `
</div>
<div
aria-disabled="true"
aria-label="Mark as not suggested"
class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_primary_outline mx_AccessibleButton_disabled"
data-state="closed"
disabled=""
role="button"
tabindex="0"
Expand Down
12 changes: 10 additions & 2 deletions test/components/views/spaces/__snapshots__/SpacePanel-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ exports[`<SpacePanel /> should show all activated MetaSpaces in the correct orde
<div
aria-label="Expand"
class="mx_AccessibleButton mx_SpacePanel_toggleCollapse"
data-state="closed"
role="button"
tabindex="0"
/>
Expand All @@ -55,6 +56,7 @@ exports[`<SpacePanel /> should show all activated MetaSpaces in the correct orde
<div
aria-label="Home"
class="mx_AccessibleButton mx_SpaceButton mx_SpaceButton_home mx_SpaceButton_narrow"
data-state="closed"
role="button"
tabindex="0"
>
Expand Down Expand Up @@ -91,6 +93,7 @@ exports[`<SpacePanel /> should show all activated MetaSpaces in the correct orde
<div
aria-label="Favourites"
class="mx_AccessibleButton mx_SpaceButton mx_SpaceButton_favourites mx_SpaceButton_narrow"
data-state="closed"
role="button"
tabindex="-1"
>
Expand Down Expand Up @@ -119,6 +122,7 @@ exports[`<SpacePanel /> should show all activated MetaSpaces in the correct orde
<div
aria-label="People"
class="mx_AccessibleButton mx_SpaceButton mx_SpaceButton_people mx_SpaceButton_narrow"
data-state="closed"
role="button"
tabindex="-1"
>
Expand Down Expand Up @@ -147,6 +151,7 @@ exports[`<SpacePanel /> should show all activated MetaSpaces in the correct orde
<div
aria-label="Other rooms"
class="mx_AccessibleButton mx_SpaceButton mx_SpaceButton_orphans mx_SpaceButton_narrow"
data-state="closed"
role="button"
tabindex="-1"
>
Expand Down Expand Up @@ -175,6 +180,7 @@ exports[`<SpacePanel /> should show all activated MetaSpaces in the correct orde
<div
aria-label="Conferences"
class="mx_AccessibleButton mx_SpaceButton mx_SpaceButton_videoRooms mx_SpaceButton_narrow"
data-state="closed"
role="button"
tabindex="-1"
>
Expand Down Expand Up @@ -203,6 +209,7 @@ exports[`<SpacePanel /> should show all activated MetaSpaces in the correct orde
<div
aria-label="Create a space"
class="mx_AccessibleButton mx_SpaceButton mx_SpaceButton_new mx_SpaceButton_narrow"
data-state="closed"
data-testid="create-space-button"
role="button"
tabindex="-1"
Expand All @@ -229,8 +236,8 @@ exports[`<SpacePanel /> should show all activated MetaSpaces in the correct orde
class="mx_ThreadsActivityCentre_container"
>
<button
aria-controls="floating-ui-7"
aria-describedby="floating-ui-7"
aria-controls="floating-ui-35"
aria-describedby="floating-ui-35"
aria-expanded="true"
aria-haspopup="dialog"
aria-label="Threads"
Expand Down Expand Up @@ -272,6 +279,7 @@ exports[`<SpacePanel /> should show all activated MetaSpaces in the correct orde
aria-expanded="false"
aria-label="Quick settings"
class="mx_AccessibleButton mx_QuickSettingsButton"
data-state="closed"
role="button"
tabindex="0"
/>
Expand Down
Loading