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

Improve new room header accessibility #12725

Merged
merged 5 commits into from
Jul 8, 2024
Merged

Improve new room header accessibility #12725

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cb4ade9
Fix room header topic not showing on keyboard navigation whilst still…
t3chguy Jul 4, 2024
0f17ee6
Fix keyboard activation of the room header FacePile
t3chguy Jul 4, 2024
5c59bdb
Fix label on room header facepile
t3chguy Jul 4, 2024
4185a40
Comment
t3chguy Jul 5, 2024
5058f80
Update src/components/views/elements/FacePile.tsx
t3chguy Jul 8, 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
12 changes: 4 additions & 8 deletions src/components/views/elements/FacePile.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { AvatarStack, Tooltip } from "@vector-im/compound-web";
import MemberAvatar from "../avatars/MemberAvatar";
import AccessibleButton, { ButtonEvent } from "./AccessibleButton";

interface IProps extends HTMLAttributes<HTMLSpanElement> {
interface IProps extends Omit<HTMLAttributes<HTMLDivElement>, "onChange"> {
members: RoomMember[];
size: string;
overflow: boolean;
Expand All @@ -40,19 +40,15 @@ const FacePile: FC<IProps> = ({
tooltipShortcut,
children,
viewUserOnClick = true,
onClick,
...props
}) => {
const faces = members.map(
tooltipLabel
? (m) => <MemberAvatar key={m.userId} member={m} size={size} hideTitle />
: (m) => (
<Tooltip key={m.userId} label={m.name} caption={tooltipShortcut}>
<MemberAvatar
member={m}
size={size}
viewUserOnClick={!props.onClick && viewUserOnClick}
hideTitle
/>
<MemberAvatar member={m} size={size} viewUserOnClick={!onClick && viewUserOnClick} hideTitle />
</Tooltip>
),
);
Expand All @@ -65,7 +61,7 @@ const FacePile: FC<IProps> = ({
);

const content = (
<AccessibleButton className="mx_FacePile" onClick={props.onClick ?? null}>
<AccessibleButton {...props} className="mx_FacePile" onClick={onClick ?? null}>
Copy link
Member

Choose a reason for hiding this comment

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

is the ?? null required?

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 because undefined != null, AccessibleButton does not accept undefined

<AvatarStack>{pileContents}</AvatarStack>
{children}
</AccessibleButton>
Expand Down
8 changes: 2 additions & 6 deletions src/components/views/rooms/RoomHeader.tsx
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

Fix keyboard activation of the room header FacePile

Guess I'm not sure what a room header FacePile is either :/

Copy link
Member Author

Choose a reason for hiding this comment

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

image

The FacePile component within the RoomHeader component

Original file line number Diff line number Diff line change
Expand Up @@ -365,12 +365,7 @@ export default function RoomHeader({
)}
</Flex>
{!isDirectMessage && (
<BodyText
as="div"
size="sm"
weight="medium"
aria-label={_t("common|n_members", { count: memberCount })}
>
<BodyText as="div" size="sm" weight="medium">
<FacePile
className="mx_RoomHeader_members"
members={members.slice(0, 3)}
Expand All @@ -382,6 +377,7 @@ export default function RoomHeader({
RightPanelStore.instance.showOrHidePanel(RightPanelPhases.RoomMemberList);
e.stopPropagation();
}}
aria-label={_t("common|n_members", { count: memberCount })}
>
{formatCount(memberCount)}
</FacePile>
Expand Down
Loading