From ec7dd2554a97aa30e74dbbe2792f9f677644d8e6 Mon Sep 17 00:00:00 2001
From: WofWca <wofwca@protonmail.com>
Date: Fri, 18 Oct 2024 20:29:43 +0400
Subject: [PATCH] improvement: arrow-key navigation for chat list

Implements the "roving tabindex" approach:
https://developer.mozilla.org/en-US/docs/Web/Accessibility/Keyboard-navigable_JavaScript_widgets#technique_1_roving_tabindex.

Supersedes https://github.com/deltachat/deltachat-desktop/pull/4211.

This improves things, but the UX as a whole is not great yet:
- The tab order is such that the chat list does not
  immediately follow the search field,
  so you have to tab through the other navbar items
  before you get to the chat list.
  Same for going back from the chat list to the search bar.
- The initially "active" element is just the first chat item,
  and not the currently selected chat.
- Since the chat list is "virtualized", the currently active element
  might get removed from DOM when the user scrolls,
  thus we lose track of the item that was last selected.

Related: https://github.com/deltachat/deltachat-desktop/issues/2784
---
 .../frontend/src/components/chat/ChatList.tsx | 188 ++++++++-------
 .../src/components/chat/ChatListItem.tsx      |  63 +++++-
 .../frontend/src/contexts/RovingTabindex.tsx  | 214 ++++++++++++++++++
 3 files changed, 373 insertions(+), 92 deletions(-)
 create mode 100644 packages/frontend/src/contexts/RovingTabindex.tsx

diff --git a/packages/frontend/src/components/chat/ChatList.tsx b/packages/frontend/src/components/chat/ChatList.tsx
index 62946acab5..3e6ed4bf74 100644
--- a/packages/frontend/src/components/chat/ChatList.tsx
+++ b/packages/frontend/src/components/chat/ChatList.tsx
@@ -44,6 +44,7 @@ import type {
   MessageChatListItemData,
 } from './ChatListItemRow'
 import { isInviteLink } from '../../../../shared/util'
