Fix spotlight opening in TAC (#12315)

* Fix spotlight opening in TAC * Add tests * Remove `RovingTabIndexProvider`
matrix-org · Mar 8, 2024 · ddbc643 · ddbc643
1 parent 70365c8
commit ddbc643
Show file tree

Hide file tree

Showing 4 changed files with 95 additions and 31 deletions.
diff --git 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
@@ -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
@@ -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 (
-        <Menu
-            align="end"
-            open={open}
-            onOpenChange={(newOpen) => {
-                // Track only when the Threads Activity Centre is opened
-                if (newOpen) PosthogTrackers.trackInteraction("WebThreadsActivityCentreButton");
+        <div
+            className="mx_ThreadsActivityCentre_container"
+            onKeyDown={(evt) => {
+                // 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={
-                <ThreadsActivityCentreButton
-                    displayLabel={displayButtonLabel}
-                    notificationLevel={roomsAndNotifications.greatestNotificationLevel}
-                />
-            }
         >
-            {/* Make the content of the pop-up scrollable */}
-            <div className="mx_ThreadsActivity_rows">
-                {roomsAndNotifications.rooms.map(({ room, notificationLevel }) => (
-                    <ThreadsActivityRow
-                        key={room.roomId}
-                        room={room}
-                        notificationLevel={notificationLevel}
-                        onClick={() => setOpen(false)}
+            <Menu
+                align="end"
+                open={open}
+                onOpenChange={(newOpen) => {
+                    // 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={
+                    <ThreadsActivityCentreButton
+                        displayLabel={displayButtonLabel}
+                        notificationLevel={roomsAndNotifications.greatestNotificationLevel}
                     />
-                ))}
-                {roomsAndNotifications.rooms.length === 0 && (
-                    <div className="mx_ThreadsActivityCentre_emptyCaption">
-                        {_t("threads_activity_centre|no_rooms_with_unreads_threads")}
-                    </div>
-                )}
-            </div>
-        </Menu>
+                }
+            >
+                {/* Make the content of the pop-up scrollable */}
+                <div className="mx_ThreadsActivity_rows">
+                    {roomsAndNotifications.rooms.map(({ room, notificationLevel }) => (
+                        <ThreadsActivityRow
+                            key={room.roomId}
+                            room={room}
+                            notificationLevel={notificationLevel}
+                            onClick={() => setOpen(false)}
+                        />
+                    ))}
+                    {roomsAndNotifications.rooms.length === 0 && (
+                        <div className="mx_ThreadsActivityCentre_emptyCaption">
+                            {_t("threads_activity_centre|no_rooms_with_unreads_threads")}
+                        </div>
+                    )}
+                </div>
+            </Menu>
+        </div>
     );
 }
 

diff --git 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(
+            <div
+                onKeyDown={(evt) => {
+                    keyDownHandler(evt.key, evt.ctrlKey);
+                }}
+            >
+                <MatrixClientContext.Provider value={cli}>
+                    <ThreadsActivityCentre />
+                </MatrixClientContext.Provider>
+            </div>,
+            { 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);
+    });
 });