From 901f71c262cf88845a1cde8393bae6c931c26ab1 Mon Sep 17 00:00:00 2001 From: WofWca Date: Wed, 30 Oct 2024 19:58:34 +0400 Subject: [PATCH] WIP: improvement: a11y: arrow-key navigation for messages Closes https://github.com/deltachat/deltachat-desktop/issues/2141 Basically what this commit comes down to: 1. Apply `useRovingTabindex` for message items 2. Set `tabindex="-1"` on all the interactive items inside every message that is currently not the active one, so that they do no have tab stops. TODO: - [ ] Address the TODOs in the code - [ ] Manage what's gonna be the initially active message, because initially they're all active, so tabbing to the messages list from the top selects the first rendered one as the active one. https://github.com/deltachat/deltachat-desktop/pull/4292 could help with this. This is also not great for performance: changing `tabindex` on a bunch of messages makes them all re-render. And otherwise, we probably want to update which one is the active one as new messages arrive. - [ ] The interactive items with `onClick` must be actual semantic ` )} @@ -623,7 +648,11 @@ export default function Message(props: { id={message.id.toString()} > {showAuthor && direction === 'incoming' && ( - + )}
)} {!message.isForwarded && ( @@ -651,6 +681,7 @@ export default function Message(props: { contact={message.sender} onContactClick={onContactClick} overrideSenderName={message.overrideSenderName} + tabIndex={tabindexForInteractiveContents} />
)} @@ -659,9 +690,14 @@ export default function Message(props: { 'msg-body--clickable': onClickMessageBody, })} onClick={onClickMessageBody} + tabIndex={onClickMessageBody ? tabindexForInteractiveContents : -1} > {message.quote !== null && ( - + )} {showAttachment(message) && ( )} {message.viewType === 'Webxdc' && ( - + )} {message.viewType === 'Vcard' && ( @@ -682,6 +721,7 @@ export default function Message(props: {
{tx('show_full_message')}
@@ -702,6 +742,7 @@ export default function Message(props: { padlock={message.showPadlock} onClickError={openMessageInfo.bind(null, openDialog, message)} viewType={message.viewType} + // TODO tabIndex={tabindexForInteractiveContents} /> {message.reactions && !isSetupmessage && ( @@ -722,9 +763,11 @@ export default function Message(props: { export const Quote = ({ quote, msgParentId, + tabIndex, }: { quote: T.MessageQuote msgParentId?: number + tabIndex: -1 | 0 }) => { const tx = useTranslationFunction() const accountId = selectedAccountId() @@ -751,6 +794,7 @@ export const Quote = ({ msgParentId ) }} + tabIndex={tabIndex} >
@@ -817,7 +862,13 @@ export function getAuthorName( return overrideSenderName ? `~${overrideSenderName}` : displayName } -function WebxdcMessageContent({ message }: { message: T.Message }) { +function WebxdcMessageContent({ + message, + tabindexForInteractiveContents, +}: { + message: T.Message + tabindexForInteractiveContents: -1 | 0 +}) { const tx = useTranslationFunction() if (message.viewType !== 'Webxdc') { return null @@ -835,6 +886,8 @@ function WebxdcMessageContent({ message }: { message: T.Message }) { src={runtime.getWebxdcIconURL(selectedAccountId(), message.id)} alt={`icon of ${info.name}`} onClick={() => openWebxdc(message.id)} + // Not setting `tabIndex={tabindexForInteractiveContents}` here + // because there is a button below that does the same />
openWebxdc(message.id)} + tabIndex={tabindexForInteractiveContents} > {tx('start_app')} diff --git a/packages/frontend/src/components/message/MessageBody.tsx b/packages/frontend/src/components/message/MessageBody.tsx index 3d1c8c5780..88c21098a0 100644 --- a/packages/frontend/src/components/message/MessageBody.tsx +++ b/packages/frontend/src/components/message/MessageBody.tsx @@ -9,9 +9,11 @@ const UPPER_LIMIT_FOR_PARSED_MESSAGES = 20_000 function MessageBody({ text, disableJumbomoji, + tabindexForInteractiveContents, }: { text: string disableJumbomoji?: boolean + tabindexForInteractiveContents?: -1 | 0 }): JSX.Element { if (text.length >= UPPER_LIMIT_FOR_PARSED_MESSAGES) { return <>{text} @@ -28,7 +30,11 @@ function MessageBody({ ) } } - return message2React(emojifiedText, false) + return message2React( + emojifiedText, + false, + tabindexForInteractiveContents ?? 0 + ) } const trimRegex = /^[\s\uFEFF\xA0\n\t]+|[\s\uFEFF\xA0\n\t]+$/g diff --git a/packages/frontend/src/components/message/MessageList.tsx b/packages/frontend/src/components/message/MessageList.tsx index da960021a2..ee0b2a5d80 100644 --- a/packages/frontend/src/components/message/MessageList.tsx +++ b/packages/frontend/src/components/message/MessageList.tsx @@ -28,6 +28,10 @@ import EmptyChatMessage from './EmptyChatMessage' const log = getLogger('render/components/message/MessageList') import type { T } from '@deltachat/jsonrpc-client' +import { + RovingTabindexProvider, + useRovingTabindex, +} from '../../contexts/RovingTabindex' type ChatTypes = | C.DC_CHAT_TYPE_SINGLE @@ -717,64 +721,69 @@ export const MessageListInner = React.memo( return (
    - {messageListItems.length === 0 && } - {activeView.map(messageId => { - if (messageId.kind === 'dayMarker') { - return ( - - ) - } - - if (messageId.kind === 'message') { - const message = messageCache[messageId.msg_id] - if (message?.kind === 'message') { + + {messageListItems.length === 0 && } + {activeView.map(messageId => { + if (messageId.kind === 'dayMarker') { return ( - ) - } else if (message?.kind === 'loadingError') { - return ( -
    -
    - loading message {messageId.msg_id} failed: {message.error} + } + + if (messageId.kind === 'message') { + const message = messageCache[messageId.msg_id] + if (message?.kind === 'message') { + return ( + + ) + } else if (message?.kind === 'loadingError') { + // TODO shall we add `useRovingTabindex` here as well? + return ( +
    +
    + loading message {messageId.msg_id} failed:{' '} + {message.error} +
    -
    - ) - } else { - // setTimeout tells it to call method in next event loop iteration, so after rendering - // it is debounced later so we can call it here multiple times and it's ok - setTimeout(loadMissingMessages) - return ( -
    -
    - Loading Message {messageId.msg_id} + ) + } else { + // setTimeout tells it to call method in next event loop iteration, so after rendering + // it is debounced later so we can call it here multiple times and it's ok + setTimeout(loadMissingMessages) + // TODO shall we add `useRovingTabindex` here as well? + return ( +
    +
    + Loading Message {messageId.msg_id} +
    -
    - ) + ) + } } - } - })} + })} +
) @@ -840,9 +849,24 @@ function JumpDownButton({ export function DayMarker(props: { timestamp: number }) { const { timestamp } = props const tx = useTranslationFunction() + + const ref = useRef(null) + // Yes, we want daymakers to be focusable. + // See https://github.com/deltachat/deltachat-desktop/issues/2141 + // > Also make the divider items proper list items that can be focused, + // > so users know when they traverse to the next/previous date. + const rovingTabindex = useRovingTabindex(ref) + return (
-
+
{moment.unix(timestamp).calendar(null, { sameDay: `[${tx('today')}]`, lastDay: `[${tx('yesterday')}]`, diff --git a/packages/frontend/src/components/message/MessageMarkdown.tsx b/packages/frontend/src/components/message/MessageMarkdown.tsx index 582b6223a3..ce26814133 100644 --- a/packages/frontend/src/components/message/MessageMarkdown.tsx +++ b/packages/frontend/src/components/message/MessageMarkdown.tsx @@ -30,7 +30,13 @@ SettingsStoreInstance.subscribe(newState => { } }) -function renderElement(elm: ParsedElement, key?: number): JSX.Element { +function renderElement( + elm: ParsedElement, + tabindexForInteractiveContents: -1 | 0, + key?: number +): JSX.Element { + const mapFn = (elm: ParsedElement, index: number) => + renderElement(elm, tabindexForInteractiveContents, index) switch (elm.t) { case 'CodeBlock': return ( @@ -48,20 +54,32 @@ function renderElement(elm: ParsedElement, key?: number): JSX.Element { ) case 'StrikeThrough': - return {elm.c.map(renderElement)} + return {elm.c.map(mapFn)} case 'Italics': - return {elm.c.map(renderElement)} + return {elm.c.map(mapFn)} case 'Bold': - return {elm.c.map(renderElement)} + return {elm.c.map(mapFn)} case 'Tag': - return + return ( + + ) case 'Link': { const { destination } = elm.c - return + return ( + + ) } case 'LabeledLink': @@ -69,18 +87,31 @@ function renderElement(elm: ParsedElement, key?: number): JSX.Element { {elm.c.label.map(renderElement)}} + label={<>{elm.c.label.map(mapFn)}} + tabIndex={tabindexForInteractiveContents} />{' '} ) case 'EmailAddress': { const email = elm.c - return + return ( + + ) } case 'BotCommandSuggestion': - return + return ( + + ) case 'Linebreak': return {'\n'} @@ -144,13 +175,25 @@ function renderElementPreview(elm: ParsedElement, key?: number): JSX.Element { } } -export function message2React(message: string, preview: boolean): JSX.Element { +export function message2React( + message: string, + preview: boolean, + /** + * Has no effect `{@link preview} === true`, because there should be + * no interactive elements in the first place + */ + tabindexForInteractiveContents: -1 | 0 +): JSX.Element { try { const elements = parseMessage(message) return preview ? (
{elements.map(renderElementPreview)}
) : ( - <>{elements.map(renderElement)} + <> + {elements.map((el, index) => + renderElement(el, tabindexForInteractiveContents, index) + )} + ) } catch (error) { log.error('parseMessage failed:', { input: message, error }) @@ -158,7 +201,13 @@ export function message2React(message: string, preview: boolean): JSX.Element { } } -function EmailLink({ email }: { email: string }): JSX.Element { +function EmailLink({ + email, + tabIndex, +}: { + email: string + tabIndex: -1 | 0 +}): JSX.Element { const accountId = selectedAccountId() const createChatByEmail = useCreateChatByEmail() const { selectChat } = useChat() @@ -176,13 +225,14 @@ function EmailLink({ email }: { email: string }): JSX.Element { x-not-a-link='email' x-target-email={email} onClick={handleClick} + tabIndex={tabIndex} > {email} ) } -function TagLink({ tag }: { tag: string }) { +function TagLink({ tag, tabIndex }: { tag: string; tabIndex: -1 | 0 }) { const setSearch = () => { log.debug( `Clicked on a hastag, this should open search for the text "${tag}"` @@ -196,13 +246,19 @@ function TagLink({ tag }: { tag: string }) { } return ( - + {tag} ) } -function BotCommandSuggestion({ suggestion }: { suggestion: string }) { +function BotCommandSuggestion({ + suggestion, + tabIndex, +}: { + suggestion: string + tabIndex: -1 | 0 +}) { const openConfirmationDialog = useConfirmationDialog() const messageDisplay = useContext(MessagesDisplayContext) const accountId = selectedAccountId() @@ -278,7 +334,12 @@ function BotCommandSuggestion({ suggestion }: { suggestion: string }) { } return ( - + {suggestion} ) diff --git a/packages/frontend/src/components/message/MessageWrapper.tsx b/packages/frontend/src/components/message/MessageWrapper.tsx index d578035982..4bd62f84c8 100644 --- a/packages/frontend/src/components/message/MessageWrapper.tsx +++ b/packages/frontend/src/components/message/MessageWrapper.tsx @@ -1,4 +1,4 @@ -import React, { useLayoutEffect } from 'react' +import React, { useLayoutEffect, useRef } from 'react' import { C } from '@deltachat/jsonrpc-client' import Message from './Message' @@ -6,6 +6,7 @@ import { ConversationType } from './MessageList' import { getLogger } from '../../../../shared/logger' import type { T } from '@deltachat/jsonrpc-client' +import { useRovingTabindex } from '../../contexts/RovingTabindex' type RenderMessageProps = { key2: string @@ -63,9 +64,33 @@ export function MessageWrapper(props: RenderMessageProps) { shouldInViewObserve, ]) + const ref = useRef(null) + const rovingTabindex = useRovingTabindex(ref) + return ( -
  • - + // TODO fix: invoking the context menu with Shift + F10 doesn't work. +
  • +
    • )