Modal refactor attempts.

src/Modal.tsx

@@ -27,6 +27,11 @@ import { useMediaQuery } from "./useMediaQuery";
 
 export interface Props {
   title: string;
+  /**
+   * Hide the modal header. Used for smaller popups where the context is readily apparent.
+   * A title should still be specified for users using assistive technology.
+   */
+  hideHeader?: boolean;
   children: ReactNode;
   className?: string;
   /**
@@ -54,6 +59,7 @@ export interface Props {
  */
 export const Modal: FC<Props> = ({
   title,
+  hideHeader,
   children,
   className,
   open,
@@ -108,6 +114,28 @@ export const Modal: FC<Props> = ({
       </Drawer.Root>
     );
   } else {
+    const titleNode = (
+      <DialogTitle asChild>
+        <Heading as="h2" weight="semibold" size="md">
+          {title}
+        </Heading>
+      </DialogTitle>
+    );
+    const header = (
+      <div className={styles.header}>
+        {titleNode}
+        {onDismiss !== undefined && (
+          <DialogClose
+            className={styles.close}
+            data-testid="modal_close"
+            aria-label={t("action.close")}
+          >
+            <CloseIcon width={20} height={20} />
+          </DialogClose>
+        )}
+      </div>
+    );
+
     return (
       <DialogRoot open={open} onOpenChange={onOpenChange}>
         <DialogPortal>
@@ -119,31 +147,19 @@ export const Modal: FC<Props> = ({
           <DialogContent asChild aria-describedby={undefined} {...rest}>
             <Glass
               className={classNames(
-                className,
                 overlayStyles.overlay,
                 overlayStyles.animate,
                 styles.modal,
                 styles.dialog,
                 { [styles.tabbed]: tabbed },
+                className,
               )}
             >
               <div className={styles.content}>
-                <div className={styles.header}>
+                {!hideHeader ? header : null}
-                  <DialogTitle asChild>
+                {hideHeader ? (
-                    <Heading as="h2" weight="semibold" size="md">
+                  <VisuallyHidden asChild>{titleNode}</VisuallyHidden>
-                      {title}
+                ) : null}
-                    </Heading>
-                  </DialogTitle>
-                  {onDismiss !== undefined && (
-                    <DialogClose
-                      className={styles.close}
-                      data-testid="modal_close"
-                      aria-label={t("action.close")}
-                    >
-                      <CloseIcon width={20} height={20} />
-                    </DialogClose>
-                  )}
-                </div>
                 <div className={styles.body}>{children}</div>
               </div>
             </Glass>

src/button/ReactionToggleButton.module.css

@@ -3,13 +3,20 @@
 }
 
 .reactionPopupMenu {
-  background: var(--cpd-color-bg-canvas-default);
+  display: flex;
-  border-radius: var(--cpd-space-4x);
+}
-  width: fit-content;
+
-  top: 70vh;
+/* These styles override the Modal styles to ensure our emoji
-  padding: 1em;
+   picker appears near the toolbar */
-  position: absolute;
+
-  border: none;
+.reactionPopupMenuModal {
+  width: fit-content !important;
+  top: 82vh !important;
+}
+
+.reactionPopupMenuModal > div > div {
+  padding-inline: var(--cpd-space-6x) !important;
+  padding-block: var(--cpd-space-6x) var(--cpd-space-8x) !important;
 }
 
 .reactionPopupMenu menu {
@@ -35,6 +42,8 @@
 }
 
 .reactionButton {
+  padding: 1em;
+  font-size: 2em;
   width: 2em;
   height: 2em;
   border-radius: 2em;

src/button/ReactionToggleButton.tsx

@@ -22,14 +22,10 @@ import {
   ChangeEventHandler,
   ComponentPropsWithoutRef,
   FC,
-  forwardRef,
   KeyboardEventHandler,
-  PropsWithRef,
   ReactNode,
   useCallback,
-  useEffect,
   useMemo,
-  useRef,
   useState,
 } from "react";
 import { useTranslation } from "react-i18next";
@@ -47,6 +43,7 @@ import {
   ReactionSet,
   ElementCallReactionEventType,
 } from "../reactions";
+import { Modal } from "../Modal";
 
 interface InnerButtonProps extends ComponentPropsWithoutRef<"button"> {
   raised: boolean;
@@ -70,17 +67,17 @@ const InnerButton: FC<InnerButtonProps> = ({ raised, open, ...props }) => {
   );
 };
 
-interface ReactionsPopupMenuProps {
+export function ReactionPopupMenu({
+  sendReaction,
+  toggleRaisedHand,
+  isHandRaised,
+  canReact,
+}: {
   sendReaction: (reaction: ReactionOption) => void;
   toggleRaisedHand: () => void;
   isHandRaised: boolean;
   canReact: boolean;
-}
+}): ReactNode {
-
-export const ReactionPopupMenu = forwardRef<
-  HTMLDialogElement,
-  ReactionsPopupMenuProps
->(({ sendReaction, toggleRaisedHand, isHandRaised, canReact }, ref) => {
   const { t } = useTranslation();
   const [searchText, setSearchText] = useState("");
   const [isSearching, setIsSearching] = useState(false);
@@ -122,84 +119,82 @@ export const ReactionPopupMenu = forwardRef<
   );
 
   return (
-    <dialog ref={ref} className={styles.reactionPopupMenu} open={false}>
+    <div className={styles.reactionPopupMenu}>
-      <div style={{ display: "flex" }}>
+      <section className={styles.handRaiseSection}>
-        <section className={styles.handRaiseSection}>
+        <Tooltip label={t("common.raise_hand")}>
-          <Tooltip label={t("common.raise_hand")}>
+          <CpdButton
-            <CpdButton
+            kind={isHandRaised ? "primary" : "secondary"}
-              kind={isHandRaised ? "primary" : "secondary"}
+            aria-pressed={isHandRaised}
-              aria-pressed={isHandRaised}
+            aria-label="Toggle hand raised"
-              aria-label="Toggle hand raised"
+            className={styles.reactionButton}
-              className={styles.reactionButton}
+            key="raise-hand"
-              key="raise-hand"
+            onClick={() => toggleRaisedHand()}
-              onClick={() => toggleRaisedHand()}
+          >
-            >
+            🖐️
-              🖐️
+          </CpdButton>
-            </CpdButton>
+        </Tooltip>
-          </Tooltip>
+      </section>
-        </section>
+      <div className={styles.verticalSeperator} />
-        <div className={styles.verticalSeperator} />
+      <section>
-        <section>
+        {isSearching ? (
-          {isSearching ? (
+          <>
-            <>
+            <Form.Root className={styles.searchForm}>
-              <Form.Root className={styles.searchForm}>
+              <Search
-                <Search
+                required
-                  required
+                value={searchText}
-                  value={searchText}
+                name="reactionSearch"
-                  name="reactionSearch"
+                placeholder="Search reactions…"
-                  placeholder="Search reactions…"
+                onChange={onSearch}
-                  onChange={onSearch}
+                onKeyDown={onSearchKeyDown}
-                  onKeyDown={onSearchKeyDown}
+                // This is a reasonable use of autofocus, we are focusing when
-                  // This is a reasonable use of autofocus, we are focusing when
+                // the search button is clicked (which matches the Element Web reaction picker)
-                  // the search button is clicked (which matches the Element Web reaction picker)
+                // eslint-disable-next-line jsx-a11y/no-autofocus
-                  // eslint-disable-next-line jsx-a11y/no-autofocus
+                autoFocus
-                  autoFocus
+              />
-                />
+              <CpdButton
+                Icon={CloseIcon}
+                aria-label="close search"
+                size="sm"
+                kind="destructive"
+                onClick={() => setIsSearching(false)}
+              />
+            </Form.Root>
+            <Separator />
+          </>
+        ) : null}
+        <menu>
+          {filteredReactionSet.map((reaction) => (
+            <li className={styles.reactionPopupMenuItem} key={reaction.name}>
+              <Tooltip label={reaction.name}>
                 <CpdButton
-                  Icon={CloseIcon}
+                  kind="secondary"
-                  aria-label="close search"
+                  className={styles.reactionButton}
-                  size="sm"
+                  disabled={!canReact}
-                  kind="destructive"
+                  onClick={() => sendReaction(reaction)}
-                  onClick={() => setIsSearching(false)}
+                >
+                  {reaction.emoji}
+                </CpdButton>
+              </Tooltip>
+            </li>
+          ))}
+          {!isSearching ? (
+            <li key="search" className={styles.reactionPopupMenuItem}>
+              <Tooltip label="Search">
+                <CpdButton
+                  iconOnly
+                  aria-label="Open reactions search"
+                  Icon={SearchIcon}
+                  kind="tertiary"
+                  onClick={() => setIsSearching(true)}
                 />
-              </Form.Root>
+              </Tooltip>
-              <Separator />
+            </li>
-            </>
           ) : null}
-          <menu>
+        </menu>
-            {filteredReactionSet.map((reaction) => (
+      </section>
-              <li className={styles.reactionPopupMenuItem} key={reaction.name}>
+    </div>
-                <Tooltip label={reaction.name}>
-                  <CpdButton
-                    kind="secondary"
-                    className={styles.reactionButton}
-                    disabled={!canReact}
-                    onClick={() => sendReaction(reaction)}
-                  >
-                    {reaction.emoji}
-                  </CpdButton>
-                </Tooltip>
-              </li>
-            ))}
-            {!isSearching ? (
-              <li key="search" className={styles.reactionPopupMenuItem}>
-                <Tooltip label="Search">
-                  <CpdButton
-                    iconOnly
-                    aria-label="Open reactions search"
-                    Icon={SearchIcon}
-                    kind="tertiary"
-                    onClick={() => setIsSearching(true)}
-                  />
-                </Tooltip>
-              </li>
-            ) : null}
-          </menu>
-        </section>
-      </div>
-    </dialog>
   );
-});
+}
 
 interface ReactionToggleButtonProps {
   rtcSession: MatrixRTCSession;
@@ -215,37 +210,10 @@ export function ReactionToggleButton({
   const userId = client.getUserId()!;
   const isHandRaised = !!raisedHands[userId];
   const memberships = useMatrixRTCSessionMemberships(rtcSession);
-  const ref = useRef<HTMLDialogElement>(null);
+  const [showReactionsMenu, setShowReactionsMenu] = useState(false);
 
   const canReact = !reactions[userId];
 
-  const showReactionsMenu = useCallback(() => {
-    if (ref.current) {
-      ref.current.showModal();
-    }
-  }, [ref]);
-
-  const hideReactionsMenu = useCallback(() => {
-    if (ref.current) {
-      ref.current.close();
-    }
-  }, [ref]);
-
-  useEffect(() => {
-    if (!ref.current) {
-      return;
-    }
-    function onClick(evt: MouseEvent) {
-      if (evt.target === ref.current) {
-        hideReactionsMenu();
-      }
-    }
-    ref.current.addEventListener("click", onClick);
-    return () => {
-      ref.current?.removeEventListener("click", onClick);
-    };
-  }, [ref]);
-
   const sendRelation = useCallback(
     async (reaction: ReactionOption) => {
       const myMembership = memberships.find((m) => m.sender === userId);
@@ -273,7 +241,7 @@ export function ReactionToggleButton({
         logger.error("Failed to send reaction", ex);
       } finally {
         setBusy(false);
-        hideReactionsMenu();
+        setShowReactionsMenu(false);
       }
     },
     [memberships, client, userId, rtcSession],
@@ -285,6 +253,7 @@ export function ReactionToggleButton({
         try {
           setBusy(true);
           await lowerHand();
+          setShowReactionsMenu(false);
         } finally {
           setBusy(false);
         }
@@ -313,7 +282,7 @@ export function ReactionToggleButton({
           logger.error("Failed to send reaction event", ex);
         } finally {
           setBusy(false);
-          hideReactionsMenu();
+          setShowReactionsMenu(false);
         }
       }
     };
@@ -332,17 +301,24 @@ export function ReactionToggleButton({
     <>
       <InnerButton
         disabled={busy}
-        onClick={showReactionsMenu}
+        onClick={() => setShowReactionsMenu((show) => !show)}
-        raised={isHandRaised}
+        raised={isHandRaised || showReactionsMenu}
-        open={!!ref.current?.open}
+        open={showReactionsMenu}
-      />
-      <ReactionPopupMenu
-        ref={ref}
-        isHandRaised={isHandRaised}
-        canReact={canReact}
-        sendReaction={(reaction) => void sendRelation(reaction)}
-        toggleRaisedHand={toggleRaisedHand}
       />
+      <Modal
+        open={showReactionsMenu}
+        title="Pick reaction"
+        hideHeader
+        className={styles.reactionPopupMenuModal}
+        onDismiss={() => setShowReactionsMenu(false)}
+      >
+        <ReactionPopupMenu
+          isHandRaised={isHandRaised}
+          canReact={canReact}
+          sendReaction={(reaction) => void sendRelation(reaction)}
+          toggleRaisedHand={toggleRaisedHand}
+        />
+      </Modal>
     </>
   );
 }