From 700b3955a4a83460f9bf0b446ed95676e1f6cc8e Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Wed, 24 Apr 2024 14:24:25 +0200 Subject: [PATCH] Add `Tooltip` to `AccessibleButton` (#12443) * Deprecate AccessibleTooltipButton Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Fix tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Fix tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Deprecate AccessibleTooltipButton Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Fix tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Fix `UserInfo-test.tsx` * Update `LoginWithQRFlow-test.tsx` snapshot * Remove tooltip provider from test * Fix `AccessibleButton` * Update snapshots * Revert to original import * Use title to populate aria-label * Rollback AccessibleButton or Tooltip changes. Will come in another PR * Rollback en.json change * Update snapshots * Fix `UserInfo` * Update snapshots * Use label instead of title in test * Use label instead of title in TAC test * Use label instead of title in read-receipt test * Remove tooltip for ContextMenu * Add extra information for caption field --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> --- playwright/e2e/read-receipts/index.ts | 6 ++-- .../spaces/threads-activity-centre/index.ts | 2 +- .../context_menu/ContextMenuButton.tsx | 1 - .../views/elements/AccessibleButton.tsx | 24 +++++++++++++- .../elements/AccessibleTooltipButton.tsx | 3 ++ src/components/views/right_panel/UserInfo.tsx | 7 +++- .../__snapshots__/UserMenu-test.tsx.snap | 1 - .../__snapshots__/DialogSidebar-test.tsx.snap | 6 ++-- .../LeftPanelLiveShareWarning-test.tsx.snap | 3 +- .../RoomLiveShareWarning-test.tsx.snap | 3 +- .../views/elements/AppTile-test.tsx | 8 ++--- .../__snapshots__/AppTile-test.tsx.snap | 25 ++++++++------- .../LocationViewDialog-test.tsx.snap | 6 ++-- .../__snapshots__/ZoomButtons-test.tsx.snap | 6 ++-- .../views/right_panel/UserInfo-test.tsx | 29 +++++++++-------- .../RoomSummaryCard-test.tsx.snap | 3 +- .../__snapshots__/UserInfo-test.tsx.snap | 12 ++++--- .../views/rooms/MemberList-test.tsx | 2 +- .../__snapshots__/MemberTile-test.tsx.snap | 9 ++++-- .../CurrentDeviceSection-test.tsx.snap | 32 ++++++++++++------- .../LoginWithQRFlow-test.tsx.snap | 18 +++++++---- .../PeopleRoomSettingsTab-test.tsx.snap | 12 ++++--- .../SessionManagerTab-test.tsx.snap | 4 +++ .../__snapshots__/SpacePanel-test.tsx.snap | 1 - 24 files changed, 147 insertions(+), 76 deletions(-) diff --git a/playwright/e2e/read-receipts/index.ts b/playwright/e2e/read-receipts/index.ts index 6b9a8381d2..4dd0450fb9 100644 --- a/playwright/e2e/read-receipts/index.ts +++ b/playwright/e2e/read-receipts/index.ts @@ -403,7 +403,7 @@ class Helpers { * tests we only open the threads panel.) */ async closeThreadsPanel() { - await this.page.locator(".mx_RightPanel").getByTitle("Close").click(); + await this.page.locator(".mx_RightPanel").getByLabel("Close").click(); await expect(this.page.locator(".mx_RightPanel")).not.toBeVisible(); } @@ -411,7 +411,7 @@ class Helpers { * Return to the list of threads, given we are viewing a single thread. */ async backToThreadsList() { - await this.page.locator(".mx_RightPanel").getByTitle("Threads").click(); + await this.page.locator(".mx_RightPanel").getByLabel("Threads").click(); } /** @@ -539,7 +539,7 @@ class Helpers { const threadPanel = this.page.locator(".mx_ThreadPanel"); await expect(threadPanel).toBeVisible(); await threadPanel.evaluate(($panel) => { - const $button = $panel.querySelector('.mx_BaseCard_back[title="Threads"]'); + const $button = $panel.querySelector('.mx_BaseCard_back[aria-label="Threads"]'); // If the Threads back button is present then click it - the // threads button can open either threads list or thread panel if ($button) { diff --git a/playwright/e2e/spaces/threads-activity-centre/index.ts b/playwright/e2e/spaces/threads-activity-centre/index.ts index 7ad477541a..8bafe2e804 100644 --- a/playwright/e2e/spaces/threads-activity-centre/index.ts +++ b/playwright/e2e/spaces/threads-activity-centre/index.ts @@ -341,7 +341,7 @@ export class Helpers { */ assertThreadPanelFocused() { return expect( - this.page.locator(".mx_ThreadPanel").locator(".mx_BaseCard_header").getByTitle("Close"), + this.page.locator(".mx_ThreadPanel").locator(".mx_BaseCard_header").getByLabel("Close"), ).toBeFocused(); } diff --git a/src/accessibility/context_menu/ContextMenuButton.tsx b/src/accessibility/context_menu/ContextMenuButton.tsx index 6ef6afef37..8f6113dbd2 100644 --- a/src/accessibility/context_menu/ContextMenuButton.tsx +++ b/src/accessibility/context_menu/ContextMenuButton.tsx @@ -36,7 +36,6 @@ export const ContextMenuButton = forwardRef(function = DynamicHtmlElementProps & * Event handler for button activation. Should be implemented exactly like a normal `onClick` handler. */ onClick: ((e: ButtonEvent) => void | Promise) | null; + /** + * The tooltip to show on hover or focus. + */ + title?: string; + /** + * The caption is a secondary text displayed under the `title` of the tooltip. + * Only valid when used in conjunction with `title`. + */ + caption?: string; }; /** @@ -116,11 +126,14 @@ const AccessibleButton = forwardRef(function , ref: Ref, ): JSX.Element { const newProps: RenderedElementProps = restProps; + newProps["aria-label"] = newProps["aria-label"] ?? title; if (disabled) { newProps["aria-disabled"] = true; newProps["disabled"] = true; @@ -182,7 +195,16 @@ const AccessibleButton = forwardRef(function + {button} + + ); + } + return button; }); // Type assertion required due to forwardRef type workaround in react.d.ts diff --git a/src/components/views/elements/AccessibleTooltipButton.tsx b/src/components/views/elements/AccessibleTooltipButton.tsx index 0af5cc9625..4bb5d6efff 100644 --- a/src/components/views/elements/AccessibleTooltipButton.tsx +++ b/src/components/views/elements/AccessibleTooltipButton.tsx @@ -60,6 +60,9 @@ type Props = ComponentProps( { title, tooltip, children, forceHide, alignment, onHideTooltip, tooltipClassName, ...props }: Props, ref: Ref, diff --git a/src/components/views/right_panel/UserInfo.tsx b/src/components/views/right_panel/UserInfo.tsx index 2e7b9bc9ab..dbc6acb29b 100644 --- a/src/components/views/right_panel/UserInfo.tsx +++ b/src/components/views/right_panel/UserInfo.tsx @@ -237,7 +237,12 @@ export function DeviceItem({ ); } else { return ( - +
{deviceName}
{trustedLabel}
diff --git a/test/components/structures/__snapshots__/UserMenu-test.tsx.snap b/test/components/structures/__snapshots__/UserMenu-test.tsx.snap index a43b718020..029db9bfd4 100644 --- a/test/components/structures/__snapshots__/UserMenu-test.tsx.snap +++ b/test/components/structures/__snapshots__/UserMenu-test.tsx.snap @@ -12,7 +12,6 @@ exports[` when video broadcast when rendered should render class="mx_AccessibleButton mx_UserMenu_contextMenuButton" role="button" tabindex="0" - title="User menu" >
renders sidebar correctly with beacons 1`] = ` View list
renders sidebar correctly without beacons 1`] = ` View list
when user has live location monitor renders correctly when minimized 1`] = `
when user has live beacons and geolocation is Retry
ambiguous display name 1`] = `
ambiguous display name 1`] = ` exports[` with display name 1`] = `
with display name 1`] = ` exports[` without display name 1`] = `
with crypto enabled renders 1`] = ` class="mx_BaseCard_header" >
{ let prevMember: RoomMember | undefined; for (const tile of memberTiles) { const memberA = prevMember; - const memberB = memberListRoom.currentState.members[tile.getAttribute("title")!.split(" ")[0]]; + const memberB = memberListRoom.currentState.members[tile.getAttribute("aria-label")!.split(" ")[0]]; prevMember = memberB; // just in case an expect fails, set this early if (!memberA) { continue; diff --git a/test/components/views/rooms/__snapshots__/MemberTile-test.tsx.snap b/test/components/views/rooms/__snapshots__/MemberTile-test.tsx.snap index f40db566bb..77c87d93da 100644 --- a/test/components/views/rooms/__snapshots__/MemberTile-test.tsx.snap +++ b/test/components/views/rooms/__snapshots__/MemberTile-test.tsx.snap @@ -4,10 +4,11 @@ exports[`MemberTile should display an verified E2EIcon when the e2E status = Ver
handles when device is falsy 1`] = ` > Current session -
renders device and correct security card when