Accessibility fixes for ConversationDetails and AvatarPopup

stylesheets/_modules.scss

@@ -7584,11 +7584,29 @@ button.module-image__border-overlay:focus {
   flex-direction: row;
   width: 100%;
 
-  &:hover {
+  @include light-theme {
-    @include light-theme {
+    &:hover {
-      background-color: $color-gray-15;
+      background-color: $color-gray-05;
     }
-    @include dark-theme {
+  }
+  @include dark-theme {
+    &:hover {
+      background-color: $color-gray-60;
+    }
+  }
+  @include keyboard-mode {
+    &:hover {
+      background-color: inherit;
+    }
+    &:focus {
+      background-color: $color-gray-05;
+    }
+  }
+  @include dark-keyboard-mode {
+    &:hover {
+      background-color: inherit;
+    }
+    &:focus {
       background-color: $color-gray-60;
     }
   }

ts/components/AvatarPreview.tsx

@@ -134,7 +134,7 @@ export const AvatarPreview = ({
 
   const isLoading = imageStatus === ImageStatus.Loading;
 
-  const clickProps = onClick ? { role: 'button', onClick } : {};
+  const clickProps = onClick ? { role: 'button', onClick, tabIndex: 0 } : {};
   const componentStyle = {
     ...style,
   };

ts/components/ChatColorPicker.tsx

@@ -18,6 +18,8 @@ import { SampleMessageBubbles } from './SampleMessageBubbles';
 import { PanelRow } from './conversation/conversation-details/PanelRow';
 import { getCustomColorStyle } from '../util/getCustomColorStyle';
 
+import { useDelayedRestoreFocus } from '../hooks/useRestoreFocus';
+
 type CustomColorDataType = {
   id?: string;
   value?: CustomColorType;
@@ -84,6 +86,8 @@ export const ChatColorPicker = ({
     CustomColorDataType | undefined
   >(undefined);
 
+  const [focusRef] = useDelayedRestoreFocus();
+
   const onSelectColor = (
     conversationColor: ConversationColorType,
     customColorData?: { id: string; value: CustomColorType }
@@ -172,7 +176,7 @@ export const ChatColorPicker = ({
       />
       <hr />
       <div className="ChatColorPicker__bubbles">
-        {ConversationColors.map(color => (
+        {ConversationColors.map((color, i) => (
           <div
             aria-label={color}
             className={classNames(
@@ -190,6 +194,7 @@ export const ChatColorPicker = ({
             }}
             role="button"
             tabIndex={0}
+            ref={i === 0 ? focusRef : undefined}
           />
         ))}
         {Object.keys(customColors).map(colorId => {

ts/components/Select.tsx

@@ -19,41 +19,40 @@ export type PropsType = Readonly<{
   value?: string | number;
 }>;
 
-export function Select({
+export const Select = React.forwardRef(
-  disabled,
+  (
-  moduleClassName,
+    { disabled, moduleClassName, name, onChange, options, value }: PropsType,
-  name,
+    ref: React.Ref<HTMLSelectElement>
-  onChange,
+  ): JSX.Element => {
-  options,
+    const onSelectChange = (event: ChangeEvent<HTMLSelectElement>) => {
-  value,
+      onChange(event.target.value);
-}: PropsType): JSX.Element {
+    };
-  const onSelectChange = (event: ChangeEvent<HTMLSelectElement>) => {
-    onChange(event.target.value);
-  };
 
-  return (
+    return (
-    <div className={classNames(['module-select', moduleClassName])}>
+      <div className={classNames(['module-select', moduleClassName])}>
-      <select
+        <select
-        disabled={disabled}
+          disabled={disabled}
-        name={name}
+          name={name}
-        value={value}
+          value={value}
-        onChange={onSelectChange}
+          onChange={onSelectChange}
-      >
+          ref={ref}
-        {options.map(
+        >
-          ({ disabled: optionDisabled, text, value: optionValue }) => {
+          {options.map(
-            return (
+            ({ disabled: optionDisabled, text, value: optionValue }) => {
-              <option
+              return (
-                disabled={optionDisabled}
+                <option
-                value={optionValue}
+                  disabled={optionDisabled}
-                key={optionValue}
+                  value={optionValue}
-                aria-label={text}
+                  key={optionValue}
-              >
+                  aria-label={text}
-                {text}
+                >
-              </option>
+                  {text}
-            );
+                </option>
-          }
+              );
-        )}
+            }
-      </select>
+          )}
-    </div>
+        </select>
-  );
+      </div>
-}
+    );
+  }
+);

ts/components/conversation/conversation-details/GroupLinkManagement.stories.tsx

@@ -88,3 +88,9 @@ story.add('On (Non-admin)', () => {
 
   return <GroupLinkManagement {...props} />;
 });
+
+story.add('Off (Non-admin) - user cannot get here', () => {
+  const props = createProps(undefined, false);
+
+  return <GroupLinkManagement {...props} />;
+});

ts/components/conversation/conversation-details/GroupLinkManagement.tsx

@@ -11,6 +11,8 @@ import { PanelRow } from './PanelRow';
 import { PanelSection } from './PanelSection';
 import { Select } from '../../Select';
 
+import { useDelayedRestoreFocus } from '../../../hooks/useRestoreFocus';
+
 const AccessControlEnum = Proto.AccessControl.AccessRequired;
 
 export type PropsType = {
@@ -36,6 +38,8 @@ export const GroupLinkManagement: React.ComponentType<PropsType> = ({
     throw new Error('GroupLinkManagement rendered without a conversation');
   }
 
+  const [focusRef] = useDelayedRestoreFocus();
+
   const createEventHandler = (handleEvent: (x: boolean) => void) => {
     return (value: string) => {
       handleEvent(value === 'true');
@@ -72,6 +76,7 @@ export const GroupLinkManagement: React.ComponentType<PropsType> = ({
                     value: 'false',
                   },
                 ]}
+                ref={focusRef}
                 value={String(Boolean(hasGroupLink))}
               />
             ) : null
@@ -90,6 +95,7 @@ export const GroupLinkManagement: React.ComponentType<PropsType> = ({
                 />
               }
               label={i18n('GroupLinkManagement--share')}
+              ref={!isAdmin ? focusRef : undefined}
               onClick={() => {
                 if (conversation.groupLink) {
                   copyGroupLink(conversation.groupLink);

ts/components/conversation/conversation-details/PanelRow.tsx

@@ -19,43 +19,55 @@ export type Props = {
 
 const bem = bemGenerator('ConversationDetails-panel-row');
 
-export const PanelRow: React.ComponentType<Props> = ({
+export const PanelRow = React.forwardRef<HTMLButtonElement, Props>(
-  alwaysShowActions,
+  (
-  className,
+    {
-  disabled,
+      alwaysShowActions,
-  icon,
+      className,
-  label,
+      disabled,
-  info,
+      icon,
-  right,
+      label,
-  actions,
+      info,
-  onClick,
+      right,
-}) => {
+      actions,
-  const content = (
+      onClick,
-    <>
+    }: Props,
-      {icon !== undefined ? <div className={bem('icon')}>{icon}</div> : null}
+    ref: React.Ref<HTMLButtonElement>
-      <div className={bem('label')}>
+  ) => {
-        <div>{label}</div>
+    const content = (
-        {info !== undefined ? <div className={bem('info')}>{info}</div> : null}
+      <>
-      </div>
+        {icon !== undefined ? <div className={bem('icon')}>{icon}</div> : null}
-      {right !== undefined ? <div className={bem('right')}>{right}</div> : null}
+        <div className={bem('label')}>
-      {actions !== undefined ? (
+          <div>{label}</div>
-        <div className={alwaysShowActions ? '' : bem('actions')}>{actions}</div>
+          {info !== undefined ? (
-      ) : null}
+            <div className={bem('info')}>{info}</div>
-    </>
+          ) : null}
-  );
+        </div>
-
+        {right !== undefined ? (
-  if (onClick) {
+          <div className={bem('right')}>{right}</div>
-    return (
+        ) : null}
-      <button
+        {actions !== undefined ? (
-        disabled={disabled}
+          <div className={alwaysShowActions ? '' : bem('actions')}>
-        type="button"
+            {actions}
-        className={classNames(bem('root', 'button'), className)}
+          </div>
-        onClick={onClick}
+        ) : null}
-      >
+      </>
-        {content}
-      </button>
     );
-  }
 
-  return <div className={classNames(bem('root'), className)}>{content}</div>;
+    if (onClick) {
-};
+      return (
+        <button
+          disabled={disabled}
+          type="button"
+          className={classNames(bem('root', 'button'), className)}
+          onClick={onClick}
+          ref={ref}
+        >
+          {content}
+        </button>
+      );
+    }
+
+    return <div className={classNames(bem('root'), className)}>{content}</div>;
+  }
+);

ts/hooks/useRestoreFocus.ts

@@ -45,3 +45,51 @@ export const useRestoreFocus = (): Array<CallbackType> => {
 
   return [setFocusRef];
 };
+
+// Panels are initially rendered outside the DOM, and then added to it. We need to
+//   delay our attempts to set focus.
+// Just like the above hook, but with a debounce.
+export const useDelayedRestoreFocus = (): Array<CallbackType> => {
+  const toFocusRef = React.useRef<HTMLElement | null>(null);
+  const lastFocusedRef = React.useRef<HTMLElement | null>(null);
+
+  const setFocusRef = React.useCallback(
+    (toFocus: HTMLElement | null | undefined) => {
+      function setFocus() {
+        if (!toFocus) {
+          return;
+        }
+
+        // We only want to do this once.
+        if (toFocusRef.current) {
+          return;
+        }
+        toFocusRef.current = toFocus;
+
+        // Remember last-focused element, focus this new target element.
+        lastFocusedRef.current = document.activeElement as HTMLElement;
+        toFocus.focus();
+      }
+
+      const timeout = setTimeout(setFocus, 250);
+
+      return () => {
+        clearTimeout(timeout);
+      };
+    },
+    []
+  );
+
+  React.useEffect(() => {
+    return () => {
+      // On unmount, returned focus to element focused before we set the focus
+      setTimeout(() => {
+        if (lastFocusedRef.current && lastFocusedRef.current.focus) {
+          lastFocusedRef.current.focus();
+        }
+      });
+    };
+  }, []);
+
+  return [setFocusRef];
+};

ts/util/lint/exceptions.json

@@ -13022,6 +13022,20 @@
     "reasonCategory": "usageTrusted",
     "updated": "2021-09-17T17:37:46.279Z"
   },
+  {
+    "rule": "React-useRef",
+    "path": "ts/hooks/useRestoreFocus.js",
+    "line": "    const toFocusRef = React.useRef(null);",
+    "reasonCategory": "usageTrusted",
+    "updated": "2021-10-22T00:52:39.251Z"
+  },
+  {
+    "rule": "React-useRef",
+    "path": "ts/hooks/useRestoreFocus.js",
+    "line": "    const lastFocusedRef = React.useRef(null);",
+    "reasonCategory": "usageTrusted",
+    "updated": "2021-10-22T00:52:39.251Z"
+  },
   {
     "rule": "React-useRef",
     "path": "ts/hooks/useRestoreFocus.ts",
@@ -13036,6 +13050,20 @@
     "reasonCategory": "usageTrusted",
     "updated": "2021-07-30T16:57:33.618Z"
   },
+  {
+    "rule": "React-useRef",
+    "path": "ts/hooks/useRestoreFocus.ts",
+    "line": "  const toFocusRef = React.useRef<HTMLElement | null>(null);",
+    "reasonCategory": "usageTrusted",
+    "updated": "2021-10-22T00:52:39.251Z"
+  },
+  {
+    "rule": "React-useRef",
+    "path": "ts/hooks/useRestoreFocus.ts",
+    "line": "  const lastFocusedRef = React.useRef<HTMLElement | null>(null);",
+    "reasonCategory": "usageTrusted",
+    "updated": "2021-10-22T00:52:39.251Z"
+  },
   {
     "rule": "jQuery-append(",
     "path": "ts/logging/debuglogs.js",