From ddbc6439ce255461aeb7bf77700b931491c4728a Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Fri, 8 Mar 2024 11:18:30 +0100 Subject: [PATCH] Fix spotlight opening in TAC (#12315) * Fix spotlight opening in TAC * Add tests * Remove `RovingTabIndexProvider` --- .../threadsActivityCentre.spec.ts | 16 ++++ .../structures/_ThreadsActivityCentre.pcss | 4 + .../ThreadsActivityCentre.tsx | 79 +++++++++++-------- .../spaces/ThreadsActivityCentre-test.tsx | 27 +++++++ 4 files changed, 95 insertions(+), 31 deletions(-) diff --git a/playwright/e2e/spaces/threads-activity-centre/threadsActivityCentre.spec.ts b/playwright/e2e/spaces/threads-activity-centre/threadsActivityCentre.spec.ts index 1b237c0b53..eb4d7c8df0 100644 --- a/playwright/e2e/spaces/threads-activity-centre/threadsActivityCentre.spec.ts +++ b/playwright/e2e/spaces/threads-activity-centre/threadsActivityCentre.spec.ts @@ -17,6 +17,7 @@ */ import { expect, test } from "."; +import { CommandOrControl } from "../../utils"; test.describe("Threads Activity Centre", () => { test.use({ @@ -118,4 +119,19 @@ test.describe("Threads Activity Centre", () => { ]); await expect(util.getTacPanel()).toMatchScreenshot("tac-panel-notification-unread.png"); }); + + test("should block the Spotlight to open when the TAC is opened", async ({ util, page }) => { + const toggleSpotlight = () => page.keyboard.press(`${CommandOrControl}+k`); + + // Sanity check + // Open and close the spotlight + await toggleSpotlight(); + await expect(page.locator(".mx_SpotlightDialog")).toBeVisible(); + await toggleSpotlight(); + + await util.openTac(); + // The spotlight should not be opened + await toggleSpotlight(); + await expect(page.locator(".mx_SpotlightDialog")).not.toBeVisible(); + }); }); diff --git a/res/css/structures/_ThreadsActivityCentre.pcss b/res/css/structures/_ThreadsActivityCentre.pcss index d803fd30e5..4f723700f2 100644 --- a/res/css/structures/_ThreadsActivityCentre.pcss +++ b/res/css/structures/_ThreadsActivityCentre.pcss @@ -16,6 +16,10 @@ * / */ +.mx_ThreadsActivityCentre_container { + display: flex; +} + .mx_ThreadsActivityCentreButton { border-radius: 8px; margin: 18px auto auto auto; diff --git a/src/components/views/spaces/threads-activity-centre/ThreadsActivityCentre.tsx b/src/components/views/spaces/threads-activity-centre/ThreadsActivityCentre.tsx index f6374ef32a..debf845164 100644 --- a/src/components/views/spaces/threads-activity-centre/ThreadsActivityCentre.tsx +++ b/src/components/views/spaces/threads-activity-centre/ThreadsActivityCentre.tsx @@ -32,6 +32,8 @@ import { useUnreadThreadRooms } from "./useUnreadThreadRooms"; import { StatelessNotificationBadge } from "../../rooms/NotificationBadge/StatelessNotificationBadge"; import { NotificationLevel } from "../../../../stores/notifications/NotificationLevel"; import PosthogTrackers from "../../../../PosthogTrackers"; +import { getKeyBindingsManager } from "../../../../KeyBindingsManager"; +import { KeyBindingAction } from "../../../../accessibility/KeyboardShortcuts"; interface ThreadsActivityCentreProps { /** @@ -49,41 +51,56 @@ export function ThreadsActivityCentre({ displayButtonLabel }: ThreadsActivityCen const roomsAndNotifications = useUnreadThreadRooms(open); return ( - { - // Track only when the Threads Activity Centre is opened - if (newOpen) PosthogTrackers.trackInteraction("WebThreadsActivityCentreButton"); +
{ + // Do nothing if the TAC is closed + if (!open) return; - setOpen(newOpen); + const action = getKeyBindingsManager().getNavigationAction(evt); + + // Block spotlight opening + if (action === KeyBindingAction.FilterRooms) { + evt.stopPropagation(); + } }} - side="right" - title={_t("threads_activity_centre|header")} - trigger={ - - } > - {/* Make the content of the pop-up scrollable */} -
- {roomsAndNotifications.rooms.map(({ room, notificationLevel }) => ( - setOpen(false)} + { + // Track only when the Threads Activity Centre is opened + if (newOpen) PosthogTrackers.trackInteraction("WebThreadsActivityCentreButton"); + + setOpen(newOpen); + }} + side="right" + title={_t("threads_activity_centre|header")} + trigger={ + - ))} - {roomsAndNotifications.rooms.length === 0 && ( -
- {_t("threads_activity_centre|no_rooms_with_unreads_threads")} -
- )} -
-
+ } + > + {/* Make the content of the pop-up scrollable */} +
+ {roomsAndNotifications.rooms.map(({ room, notificationLevel }) => ( + setOpen(false)} + /> + ))} + {roomsAndNotifications.rooms.length === 0 && ( +
+ {_t("threads_activity_centre|no_rooms_with_unreads_threads")} +
+ )} +
+ + ); } diff --git a/test/components/views/spaces/ThreadsActivityCentre-test.tsx b/test/components/views/spaces/ThreadsActivityCentre-test.tsx index d0a14bd805..f5f183e7c6 100644 --- a/test/components/views/spaces/ThreadsActivityCentre-test.tsx +++ b/test/components/views/spaces/ThreadsActivityCentre-test.tsx @@ -180,4 +180,31 @@ describe("ThreadsActivityCentre", () => { expect(screen.getByRole("menu")).toMatchSnapshot(); }); + + it("should block Ctrl/CMD + k shortcut", async () => { + cli.getVisibleRooms = jest.fn().mockReturnValue([roomWithHighlight]); + + const keyDownHandler = jest.fn(); + render( +
{ + keyDownHandler(evt.key, evt.ctrlKey); + }} + > + + + +
, + { wrapper: TooltipProvider }, + ); + await userEvent.click(getTACButton()); + + // CTRL/CMD + k should be blocked + await userEvent.keyboard("{Control>}k{/Control}"); + expect(keyDownHandler).not.toHaveBeenCalledWith("k", true); + + // Sanity test + await userEvent.keyboard("{Control>}a{/Control}"); + expect(keyDownHandler).toHaveBeenCalledWith("a", true); + }); });