From ebfe971b4d879af5c1e8e82aca0b19cd0e228913 Mon Sep 17 00:00:00 2001
From: automated-signal <37887102+automated-signal@users.noreply.github.com>
Date: Mon, 4 Mar 2024 17:58:15 -0600
Subject: [PATCH] Fix supertab

Co-authored-by: Jamie Kyle <113370520+jamiebuilds-signal@users.noreply.github.com>
---
 ts/components/NavTabs.tsx                 |  2 +-
 ts/components/Preferences.tsx             |  7 ++--
 ts/services/addGlobalKeyboardShortcuts.ts | 41 +++++++++++++----------
 ts/state/smart/ConversationPanel.tsx      |  8 ++---
 ts/util/focusableSelectors.ts             | 14 ++++++++
 5 files changed, 44 insertions(+), 28 deletions(-)

diff --git a/ts/components/NavTabs.tsx b/ts/components/NavTabs.tsx
index 0db6eb300f..a58b45c136 100644
--- a/ts/components/NavTabs.tsx
+++ b/ts/components/NavTabs.tsx
@@ -234,6 +234,7 @@ export function NavTabs({
   return (
     <Tabs orientation="vertical" className="NavTabs__Container">
       <nav
+        data-supertab
         className={classNames('NavTabs', {
           'NavTabs--collapsed': navTabsCollapsed,
         })}
@@ -356,7 +357,6 @@ export function NavTabs({
           <button
             type="button"
             className="NavTabs__Item NavTabs__Item--Profile"
-            data-supertab
             onClick={() => {
               onToggleProfileEditor();
             }}
diff --git a/ts/components/Preferences.tsx b/ts/components/Preferences.tsx
index f1bf00e8fc..f859d43ba3 100644
--- a/ts/components/Preferences.tsx
+++ b/ts/components/Preferences.tsx
@@ -61,7 +61,7 @@ import {
 import { DurationInSeconds } from '../util/durations';
 import { useEscapeHandling } from '../hooks/useEscapeHandling';
 import { useUniqueId } from '../hooks/useUniqueId';
-import { focusableSelectors } from '../util/focusableSelectors';
+import { focusableSelector } from '../util/focusableSelectors';
 import { Modal } from './Modal';
 import { SearchInput } from './SearchInput';
 import { removeDiacritics } from '../util/removeDiacritics';
@@ -400,7 +400,6 @@ export function Preferences({
     [onSelectedMicrophoneChange, availableMicrophones]
   );
 
-  const selectors = useMemo(() => focusableSelectors.join(','), []);
   const settingsPaneRef = useRef<HTMLDivElement | null>(null);
   useEffect(() => {
     const settingsPane = settingsPaneRef.current;
@@ -414,12 +413,12 @@ export function Preferences({
       | HTMLInputElement
       | HTMLSelectElement
       | HTMLTextAreaElement
-    >(selectors);
+    >(focusableSelector);
     if (!elements.length) {
       return;
     }
     elements[0]?.focus();
-  }, [page, selectors]);
+  }, [page]);
 
   const onAudioOutputSelectChange = useCallback(
     (value: string) => {
diff --git a/ts/services/addGlobalKeyboardShortcuts.ts b/ts/services/addGlobalKeyboardShortcuts.ts
index 25917d79b9..3d5cb79c2c 100644
--- a/ts/services/addGlobalKeyboardShortcuts.ts
+++ b/ts/services/addGlobalKeyboardShortcuts.ts
@@ -6,7 +6,7 @@ import * as log from '../logging/log';
 import { PanelType } from '../types/Panels';
 import { clearConversationDraftAttachments } from '../util/clearConversationDraftAttachments';
 import { drop } from '../util/drop';
-import { focusableSelectors } from '../util/focusableSelectors';
+import { matchOrQueryFocusable } from '../util/focusableSelectors';
 import { getQuotedMessageSelector } from '../state/selectors/composer';
 import { removeLinkPreview } from './LinkPreview';
 
@@ -48,25 +48,32 @@ export function addGlobalKeyboardShortcuts(): void {
     ) {
       window.enterKeyboardMode();
       const focusedElement = document.activeElement;
-      const targets: Array<HTMLElement> = Array.from(
-        document.querySelectorAll('[data-supertab="true"]')
+      const targets = Array.from(
+        document.querySelectorAll<HTMLElement>('[data-supertab="true"]')
       );
-      const focusedIndex = targets.findIndex(target => {
-        if (!target || !focusedElement) {
-          return false;
-        }
+      const focusedIndexes: Array<number> = [];
 
-        if (target === focusedElement) {
-          return true;
+      targets.forEach((target, index) => {
+        if (
+          (focusedElement != null && target === focusedElement) ||
+          target.contains(focusedElement)
+        ) {
+          focusedIndexes.push(index);
         }
-
-        if (target.contains(focusedElement)) {
-          return true;
-        }
-
-        return false;
       });
 
+      if (focusedIndexes.length > 1) {
+        log.error(
+          `supertab: found multiple supertab elements containing the current active element: ${focusedIndexes.join(
+            ', '
+          )}`
+        );
+      }
+
+      // Default to the last focusable element to avoid cycles when multiple
+      // elements match (generally going to be a parent element)
+      const focusedIndex = focusedIndexes.at(-1) ?? -1;
+
       const lastIndex = targets.length - 1;
       const increment = shiftKey ? -1 : 1;
 
@@ -85,9 +92,7 @@ export function addGlobalKeyboardShortcuts(): void {
       }
 
       const node = targets[index];
-      const firstFocusableElement = node.querySelectorAll<HTMLElement>(
-        focusableSelectors.join(',')
-      )[0];
+      const firstFocusableElement = matchOrQueryFocusable(node);
 
       if (firstFocusableElement) {
         firstFocusableElement.focus();
diff --git a/ts/state/smart/ConversationPanel.tsx b/ts/state/smart/ConversationPanel.tsx
index 8b196e6f02..bf33512fb7 100644
--- a/ts/state/smart/ConversationPanel.tsx
+++ b/ts/state/smart/ConversationPanel.tsx
@@ -6,7 +6,6 @@ import React, {
   forwardRef,
   useCallback,
   useEffect,
-  useMemo,
   useRef,
   useState,
 } from 'react';
@@ -32,7 +31,7 @@ import {
   getPanelInformation,
   getWasPanelAnimated,
 } from '../selectors/conversations';
-import { focusableSelectors } from '../../util/focusableSelectors';
+import { focusableSelector } from '../../util/focusableSelectors';
 import { missingCaseError } from '../../util/missingCaseError';
 import { useConversationsActions } from '../ducks/conversations';
 import { useReducedMotion } from '../../hooks/useReducedMotion';
@@ -269,7 +268,6 @@ const PanelContainer = forwardRef<
   const { popPanelForConversation } = useConversationsActions();
   const conversationTitle = getConversationTitleForPanelType(i18n, panel.type);
 
-  const selectors = useMemo(() => focusableSelectors.join(','), []);
   const focusRef = useRef<HTMLDivElement | null>(null);
   useEffect(() => {
     if (!isActive) {
@@ -281,12 +279,12 @@ const PanelContainer = forwardRef<
       return;
     }
 
-    const elements = focusNode.querySelectorAll<HTMLElement>(selectors);
+    const elements = focusNode.querySelectorAll<HTMLElement>(focusableSelector);
     if (!elements.length) {
       return;
     }
     elements[0]?.focus();
-  }, [isActive, panel, selectors]);
+  }, [isActive, panel]);
 
   return (
     <div className="ConversationPanel" ref={ref}>
diff --git a/ts/util/focusableSelectors.ts b/ts/util/focusableSelectors.ts
index 0e8f7a7961..a683fbcb10 100644
--- a/ts/util/focusableSelectors.ts
+++ b/ts/util/focusableSelectors.ts
@@ -28,3 +28,17 @@ export const focusableSelectors = [
   `[contenteditable]${not.inert}${not.negTabIndex}`,
   `[tabindex]${not.inert}${not.negTabIndex}`,
 ];
+
+export const focusableSelector = focusableSelectors.join(', ');
+
+/**
+ * Matches the first focusable element within the given element or itself if it
+ * is focusable.
+ */
+export function matchOrQueryFocusable(
+  element: HTMLElement
+): HTMLElement | null {
+  return element.matches(focusableSelector)
+    ? element
+    : element.querySelector(focusableSelector);
+}