+import { RovingTabindexProvider } from '../../contexts/RovingTabindex'
 
 const enum LoadStatus {
   FETCHING = 1,
@@ -163,6 +164,8 @@ export default function ChatList(props: {
   const createChatByContactId = useCreateChatByContactId()
   const { selectChat } = useChat()
 
+  const tabindexWrapperElement = useRef<HTMLDivElement>(null)
+
   const addContactOnClick = async () => {
     if (!queryStrIsValidEmail || !queryStr) return
 
@@ -344,27 +347,31 @@ export default function ChatList(props: {
         <div className='chat-list'>
           <AutoSizer>
             {({ width, height }) => (
-              <div>
+              <div ref={tabindexWrapperElement}>
                 <div className='search-result-divider' style={{ width: width }}>
                   {tx('search_in', searchChatInfo.name)}
                   {messageResultIds.length !== 0 &&
                     ': ' + translate_n('n_messages', messageResultIds.length)}
                 </div>
-                <ChatListPart
-                  isRowLoaded={isMessageLoaded}
-                  loadMoreRows={loadMessages}
-                  rowCount={messageResultIds.length}
-                  width={width}
-                  height={
-                    /* take remaining space */
-                    height - DIVIDER_HEIGHT
-                  }
-                  itemKey={index => 'key' + messageResultIds[index]}
-                  itemData={messagelistData}
-                  itemHeight={CHATLISTITEM_MESSAGE_HEIGHT}
+                <RovingTabindexProvider
+                  wrapperElementRef={tabindexWrapperElement}
                 >
-                  {ChatListItemRowMessage}
-                </ChatListPart>
+                  <ChatListPart
+                    isRowLoaded={isMessageLoaded}
+                    loadMoreRows={loadMessages}
+                    rowCount={messageResultIds.length}
+                    width={width}
+                    height={
+                      /* take remaining space */
+                      height - DIVIDER_HEIGHT
+                    }
+                    itemKey={index => 'key' + messageResultIds[index]}
+                    itemData={messagelistData}
+                    itemHeight={CHATLISTITEM_MESSAGE_HEIGHT}
+                  >
+                    {ChatListItemRowMessage}
+                  </ChatListPart>
+                </RovingTabindexProvider>
               </div>
             )}
           </AutoSizer>
@@ -378,90 +385,97 @@ export default function ChatList(props: {
       <div className='chat-list'>
         <AutoSizer>
           {({ width, height }) => (
-            <div>
+            <div ref={tabindexWrapperElement}>
               {isSearchActive && (
                 <div className='search-result-divider' style={{ width: width }}>
                   {translate_n('n_chats', chatListIds.length)}
                 </div>
               )}
-              <ChatListPart
-                isRowLoaded={isChatLoaded}
-                loadMoreRows={loadChats}
-                rowCount={chatListIds.length}
-                width={width}
-                height={chatsHeight(height)}
-                setListRef={(ref: List<any> | null) =>
-                  ((listRefRef.current as any) = ref)
-                }
-                itemKey={index => 'key' + chatListIds[index]}
-                itemData={chatlistData}
-                itemHeight={CHATLISTITEM_CHAT_HEIGHT}
+              {/* TODO RovingTabindex doesn't work well with virtualized
+              lists, because the currently active element might get removed
+              from DOM if scrolled out of view. */}
+              <RovingTabindexProvider
+                wrapperElementRef={tabindexWrapperElement}
               >
-                {ChatListItemRowChat}
-              </ChatListPart>
-              {isSearchActive && (
-                <>
-                  <div
-                    className='search-result-divider'
-                    style={{ width: width }}
-                  >
-                    {translate_n('n_contacts', contactIds.length)}
-                  </div>
-                  <ChatListPart
-                    isRowLoaded={isContactLoaded}
-                    loadMoreRows={loadContact}
-                    rowCount={contactIds.length}
-                    width={width}
-                    height={contactsHeight(height)}
-                    itemKey={index => 'key' + contactIds[index]}
-                    itemData={contactlistData}
-                    itemHeight={CHATLISTITEM_CONTACT_HEIGHT}
-                  >
-                    {ChatListItemRowContact}
-                  </ChatListPart>
-                  {contactIds.length === 0 &&
-                    chatListIds.length === 0 &&
-                    queryStrIsValidEmail && (
+                <ChatListPart
+                  isRowLoaded={isChatLoaded}
+                  loadMoreRows={loadChats}
+                  rowCount={chatListIds.length}
+                  width={width}
+                  height={chatsHeight(height)}
+                  setListRef={(ref: List<any> | null) =>
+                    ((listRefRef.current as any) = ref)
+                  }
+                  itemKey={index => 'key' + chatListIds[index]}
+                  itemData={chatlistData}
+                  itemHeight={CHATLISTITEM_CHAT_HEIGHT}
+                >
+                  {ChatListItemRowChat}
+                </ChatListPart>
+                {isSearchActive && (
+                  <>
+                    <div
+                      className='search-result-divider'
+                      style={{ width: width }}
+                    >
+                      {translate_n('n_contacts', contactIds.length)}
+                    </div>
+                    <ChatListPart
+                      isRowLoaded={isContactLoaded}
+                      loadMoreRows={loadContact}
+                      rowCount={contactIds.length}
+                      width={width}
+                      height={contactsHeight(height)}
+                      itemKey={index => 'key' + contactIds[index]}
+                      itemData={contactlistData}
+                      itemHeight={CHATLISTITEM_CONTACT_HEIGHT}
+                    >
+                      {ChatListItemRowContact}
+                    </ChatListPart>
+                    {contactIds.length === 0 &&
+                      chatListIds.length === 0 &&
+                      queryStrIsValidEmail && (
+                        <div style={{ width: width }}>
+                          <PseudoListItemAddContact
+                            queryStr={queryStr?.trim() || ''}
+                            queryStrIsEmail={queryStrIsValidEmail}
+                            onClick={addContactOnClick}
+                          />
+                        </div>
+                      )}
+                    {showPseudoListItemAddContactFromInviteLink && (
                       <div style={{ width: width }}>
-                        <PseudoListItemAddContact
-                          queryStr={queryStr?.trim() || ''}
-                          queryStrIsEmail={queryStrIsValidEmail}
-                          onClick={addContactOnClick}
+                        <PseudoListItemAddContactOrGroupFromInviteLink
+                          inviteLink={queryStr!}
+                          accountId={accountId}
                         />
                       </div>
                     )}
-                  {showPseudoListItemAddContactFromInviteLink && (
-                    <div style={{ width: width }}>
-                      <PseudoListItemAddContactOrGroupFromInviteLink
-                        inviteLink={queryStr!}
-                        accountId={accountId}
-                      />
+                    <div
+                      className='search-result-divider'
+                      style={{ width: width }}
+                    >
+                      {translated_messages_label(messageResultIds.length)}
                     </div>
-                  )}
-                  <div
-                    className='search-result-divider'
-                    style={{ width: width }}
-                  >
-                    {translated_messages_label(messageResultIds.length)}
-                  </div>
 
-                  <ChatListPart
-                    isRowLoaded={isMessageLoaded}
-                    loadMoreRows={loadMessages}
-                    rowCount={messageResultIds.length}
-                    width={width}
-                    height={
-                      // take remaining space
-                      messagesHeight(height)
-                    }
-                    itemKey={index => 'key' + messageResultIds[index]}
-                    itemData={messagelistData}
-                    itemHeight={CHATLISTITEM_MESSAGE_HEIGHT}
-                  >
-                    {ChatListItemRowMessage}
-                  </ChatListPart>
-                </>
-              )}
+                    <ChatListPart
+                      isRowLoaded={isMessageLoaded}
+                      loadMoreRows={loadMessages}
+                      rowCount={messageResultIds.length}
+                      width={width}
+                      height={
+                        // take remaining space
+                        messagesHeight(height)
+                      }
+                      itemKey={index => 'key' + messageResultIds[index]}
+                      itemData={messagelistData}
+                      itemHeight={CHATLISTITEM_MESSAGE_HEIGHT}
+                    >
+                      {ChatListItemRowMessage}
+                    </ChatListPart>
+                  </>
+                )}
+              </RovingTabindexProvider>
               <div
                 className='floating-action-button'
                 onClick={onCreateChat}
diff --git a/packages/frontend/src/components/chat/ChatListItem.tsx b/packages/frontend/src/components/chat/ChatListItem.tsx
index 28b6c93f9f..3c6e7ac198 100644
--- a/packages/frontend/src/components/chat/ChatListItem.tsx
+++ b/packages/frontend/src/components/chat/ChatListItem.tsx
@@ -1,4 +1,4 @@
-import React from 'react'
+import React, { useRef } from 'react'
 import classNames from 'classnames'
 import { T, C } from '@deltachat/jsonrpc-client'
 
@@ -12,6 +12,7 @@ import { selectedAccountId } from '../../ScreenController'
 import { InlineVerifiedIcon } from '../VerifiedIcon'
 import { runtime } from '@deltachat-desktop/runtime-interface'
 import { message2React } from '../message/MessageMarkdown'
+import { useRovingTabindex } from '../../contexts/RovingTabindex'
 
 const log = getLogger('renderer/chatlist/item')
 
@@ -171,11 +172,23 @@ function ChatListItemArchiveLink({
     },
   ])
 
+  const ref = useRef<HTMLButtonElement>(null)
+
+  const {
+    tabIndex,
+    onKeydown: tabindexOnKeydown,
+    setAsActiveElement: tabindexSetAsActiveElement,
+  } = useRovingTabindex(ref)
+
   return (
     <button
+      ref={ref}
+      tabIndex={tabIndex}
       onClick={onClick}
+      onKeyDown={tabindexOnKeydown}
+      onFocus={tabindexSetAsActiveElement}
       onContextMenu={onContextMenu}
-      className={`chat-list-item archive-link-item ${
+      className={`chat-list-item archive-link-item roving-tabindex ${
         isContextMenuActive ? 'context-menu-active' : ''
       }`}
     >
@@ -206,11 +219,24 @@ function ChatListItemError({
   isSelected?: boolean
 }) {
   log.info('Error Loading Chatlistitem ' + chatListItem.id, chatListItem.error)
+
+  const ref = useRef<HTMLButtonElement>(null)
+
+  const {
+    tabIndex,
+    onKeydown: tabindexOnKeydown,
+    setAsActiveElement: tabindexSetAsActiveElement,
+  } = useRovingTabindex(ref)
+
   return (
     <button
+      ref={ref}
+      tabIndex={tabIndex}
       onClick={onClick}
+      onKeyDown={tabindexOnKeydown}
+      onFocus={tabindexSetAsActiveElement}
       onContextMenu={onContextMenu}
-      className={classNames('chat-list-item', {
+      className={classNames('chat-list-item roving-tabindex', {
         isError: true,
         selected: isSelected,
       })}
@@ -256,11 +282,24 @@ function ChatListItemNormal({
   isSelected?: boolean
   hover?: boolean
 }) {
+  const ref = useRef<HTMLButtonElement>(null)
+
+  const {
+    tabIndex,
+    onKeydown: tabindexOnKeydown,
+    setAsActiveElement: tabindexSetAsActiveElement,
+  } = useRovingTabindex(ref)
+  // TODO `setAsActiveElement` if `isSelected` and `activeElement === null`
+
   return (
     <button
+      ref={ref}
+      tabIndex={tabIndex}
       onClick={onClick}
+      onKeyDown={tabindexOnKeydown}
+      onFocus={tabindexSetAsActiveElement}
       onContextMenu={onContextMenu}
-      className={classNames('chat-list-item', {
+      className={classNames('chat-list-item roving-tabindex', {
         'has-unread': chatListItem.freshMessageCounter > 0,
         'is-contact-request': chatListItem.isContactRequest,
         pinned: chatListItem.isPinned,
@@ -368,11 +407,25 @@ export const ChatListItemMessageResult = React.memo<{
   queryStr: string
 }>(props => {
   const { msr, onClick, queryStr } = props
+
+  const ref = useRef<HTMLButtonElement>(null)
+
+  const {
+    tabIndex,
+    onKeydown: tabindexOnKeydown,
+    setAsActiveElement: tabindexSetAsActiveElement,
+  } = useRovingTabindex(ref)
+
   if (typeof msr === 'undefined') return <PlaceholderChatListItem />
+
   return (
     <button
+      ref={ref}
+      tabIndex={tabIndex}
       onClick={onClick}
-      className='pseudo-chat-list-item message-search-result'
+      onKeyDown={tabindexOnKeydown}
+      onFocus={tabindexSetAsActiveElement}
+      className='pseudo-chat-list-item message-search-result roving-tabindex'
     >
       <div className='avatars'>
         <Avatar
diff --git a/packages/frontend/src/contexts/RovingTabindex.tsx b/packages/frontend/src/contexts/RovingTabindex.tsx
new file mode 100644
index 0000000000..cc723c8390
--- /dev/null
+++ b/packages/frontend/src/contexts/RovingTabindex.tsx
@@ -0,0 +1,214 @@
+import { getLogger } from '@deltachat-desktop/shared/logger'
+import React, {
+  createContext,
+  PropsWithChildren,
+  RefObject,
+  useCallback,
+  useContext,
+  useEffect,
+  useState,
+} from 'react'
+
+/**
+ * Helps implement a keyboard-accessiable UI widget such that
+ * there is only one tab-stop in the entire widget,
+ * even if there are multiple interactive (focusable) elements
+ * within the widget itself (such as, say, a long list of chats).
+ * Implements [the "roving tabindex" approach](
+ * https://developer.mozilla.org/en-US/docs/Web/Accessibility/Keyboard-navigable_JavaScript_widgets#technique_1_roving_tabindex
+ * ).
+ *
+ * This is similar to https://www.npmjs.com/package/react-roving-tabindex, but
+ * - it handles elements changing their order in DOM:
+ *   'ArrowDown' will always focus the element that is next in DOM,
+ *   the same way as pressing Tab would,
+ *   regardless of where the element was on the initial render.
+ * - it handles elements getting removed from DOM, without `.focus()`ing
+ *   another element when the previously active one gets removed.
+ * - it's possible that there is no "active" element at all,
+ *   e.g. upon initialization.
+ *   Then `tabindex` is 0 for all the components,
+ *   until `setAsActiveElement` is called explicitly.
+ * - lacks "Home", "End" key handling, and some other features.
+ */
+export function useRovingTabindex(elementRef: RefObject<HTMLElement>) {
+  const context = useContext(RovingTabindexContext)
+
+  const tabIndex =
+    // If the active element has not been chosen yet,
+    // let' keep the default behavior (tabindex="0")
+    context.activeElement == null ||
+    context.activeElement === elementRef.current
+      ? 0
+      : -1
+
+  return {
+    tabIndex: tabIndex,
+    onKeydown: context.onKeydown,
+
+    // TODO make sure it is easy for the caller to select
+    // the initial active element (e.g. the currently selected chat).
+    // Maybe it already is.
+    // Or perhaps a provide a way to just set the first one in DOM
+    // as the active one.
+    /**
+     * Sets `elementRef.current` as the one who will have tabindex="0",
+     * and sets tabindex="-1" for all the other ones within this context.
+     * It is recommended to `setAsActiveElement` when it gets focused
+     * (`onFocus={setAsActiveElement}`)
+     */
+    // `elementRef.current` could be null, I am not sure
+    // if this means something to us practically.
+    setAsActiveElement: () => context.setActiveElement(elementRef.current),
+    activeElement: context.activeElement,
+  }
+}
+
+const log = getLogger('contexts/RovingTabindex')
+
+type ContextValue = {
+  activeElement: HTMLElement | null
+  onKeydown: (event: React.KeyboardEvent) => void
+  setActiveElement: (element: HTMLElement | null) => void
+}
+
+type ProviderProps = PropsWithChildren<{
+  /**
+   * An element that wraps all the elements that we need the roving tabindex
+   * behavior applied to them within this UI widget.
+   */
+  wrapperElementRef: RefObject<HTMLElement>
+  /**
+   * All elements that will utilize roving tabindex must have this
+   * class name set.
+   * @default 'roving-tabindex'
+   */
+  classNameOfTargetElements?: string
+}>
+
+export const RovingTabindexContext = createContext<ContextValue>({
+  activeElement: null,
+  onKeydown: () => {},
+  setActiveElement: () => {},
+})
+
+/** @see {@link useRovingTabindex} */
+export function RovingTabindexProvider({
+  children,
+  wrapperElementRef,
+  classNameOfTargetElements,
+}: ProviderProps) {
+  if (classNameOfTargetElements == undefined) {
+    classNameOfTargetElements = 'roving-tabindex'
+  }
+
+  const [activeElement, setActiveElement] = useState<HTMLElement | null>(null)
+
+  // Ensure that the "active" element is actually in DOM,
+  // because it's the only one that is keyboard focusable.
+  // Otherwise it's impossible to focus the widget by pressing "Tab".
+  // If it's not in DOM, we need to either set another element
+  // as the active one, or `setActiveElement(null)` to make
+  // all the elements focusable.
+  //
+  // FYI instead of using a `MutationObserver` we could probaby have
+  // utilized React's "unmount" callback (i.e. `useEffect`'s cleanup function),
+  // but I failed at it.
+  // Let's keep things vanilla then I guess.
+  useEffect(() => {
+    if (activeElement == null) {
+      return
+    }
+
+    const observer = new MutationObserver(_mutations => {
+      if (!document.body.contains(activeElement)) {
+        // Another option is to select the next/closest element
+        // as the new active one.
+        setActiveElement(null)
+      }
+    })
+    // Maybe we could observe `wrapperElementRef` for performance,
+    // but IDK let's play it safe.
+    observer.observe(document.body, { childList: true, subtree: true })
+    return () => observer.disconnect()
+  }, [activeElement])
+
+  const onKeydown = useCallback(
+    (event: React.KeyboardEvent) => {
+      if (!wrapperElementRef.current) {
+        log.warn(
+          'Received keydown event, but there is no wrapperElement? How?',
+          activeElement
+        )
+        return
+      }
+      if (!activeElement) {
+        // This could happen either after the initial render,
+        // or if the active element was removed from DOM.
+        // Let's just wait for the user to focus another element
+        // (and thus set `activeElement`).
+        return
+      }
+
+      const keyCode = event.code
+      if (keyCode !== 'ArrowDown' && keyCode !== 'ArrowUp') {
+        return
+      }
+
+      // This is mainly to prevent the scroll that usually happens by default
+      // when you press an arrow key,
+      // but it might have other benefits.
+      event.preventDefault()
+
+      // TODO perf: `getElementsByClassName` returns a live range.
+      // Perhaps we could cache it so as to not re-make it on every key press.
+      const eligibleElements = wrapperElementRef.current.getElementsByClassName(
+        classNameOfTargetElements
+      )
+
+      let oldActiveElementInd: number | undefined
+      for (let i = 0; i < eligibleElements.length; i++) {
+        const eligibleEl = eligibleElements[i]
+        if (eligibleEl === activeElement) {
+          oldActiveElementInd = i
+          break
+        }
+      }
+      if (oldActiveElementInd == undefined) {
+        log.warn(
+          'Could not find the currently active element in DOM',
+          activeElement
+        )
+        return
+      }
+
+      const newActiveElement =
+        eligibleElements[
+          oldActiveElementInd + (keyCode === 'ArrowDown' ? 1 : -1)
+        ]
+      // `newActiveElement` could be `undefined` if the active element is either
+      // the last or the first.
+      if (newActiveElement != undefined) {
+        const newActiveElement_ = newActiveElement as HTMLElement
+        setActiveElement(newActiveElement_)
+        // It is fine to `.focus()` here without wating for a render
+        // or `useEffect`, because elements with `tabindex="-1"`
+        // are still programmatically focusable.
+        newActiveElement_.focus()
+      }
+    },
+    [activeElement, classNameOfTargetElements, wrapperElementRef]
+  )
+
+  return (
+    <RovingTabindexContext.Provider
+      value={{
+        activeElement,
+        onKeydown,
+        setActiveElement,
+      }}
+    >
+      {children}
+    </RovingTabindexContext.Provider>
+  )
+}