Test SpotlightTile more thoroughly

Catching two accessibility issues along the way: we were putting the wrong accessible labels on the 'expand' button, and even the off-screen pages of the spotlight tile were being exposed to accessibility technologies rather than hidden.

public/locales/en-GB/app.json

@@ -161,8 +161,8 @@
   "video_tile": {
     "always_show": "Always show",
     "change_fit_contain": "Fit to frame",
-    "exit_full_screen": "Exit full screen",
-    "full_screen": "Full screen",
+    "collapse": "Collapse",
+    "expand": "Expand",
     "mute_for_me": "Mute for me",
     "volume": "Volume"
   }

src/state/MediaViewModel.test.ts

@@ -81,14 +81,14 @@ test("toggle fit/contain for a participant's video", async () => {
 });
 
 test("local media remembers whether it should always be shown", async () => {
-  await withLocalMedia(async (vm) =>
+  await withLocalMedia({}, async (vm) =>
     withTestScheduler(({ expectObservable, schedule }) => {
       schedule("-a|", { a: () => vm.setAlwaysShow(false) });
       expectObservable(vm.alwaysShow).toBe("ab", { a: true, b: false });
     }),
   );
   // Next local media should start out *not* always shown
-  await withLocalMedia(async (vm) =>
+  await withLocalMedia({}, async (vm) =>
     withTestScheduler(({ expectObservable, schedule }) => {
       schedule("-a|", { a: () => vm.setAlwaysShow(true) });
       expectObservable(vm.alwaysShow).toBe("ab", { a: false, b: true });

src/tile/SpotlightTile.test.tsx

@@ -14,13 +14,13 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import { RemoteTrackPublication } from "livekit-client";
-import { test, expect } from "vitest";
-import { render, screen } from "@testing-library/react";
+import { test, expect, vi } from "vitest";
+import { isInaccessible, render, screen } from "@testing-library/react";
 import { axe } from "vitest-axe";
+import userEvent from "@testing-library/user-event";
 
 import { SpotlightTile } from "./SpotlightTile";
-import { withRemoteMedia } from "../utils/test";
+import { withLocalMedia, withRemoteMedia } from "../utils/test";
 
 global.IntersectionObserver = class MockIntersectionObserver {
   public observe(): void {}
@@ -33,25 +33,50 @@ test("SpotlightTile is accessible", async () => {
       rawDisplayName: "Alice",
       getMxcAvatarUrl: () => "mxc://adfsg",
     },
-    {
-      getTrackPublication: () =>
-        ({}) as Partial<RemoteTrackPublication> as RemoteTrackPublication,
-    },
-    async (vm) => {
-      const { container } = render(
-        <SpotlightTile
-          vms={[vm]}
-          targetWidth={300}
-          targetHeight={200}
-          maximised={false}
-          expanded={false}
-          onToggleExpanded={() => {}}
-          showIndicators
-        />,
+    {},
+    async (vm1) => {
+      await withLocalMedia(
+        {
+          rawDisplayName: "Bob",
+          getMxcAvatarUrl: () => "mxc://dlskf",
+        },
+        async (vm2) => {
+          const user = userEvent.setup();
+          const toggleExpanded = vi.fn();
+          const { container } = render(
+            <SpotlightTile
+              vms={[vm1, vm2]}
+              targetWidth={300}
+              targetHeight={200}
+              maximised={false}
+              expanded={false}
+              onToggleExpanded={toggleExpanded}
+              showIndicators
+            />,
+          );
+
+          expect(await axe(container)).toHaveNoViolations();
+          // Alice should be in the spotlight, with her name and avatar on the
+          // first page
+          screen.getByText("Alice");
+          const aliceAvatar = screen.getByRole("img");
+          expect(screen.queryByRole("button", { name: "common.back" })).toBe(
+            null,
+          );
+          // Bob should be out of the spotlight, and therefore invisible
+          expect(isInaccessible(screen.getByText("Bob"))).toBe(true);
+          // Now navigate to Bob
+          await user.click(screen.getByRole("button", { name: "common.next" }));
+          screen.getByText("Bob");
+          expect(screen.getByRole("img")).not.toBe(aliceAvatar);
+          expect(isInaccessible(screen.getByText("Alice"))).toBe(true);
+          // Can toggle whether the tile is expanded
+          await user.click(
+            screen.getByRole("button", { name: "video_tile.expand" }),
+          );
+          expect(toggleExpanded).toHaveBeenCalled();
+        },
       );
-      expect(await axe(container)).toHaveNoViolations();
-      // Name should be visible
-      screen.getByText("Alice");
     },
   );
 });

src/tile/SpotlightTile.tsx

@@ -61,6 +61,7 @@ interface SpotlightItemBaseProps {
   member: RoomMember | undefined;
   unencryptedWarning: boolean;
   displayName: string;
+  "aria-hidden"?: boolean;
 }
 
 interface SpotlightUserMediaItemBaseProps extends SpotlightItemBaseProps {
@@ -118,10 +119,21 @@ interface SpotlightItemProps {
    * Whether this item should act as a scroll snapping point.
    */
   snap: boolean;
+  "aria-hidden"?: boolean;
 }
 
 const SpotlightItem = forwardRef<HTMLDivElement, SpotlightItemProps>(
-  ({ vm, targetWidth, targetHeight, intersectionObserver, snap }, theirRef) => {
+  (
+    {
+      vm,
+      targetWidth,
+      targetHeight,
+      intersectionObserver,
+      snap,
+      "aria-hidden": ariaHidden,
+    },
+    theirRef,
+  ) => {
     const ourRef = useRef<HTMLDivElement | null>(null);
     const ref = useMergedRefs(ourRef, theirRef);
     const displayName = useDisplayName(vm);
@@ -153,6 +165,7 @@ const SpotlightItem = forwardRef<HTMLDivElement, SpotlightItemProps>(
       member: vm.member,
       unencryptedWarning,
       displayName,
+      "aria-hidden": ariaHidden,
     };
 
     return vm instanceof ScreenShareViewModel ? (
@@ -280,7 +293,12 @@ export const SpotlightTile = forwardRef<HTMLDivElement, Props>(
               targetWidth={targetWidth}
               targetHeight={targetHeight}
               intersectionObserver={intersectionObserver}
+              // This is how we get the container to scroll to the right media
+              // when the previous/next buttons are clicked: we temporarily
+              // remove all scroll snap points except for just the one media
+              // that we want to bring into view
               snap={scrollToId === null || scrollToId === vm.id}
+              aria-hidden={(scrollToId ?? visibleId) !== vm.id}
             />
           ))}
         </div>
@@ -288,9 +306,7 @@ export const SpotlightTile = forwardRef<HTMLDivElement, Props>(
           <button
             className={classNames(styles.expand)}
             aria-label={
-              expanded
-                ? t("video_tile.full_screen")
-                : t("video_tile.exit_full_screen")
+              expanded ? t("video_tile.collapse") : t("video_tile.expand")
             }
             onClick={onToggleExpanded}
           >

src/utils/test.ts

@@ -17,7 +17,12 @@ import { map } from "rxjs";
 import { RunHelpers, TestScheduler } from "rxjs/testing";
 import { expect, vi } from "vitest";
 import { RoomMember } from "matrix-js-sdk/src/matrix";
-import { LocalParticipant, RemoteParticipant } from "livekit-client";
+import {
+  LocalParticipant,
+  LocalTrackPublication,
+  RemoteParticipant,
+  RemoteTrackPublication,
+} from "livekit-client";
 
 import {
   LocalUserMediaViewModel,
@@ -66,14 +71,47 @@ export function withTestScheduler(
   );
 }
 
+function mockMember(member: Partial<RoomMember>): RoomMember {
+  return {
+    on() {
+      return this;
+    },
+    off() {
+      return this;
+    },
+    addListener() {
+      return this;
+    },
+    removeListener() {
+      return this;
+    },
+    ...member,
+  } as RoomMember;
+}
+
 export async function withLocalMedia(
+  member: Partial<RoomMember>,
   continuation: (vm: LocalUserMediaViewModel) => Promise<void>,
 ): Promise<void> {
-  const member = {} as unknown as RoomMember;
   const vm = new LocalUserMediaViewModel(
-    "a",
-    member,
-    {} as Partial<LocalParticipant> as LocalParticipant,
+    "local",
+    mockMember(member),
+    {
+      getTrackPublication: () =>
+        ({}) as Partial<LocalTrackPublication> as LocalTrackPublication,
+      on() {
+        return this as LocalParticipant;
+      },
+      off() {
+        return this as LocalParticipant;
+      },
+      addListener() {
+        return this as LocalParticipant;
+      },
+      removeListener() {
+        return this as LocalParticipant;
+      },
+    } as Partial<LocalParticipant> as LocalParticipant,
     true,
   );
   try {
@@ -89,24 +127,12 @@ export async function withRemoteMedia(
   continuation: (vm: RemoteUserMediaViewModel) => Promise<void>,
 ): Promise<void> {
   const vm = new RemoteUserMediaViewModel(
-    "a",
-    {
-      on() {
-        return this;
-      },
-      off() {
-        return this;
-      },
-      addListener() {
-        return this;
-      },
-      removeListener() {
-        return this;
-      },
-      ...member,
-    } as RoomMember,
+    "remote",
+    mockMember(member),
     {
       setVolume() {},
+      getTrackPublication: () =>
+        ({}) as Partial<RemoteTrackPublication> as RemoteTrackPublication,
       on() {
         return this;
       },