Merge pull request #4150 from element-hq/more-performance

Performance: Avoid re-rendering context menus so often

src/tile/GridTile.test.tsx

@@ -5,7 +5,10 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
 Please see LICENSE in the repository root for full details.
 */
 
-import { type RemoteTrackPublication } from "livekit-client";
+import {
+  type LocalTrackPublication,
+  type RemoteTrackPublication,
+} from "livekit-client";
 import { test, expect } from "vitest";
 import { act, render, screen } from "@testing-library/react";
 import { axe } from "vitest-axe";
@@ -17,6 +20,9 @@ import {
   mockRtcMembership,
   mockRemoteMedia,
   mockRemoteParticipant,
+  mockLocalMedia,
+  mockLocalParticipant,
+  mockMediaDevices,
 } from "../utils/test";
 import { GridTileViewModel } from "../state/TileViewModel";
 import { ReactionsSenderProvider } from "../reactions/useReactionsSender";
@@ -54,7 +60,7 @@ const callVm = {
   handsRaised$: constant({}),
 } as Partial<CallViewModel> as CallViewModel;
 
-test("GridTile is accessible", async () => {
+test("GridTile displays remote media", async () => {
   const vm = mockRemoteMedia(
     mockRtcMembership("@alice:example.org", "AAAA"),
     {
@@ -88,6 +94,40 @@ test("GridTile is accessible", async () => {
   screen.getByText("Alice");
 });
 
+test("GridTile displays local media", async () => {
+  const vm = mockLocalMedia(
+    mockRtcMembership("@alice:example.org", "AAAA"),
+    {
+      rawDisplayName: "Alice",
+      getMxcAvatarUrl: () => "mxc://adfsg",
+    },
+    mockLocalParticipant({
+      getTrackPublication: () =>
+        ({}) as Partial<LocalTrackPublication> as LocalTrackPublication,
+    }),
+    mockMediaDevices({}),
+  );
+
+  const { container } = render(
+    <ReactionsSenderProvider vm={callVm} rtcSession={fakeRtcSession}>
+      <GridTile
+        vm={new GridTileViewModel(constant(vm))}
+        onOpenProfile={() => {}}
+        targetWidth={300}
+        targetHeight={200}
+        showSpeakingIndicators
+        showNameTags
+        showRingingStatus
+        showOutline
+        focusable
+      />
+    </ReactionsSenderProvider>,
+  );
+  expect(await axe(container)).toHaveNoViolations();
+  // Name should be visible
+  screen.getByText("Alice");
+});
+
 test("GridTile displays ringing media", async () => {
   const pickupState$ = new BehaviorSubject<
     RingingMediaViewModel["pickupState$"]["value"]

src/tile/GridTile.tsx

@@ -14,6 +14,7 @@ import {
   useEffect,
   useRef,
   useState,
+  useMemo,
 } from "react";
 import { type animated } from "@react-spring/web";
 import classNames from "classnames";
@@ -105,20 +106,22 @@ interface UserMediaTileProps extends TileProps {
   playbackMuted: boolean;
   waitingForMedia?: boolean;
   primaryButton?: ReactNode;
-  menuStart?: ReactNode;
-  menuEnd?: ReactNode;
   focusUrl: string | undefined;
 }
 
-const UserMediaTile: FC<UserMediaTileProps> = ({
+/**
+ * A user media tile without a context menu.
+ */
+// The context menu is kept separate from this component for performance
+// reasons (c.f. UserMediaTile)
+const UserMediaTileInner: FC<UserMediaTileProps & { menu: ReactNode }> = ({
   ref,
   vm,
   showSpeakingIndicators,
   playbackMuted,
   waitingForMedia,
   primaryButton,
-  menuStart,
+  menu,
-  menuEnd,
   className,
   focusUrl,
   displayName,
@@ -166,24 +169,26 @@ const UserMediaTile: FC<UserMediaTileProps> = ({
       : t("microphone_off");
 
   const [menuOpen, setMenuOpen] = useState(false);
-  const menu = (
+  const menuTrigger = useMemo(
-    <>
+    () => (
-      {menuStart}
+      <button
-      {/*
+        aria-label={t("common.options")}
-       No additional menu item (used to be the manual fit to frame.
+        tabIndex={focusable ? undefined : -1}
-       Placeholder for future menu items that should be placed here.
+      >
-       */}
+        <OverflowHorizontalIcon aria-hidden width={20} height={20} />
-      {menuEnd}
+      </button>
-    </>
+    ),
+    [t, focusable],
   );
 
-  const raisedHandOnClick = vm.local
+  const raisedHandOnClick = useMemo(
-    ? (): void => void toggleRaisedHand()
+    () => (vm.local ? (): void => void toggleRaisedHand() : undefined),
-    : undefined;
+    [vm.local, toggleRaisedHand],
+  );
 
   const showSpeaking = showSpeakingIndicators && speaking;
 
-  const tile = (
+  return (
     <MediaView
       ref={ref}
       video={video}
@@ -213,14 +218,7 @@ const UserMediaTile: FC<UserMediaTileProps> = ({
             open={menuOpen}
             onOpenChange={setMenuOpen}
             title={displayName}
-            trigger={
+            trigger={menuTrigger}
-              <button
-                aria-label={t("common.options")}
-                tabIndex={focusable ? undefined : -1}
-              >
-                <OverflowHorizontalIcon aria-hidden width={20} height={20} />
-              </button>
-            }
             side="left"
             align="start"
           >
@@ -241,9 +239,37 @@ const UserMediaTile: FC<UserMediaTileProps> = ({
       {...props}
     />
   );
+};
 
+/**
+ * A user media tile enhanced with a context menu.
+ */
+const UserMediaTile: FC<
+  UserMediaTileProps & { menuStart?: ReactNode; menuEnd?: ReactNode }
+> = ({ menuStart, menuEnd, ...props }) => {
+  const menu = useMemo(
+    () => (
+      <>
+        {menuStart}
+        {/*
+       No additional menu item (used to be the manual fit to frame.
+       Placeholder for future menu items that should be placed here.
+       */}
+        {menuEnd}
+      </>
+    ),
+    [menuStart, menuEnd],
+  );
+
+  // ContextMenu is expensive to render, so we avoid subscribing to any
+  // frequently-changing behaviors here and instead keep them isolated in the
+  // UserMediaTileInner component
   return (
-    <ContextMenu title={displayName} trigger={tile} hasAccessibleAlternative>
+    <ContextMenu
+      title={props.displayName}
+      trigger={<UserMediaTileInner {...props} menu={menu} />}
+      hasAccessibleAlternative
+    >
       {menu}
     </ContextMenu>
   );
@@ -279,6 +305,29 @@ const LocalUserMediaTile: FC<LocalUserMediaTileProps> = ({
     [vm, latestAlwaysShow],
   );
 
+  const menuStart = useMemo(
+    () => (
+      <ToggleMenuItem
+        Icon={VisibilityOnIcon}
+        label={t("video_tile.always_show")}
+        checked={alwaysShow}
+        onSelect={onSelectAlwaysShow}
+      />
+    ),
+    [t, alwaysShow, onSelectAlwaysShow],
+  );
+  const menuEnd = useMemo(
+    () =>
+      onOpenProfile && (
+        <MenuItem
+          Icon={UserProfileIcon}
+          label={t("common.profile")}
+          onSelect={onOpenProfile}
+        />
+      ),
+    [t, onOpenProfile],
+  );
+
   return (
     <UserMediaTile
       ref={ref}
@@ -297,23 +346,8 @@ const LocalUserMediaTile: FC<LocalUserMediaTileProps> = ({
           </button>
         )
       }
-      menuStart={
+      menuStart={menuStart}
-        <ToggleMenuItem
+      menuEnd={menuEnd}
-          Icon={VisibilityOnIcon}
-          label={t("video_tile.always_show")}
-          checked={alwaysShow}
-          onSelect={onSelectAlwaysShow}
-        />
-      }
-      menuEnd={
-        onOpenProfile && (
-          <MenuItem
-            Icon={UserProfileIcon}
-            label={t("common.profile")}
-            onSelect={onOpenProfile}
-          />
-        )
-      }
       focusable={focusable}
       focusUrl={focusUrl}
       {...props}