Composer redesign: Fixes glitchy & inaccessible language selector (#40859)

app/javascript/mastodon/components/form_fields/combobox.module.scss

@@ -22,19 +22,23 @@
   }
 }
 
-.popover {
-  z-index: 9999;
+.listBoxWrapper {
+  position: relative;
   box-sizing: border-box;
-  max-height: min(320px, 50dvh);
   border-radius: 4px;
-  color: var(--color-text-primary);
-  background: var(--color-bg-primary);
-  border: 1px solid var(--color-border-primary);
-  box-shadow: var(--dropdown-shadow);
+  max-height: min(320px, 50dvh);
   overflow-y: auto;
   scrollbar-width: thin;
   scrollbar-gutter: stable;
   overscroll-behavior-y: contain;
+  color: var(--color-text-primary);
+  background: var(--color-bg-primary);
+}
+
+.listBoxWrapperPopover {
+  z-index: 9999;
+  border: 1px solid var(--color-border-primary);
+  box-shadow: var(--dropdown-shadow);
 }
 
 .groupTitle {

app/javascript/mastodon/components/form_fields/combobox_field.tsx

@@ -14,6 +14,8 @@ import { useIntl } from 'react-intl';
 
 import classNames from 'classnames';
 
+import { getScrollParent } from '@react-aria/utils';
+
 import { useMergedRefs } from '@/mastodon/hooks/useMergedRefs';
 import KeyboardArrowDownIcon from '@/material-icons/400-24px/keyboard_arrow_down.svg?react';
 import KeyboardArrowUpIcon from '@/material-icons/400-24px/keyboard_arrow_up.svg?react';
@@ -21,6 +23,7 @@ import SearchIcon from '@/material-icons/400-24px/search.svg?react';
 import { IconButton } from 'mastodon/components/icon_button';
 
 import { LoadingIndicator } from '../loading_indicator';
+import type { PopoverProps } from '../popover';
 import { Popover } from '../popover';
 
 import classes from './combobox.module.scss';
@@ -30,7 +33,7 @@ import { TextInput } from './text_input_field';
 import type { TextInputProps } from './text_input_field';
 
 interface ComboboxItem {
-  id: string;
+  id?: string;
 }
 
 export interface ComboboxItemState {
@@ -108,22 +111,31 @@ interface ComboboxProps<
    */
   icon?: TextInputProps['icon'] | null;
   /**
-   * Set to true to open as soon as there is focus
+   * By default, suggestions are rendered into a popover.
+   * Change this to 'in-place' to render them directly below the text input
+   */
+  listBoxType?: 'popover' | 'in-place';
+  /**
+   * Set to true to open the suggestion popover as soon as the input is focused.
+   * Doesn't apply when `listBoxType='in-place'` is used.
    */
   openOnFocus?: boolean;
   /**
+   * Control whether the first suggestion is highlighted by default.
    * Keep this enabled for autocomplete-like menu suggestions, where pressing
    * Enter should select the first item in the suggestion dropdown.
-   * Disable when suggested items aren't necessarily related to user input, or
-   * when pressing Enter should submit a search instead.
+   * Disable when freeform text input is expected, or when pressing Enter
+   * should submit a search instead.
    */
   autoHighlightFirstItem?: boolean;
   /**
-   * Set to false to keep the menu open when an item is selected
+   * Set to false to keep the menu open when an item is selected.
+   * Doesn't apply when `listBoxType='in-place'` is used.
    */
   closeOnSelect?: boolean;
   /**
-   * Prevent the menu from opening, e.g. to prevent the empty state from showing
+   * Prevent the menu from opening, e.g. to prevent the empty state from showing.
+   * Doesn't apply when `listBoxType='in-place'` is used.
    */
   suppressMenu?: boolean;
 }
@@ -237,7 +249,7 @@ const ComboboxWithRef = <Item extends ComboboxItem, GroupKey extends string>(
     value,
     isLoading = false,
     items,
-    getItemId = (item) => item.id,
+    getItemId = (item) => ('id' in item ? (item.id ?? '') : ''),
     getIsItemDisabled,
     getIsItemSelected,
     getGroupEmptyMessage,
@@ -248,8 +260,9 @@ const ComboboxWithRef = <Item extends ComboboxItem, GroupKey extends string>(
     onFocus,
     onChange,
     onKeyDown,
-    openOnFocus = false,
-    closeOnSelect = true,
+    listBoxType = 'popover',
+    openOnFocus = listBoxType === 'in-place' || false,
+    closeOnSelect = listBoxType === 'in-place' ? false : true,
     autoHighlightFirstItem = true,
     suppressMenu = false,
     icon = SearchIcon,
@@ -266,17 +279,12 @@ const ComboboxWithRef = <Item extends ComboboxItem, GroupKey extends string>(
   const [popoverElement, setPopoverElement] = useState<HTMLDivElement | null>(
     null,
   );
-
+  const [shouldMenuOpen, setShouldMenuOpen] = useState(false);
   // This ref tracks whether the menu was just closed following a
   // selection, and prevents the menu from re-opening again
   // when focus is returned to the input.
   const wasMenuJustClosedRef = useRef(false);
 
-  const [highlightedItemId, setHighlightedItemId] = useState<string | null>(
-    null,
-  );
-  const [shouldMenuOpen, setShouldMenuOpen] = useState(false);
-
   const hasGroups = !Array.isArray(items);
   const flatItems = useMemo(
     () =>
@@ -288,6 +296,18 @@ const ComboboxWithRef = <Item extends ComboboxItem, GroupKey extends string>(
     [hasGroups, items],
   );
 
+  /**
+   * Stores the id of the currently highlighted item. If `null`, the id of
+   * the first item will be used unless `autoHighlightFirstItem` is false
+   */
+  const [highlightedItemIdState, setHighlightedItemId] = useState<
+    string | null
+  >(null);
+  const firstItem = flatItems[0];
+  const firstItemId = firstItem ? getItemId(firstItem) : null;
+  const highlightedItemId =
+    highlightedItemIdState ?? (autoHighlightFirstItem ? firstItemId : null);
+
   const hasGroupEmptyMessages =
     hasGroups &&
     !!getGroupEmptyMessage &&
@@ -306,7 +326,8 @@ const ComboboxWithRef = <Item extends ComboboxItem, GroupKey extends string>(
     !disabled &&
     !suppressMenu &&
     (flatItems.length > 0 || hasGroupEmptyMessages || showStatusMessageInMenu);
-  const isMenuOpen = shouldMenuOpen && hasMenuContent;
+  const isMenuOpen =
+    listBoxType === 'in-place' || (shouldMenuOpen && hasMenuContent);
 
   const openMenu = useCallback(() => {
     setShouldMenuOpen(true);
@@ -317,40 +338,17 @@ const ComboboxWithRef = <Item extends ComboboxItem, GroupKey extends string>(
     setShouldMenuOpen(false);
   }, []);
 
-  const highlightItem = useCallback(
-    (id: string | null) => {
-      setHighlightedItemId(id);
-      if (id) {
-        const itemElement = popoverElement?.querySelector<HTMLLIElement>(
-          `[data-item-id='${id}']`,
-        );
-        if (itemElement && popoverElement) {
-          scrollItemIntoView(itemElement, popoverElement);
-        }
-      }
-    },
-    [popoverElement],
-  );
-
-  const resetHighlight = useCallback(() => {
-    if (autoHighlightFirstItem) {
-      const firstItem = flatItems[0];
-      const firstItemId = firstItem ? getItemId(firstItem) : null;
-      highlightItem(firstItemId);
-    } else {
-      highlightItem(null);
-    }
-  }, [autoHighlightFirstItem, flatItems, getItemId, highlightItem]);
-
-  // Reset scroll & highlight when menu items change
+  // Scroll highlighted item into view as it changes
   useEffect(() => {
-    if (flatItems.length && autoHighlightFirstItem) {
-      // This only runs when the items change so should be safe from
-      // cascade renders.
-      // eslint-disable-next-line react-hooks/set-state-in-effect
-      resetHighlight();
+    if (highlightedItemId) {
+      const itemElement = popoverElement?.querySelector<HTMLLIElement>(
+        `[data-item-id='${highlightedItemId}']`,
+      );
+      if (itemElement) {
+        scrollItemIntoView(itemElement);
+      }
     }
-  }, [flatItems, resetHighlight, autoHighlightFirstItem]);
+  }, [highlightedItemId, popoverElement]);
 
   const handleFocus: React.FocusEventHandler<HTMLInputElement> = useCallback(
     (e) => {
@@ -365,20 +363,20 @@ const ComboboxWithRef = <Item extends ComboboxItem, GroupKey extends string>(
   const handleInputChange = useCallback(
     (e: React.ChangeEvent<HTMLInputElement>) => {
       onChange(e);
-      resetHighlight();
+      setHighlightedItemId(null);
       setShouldMenuOpen(!!e.target.value);
     },
-    [onChange, resetHighlight],
+    [onChange],
   );
 
   const handleItemMouseEnter = useCallback(
     (e: React.MouseEvent<HTMLLIElement>) => {
       const { itemId } = e.currentTarget.dataset;
       if (itemId) {
-        highlightItem(itemId);
+        setHighlightedItemId(itemId);
       }
     },
-    [highlightItem],
+    [setHighlightedItemId],
   );
 
   const selectItem = useCallback(
@@ -434,10 +432,10 @@ const ComboboxWithRef = <Item extends ComboboxItem, GroupKey extends string>(
         // If no item is highlighted yet, highlight the first or last
         if (direction > 0) {
           const firstItem = flatItems.at(0);
-          highlightItem(firstItem ? getItemId(firstItem) : null);
+          setHighlightedItemId(firstItem ? getItemId(firstItem) : null);
         } else {
           const lastItem = flatItems.at(-1);
-          highlightItem(lastItem ? getItemId(lastItem) : null);
+          setHighlightedItemId(lastItem ? getItemId(lastItem) : null);
         }
       } else {
         // If there is a highlighted item, select the next or previous item
@@ -450,12 +448,12 @@ const ComboboxWithRef = <Item extends ComboboxItem, GroupKey extends string>(
         }
 
         const newHighlightedItem = flatItems[newIndex];
-        highlightItem(
+        setHighlightedItemId(
           newHighlightedItem ? getItemId(newHighlightedItem) : null,
         );
       }
     },
-    [getItemId, highlightItem, highlightedItemId, flatItems],
+    [getItemId, setHighlightedItemId, highlightedItemId, flatItems],
   );
 
   const handleInputKeyDown = useCallback(
@@ -552,8 +550,8 @@ const ComboboxWithRef = <Item extends ComboboxItem, GroupKey extends string>(
         {...otherProps}
         disabled={disabled}
         aria-controls={listId}
-        aria-expanded={isMenuOpen ? 'true' : 'false'}
-        aria-haspopup='listbox'
+        aria-expanded={listBoxType === 'popover' ? isMenuOpen : undefined}
+        aria-haspopup={listBoxType === 'popover' ? 'listbox' : undefined}
         aria-activedescendant={
           isMenuOpen && highlightedItemId ? highlightedItemId : undefined
         }
@@ -568,7 +566,7 @@ const ComboboxWithRef = <Item extends ComboboxItem, GroupKey extends string>(
         className={classNames(classes.input, className)}
         ref={mergedRef}
       />
-      {hasMenuContent && (
+      {hasMenuContent && listBoxType === 'popover' && (
         <IconButton
           title={
             isMenuOpen
@@ -592,80 +590,69 @@ const ComboboxWithRef = <Item extends ComboboxItem, GroupKey extends string>(
       <span role='status' aria-live='polite' className='sr-only'>
         {isMenuOpen && statusMessage}
       </span>
-      <Popover
-        matchReferenceWidth
-        isOpen={isMenuOpen}
+      <OptionalPopover
+        renderAsPopover={listBoxType === 'popover'}
+        isMenuOpen={isMenuOpen}
         onClose={closeMenu}
-        offset={1}
-        placement='bottom-start'
-        strategy='absolute'
+        ref={setPopoverElement}
         popoverElement={popoverElement}
-        reference={inputElement}
-        container={null}
+        popoverReference={inputElement}
       >
-        {({ props, placement }) => (
-          <div
-            {...props}
-            ref={setPopoverElement}
-            className={classNames(classes.popover, placement)}
-          >
-            <StatusMessageWrapper
-              showStatus={showStatusMessageInMenu && !hasGroupEmptyMessages}
-              isLoading={isLoading}
-              status={statusMessage}
-            >
-              {hasGroups ? (
-                <div role='listbox' id={listId} tabIndex={-1}>
-                  {(Object.keys(items) as GroupKey[]).map((groupKey) => {
-                    const groupItems = items[groupKey];
-                    const groupTitleId = `${listId}-group-${groupKey}`;
-                    const customGroupTitle = renderGroupTitle?.(
-                      groupKey,
-                      groupTitleId,
-                    );
-                    const hasTitle = customGroupTitle !== null;
-                    const hasGroupItems = !!groupItems?.length;
-                    const groupEmptyState = getGroupEmptyMessage?.(groupKey);
+        <StatusMessageWrapper
+          showStatus={showStatusMessageInMenu && !hasGroupEmptyMessages}
+          isLoading={isLoading}
+          status={statusMessage}
+        >
+          {hasGroups ? (
+            <div role='listbox' id={listId} tabIndex={-1}>
+              {(Object.keys(items) as GroupKey[]).map((groupKey) => {
+                const groupItems = items[groupKey];
+                const groupTitleId = `${listId}-group-${groupKey}`;
+                const customGroupTitle = renderGroupTitle?.(
+                  groupKey,
+                  groupTitleId,
+                );
+                const hasTitle = customGroupTitle !== null;
+                const hasGroupItems = !!groupItems?.length;
+                const groupEmptyState = getGroupEmptyMessage?.(groupKey);
 
-                    if (!hasGroupItems && !groupEmptyState) {
-                      return null;
-                    }
+                if (!hasGroupItems && !groupEmptyState) {
+                  return null;
+                }
 
-                    const showEmptyState = !hasGroupItems && groupEmptyState;
-                    const GroupWrapperElement = showEmptyState ? 'div' : 'ul';
+                const showEmptyState = !hasGroupItems && groupEmptyState;
+                const GroupWrapperElement = showEmptyState ? 'div' : 'ul';
 
-                    return (
-                      <GroupWrapperElement
-                        key={groupKey}
-                        role='group'
-                        aria-labelledby={hasTitle ? groupTitleId : undefined}
-                      >
-                        {hasTitle &&
-                          (customGroupTitle ?? (
-                            <ComboboxMenuGroupTitle id={groupTitleId}>
-                              {groupKey}
-                            </ComboboxMenuGroupTitle>
-                          ))}
-                        {hasGroupItems ? (
-                          renderItems(groupItems)
-                        ) : (
-                          <div className={classes.groupEmptyMessage}>
-                            {groupEmptyState}
-                          </div>
-                        )}
-                      </GroupWrapperElement>
-                    );
-                  })}
-                </div>
-              ) : (
-                <ul role='listbox' id={listId} tabIndex={-1}>
-                  {renderItems(items)}
-                </ul>
-              )}
-            </StatusMessageWrapper>
-          </div>
-        )}
-      </Popover>
+                return (
+                  <GroupWrapperElement
+                    key={groupKey}
+                    role='group'
+                    aria-labelledby={hasTitle ? groupTitleId : undefined}
+                  >
+                    {hasTitle &&
+                      (customGroupTitle ?? (
+                        <ComboboxMenuGroupTitle id={groupTitleId}>
+                          {groupKey}
+                        </ComboboxMenuGroupTitle>
+                      ))}
+                    {hasGroupItems ? (
+                      renderItems(groupItems)
+                    ) : (
+                      <div className={classes.groupEmptyMessage}>
+                        {groupEmptyState}
+                      </div>
+                    )}
+                  </GroupWrapperElement>
+                );
+              })}
+            </div>
+          ) : (
+            <ul role='listbox' id={listId} tabIndex={-1}>
+              {renderItems(items)}
+            </ul>
+          )}
+        </StatusMessageWrapper>
+      </OptionalPopover>
     </div>
   );
 };
@@ -683,6 +670,60 @@ export const Combobox = forwardRef(ComboboxWithRef) as {
 
 Combobox.displayName = 'Combobox';
 
+const OptionalPopover: React.FC<{
+  renderAsPopover: boolean;
+  isMenuOpen: boolean;
+  popoverElement: PopoverProps['popoverElement'];
+  popoverReference: PopoverProps['reference'];
+  onClose: PopoverProps['onClose'];
+  ref: React.Ref<HTMLDivElement>;
+  children: React.ReactNode;
+}> = ({
+  renderAsPopover,
+  isMenuOpen,
+  popoverElement,
+  popoverReference,
+  ref,
+  onClose,
+  children,
+}) => {
+  if (renderAsPopover) {
+    return (
+      <Popover
+        matchReferenceWidth
+        isOpen={isMenuOpen}
+        onClose={onClose}
+        offset={1}
+        placement='bottom-start'
+        strategy='absolute'
+        popoverElement={popoverElement}
+        reference={popoverReference}
+        container={null}
+      >
+        {({ props, placement }) => (
+          <div
+            {...props}
+            ref={ref}
+            className={classNames(
+              classes.listBoxWrapper,
+              classes.listBoxWrapperPopover,
+              placement,
+            )}
+          >
+            {children}
+          </div>
+        )}
+      </Popover>
+    );
+  }
+
+  return (
+    <div ref={ref} className={classes.listBoxWrapper}>
+      {children}
+    </div>
+  );
+};
+
 const StatusMessageWrapper: React.FC<{
   showStatus: boolean;
   status: string;
@@ -747,7 +788,13 @@ function useGetA11yStatusMessage({
 
 const SCROLL_MARGIN = 6;
 
-function scrollItemIntoView(item: HTMLElement, scrollParent: HTMLElement) {
+function scrollItemIntoView(item: HTMLElement) {
+  const scrollParent = getScrollParent(item);
+
+  if (!(scrollParent instanceof HTMLElement)) {
+    return;
+  }
+
   const itemTopEdge = item.offsetTop;
   const itemBottomEdge = itemTopEdge + item.offsetHeight;
 

app/javascript/mastodon/features/compose/redesign/language.tsx

@@ -1,18 +1,17 @@
 import type React from 'react';
-import { useCallback } from 'react';
+import { useCallback, useId, useMemo, useState } from 'react';
 
 import { defineMessages, FormattedMessage, useIntl } from 'react-intl';
 
 import { MagnifyingGlassIcon, TranslateIcon } from '@phosphor-icons/react';
 
 import { changeComposeLanguage } from '@/mastodon/actions/compose';
-import { TextInput } from '@/mastodon/components/form_fields/redesign';
-import {
-  Menu,
-  MenuItem,
-  MenuList,
-  MenuTrigger,
-} from '@/mastodon/components/menu';
+import { Button } from '@/mastodon/components/button/redesign';
+import { Combobox } from '@/mastodon/components/form_fields';
+import { ComboboxMenuItem } from '@/mastodon/components/form_fields/combobox_field';
+import { PopoverMenuCard } from '@/mastodon/components/menu/card';
+import { useToggle } from '@/mastodon/hooks/useToggle';
+import { languages as initialStateLanguages } from '@/mastodon/initial_state';
 import { useAppDispatch, useAppSelector } from '@/mastodon/store';
 
 import { useLanguageList } from './hooks';
@@ -25,77 +24,125 @@ const messages = defineMessages({
   },
 });
 
+function getLanguageNameFromCode(code: string) {
+  const language = initialStateLanguages?.find(
+    ([languageCode]) => code === languageCode,
+  );
+  return language ? language[1] : undefined;
+}
+
 export const LanguageButton: React.FC = () => {
   const langCode = useAppSelector(
     (state) => state.compose.get('language') as string,
   );
 
-  return (
-    <Menu>
-      <MenuTrigger size='sm' leadingIcon={TranslateIcon}>
-        {langCode.toLocaleUpperCase()}
-      </MenuTrigger>
+  const [isOpen, { onToggle: toggle, onFalse: close }] = useToggle(false);
+  const [triggerElement, setTriggerElement] =
+    useState<HTMLButtonElement | null>(null);
 
-      <MenuList
+  return (
+    <>
+      <Button
+        ref={setTriggerElement}
+        size='sm'
+        leadingIcon={TranslateIcon}
+        aria-expanded={isOpen}
+        tooltip={{
+          type: 'description',
+          text: (
+            <FormattedMessage
+              id='compose.language.change'
+              defaultMessage='Change language'
+            />
+          ),
+        }}
+        onClick={toggle}
+      >
+        {langCode.toLocaleUpperCase()}
+        <span className='sr-only'>({getLanguageNameFromCode(langCode)})</span>
+      </Button>
+
+      <PopoverMenuCard
+        isOpen={isOpen}
         placement='bottom-end'
+        reference={triggerElement}
+        container={null}
         className={classes.languageMenu}
         maxWidth={280}
+        onClose={close}
       >
-        <LanguageDropdown />
-      </MenuList>
-    </Menu>
+        <LanguageDropdown onClose={close} />
+      </PopoverMenuCard>
+    </>
   );
 };
 
-export const LanguageDropdown = () => {
+interface LanguageObject {
+  id: string;
+  code: string;
+  name: string;
+  localName: string;
+}
+
+export const LanguageDropdown: React.FC<{ onClose: () => void }> = ({
+  onClose,
+}) => {
+  const intl = useIntl();
+  const dispatch = useAppDispatch();
+
+  const [inputValue, setInputValue] = useState('');
   const { languages, onSearch } = useLanguageList();
 
-  const dispatch = useAppDispatch();
-  const handleChange: React.MouseEventHandler<HTMLButtonElement> = useCallback(
-    (event) => {
-      const newLanguage = event.currentTarget.dataset.language;
-      if (newLanguage) {
-        dispatch(changeComposeLanguage(newLanguage));
-      }
-    },
-    [dispatch],
-  );
-
-  const intl = useIntl();
   const handleSearch: React.ChangeEventHandler<HTMLInputElement> = useCallback(
     (event) => {
+      setInputValue(event.target.value);
       onSearch(event.target.value);
     },
     [onSearch],
   );
+  const handleSelectLanguage = useCallback(
+    ({ code }: LanguageObject) => {
+      if (code) {
+        dispatch(changeComposeLanguage(code));
+        onClose();
+      }
+    },
+    [dispatch, onClose],
+  );
+
+  const uniqueId = useId();
+  const languageObjects = useMemo(
+    () =>
+      languages.map(([code, name, localName]) => ({
+        id: `${uniqueId}-${code}`,
+        code,
+        name,
+        localName,
+      })),
+    [languages, uniqueId],
+  );
 
   return (
-    <>
-      <TextInput
-        type='search'
-        onChange={handleSearch}
-        placeholder={intl.formatMessage(messages.searchPlaceholder)}
-        icon={MagnifyingGlassIcon}
-      />
-      <div className={classes.languageList}>
-        {languages.map((lang) => (
-          <MenuItem
-            key={lang[0]}
-            onClick={handleChange}
-            data-language={lang[0]}
-            className={classes.languageItem}
-          >
-            <strong>{lang[2]}</strong>&nbsp;<span>({lang[1]})</span>
-          </MenuItem>
-        ))}
-
-        {languages.length === 0 && (
-          <FormattedMessage
-            id='compose.language.not-found'
-            defaultMessage='No language found'
-          />
-        )}
-      </div>
-    </>
+    <Combobox
+      autoFocus
+      listBoxType='in-place'
+      value={inputValue}
+      onChange={handleSearch}
+      placeholder={intl.formatMessage(messages.searchPlaceholder)}
+      icon={MagnifyingGlassIcon}
+      items={languageObjects}
+      renderItem={renderLanguageItem}
+      onSelectItem={handleSelectLanguage}
+    />
   );
 };
+
+function renderLanguageItem({ code, name, localName }: LanguageObject) {
+  return (
+    <ComboboxMenuItem>
+      <span>
+        <strong lang={code}>{localName}</strong> ({name})
+      </span>
+    </ComboboxMenuItem>
+  );
+}

app/javascript/mastodon/locales/en.json

@@ -532,7 +532,6 @@
   "compose.hints.missing-alt-many": "One or more of your attachments are missing alt text.",
   "compose.hints.too-long": "Post exceeds character maximum",
   "compose.language.change": "Change language",
-  "compose.language.not-found": "No language found",
   "compose.language.search": "Search languages...",
   "compose.message.placeholder": "Add your recipients and your message.",
   "compose.message.publish": "Send",