From 21857cf645f669198e5223fb61d2dd4c386ff6b4 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. --- .../components/message/EmptyChatMessage.tsx | 14 +- .../frontend/src/components/message/Link.tsx | 19 ++- .../src/components/message/Message.tsx | 74 ++++++++-- .../src/components/message/MessageBody.tsx | 8 +- .../src/components/message/MessageList.tsx | 130 +++++++++++------- .../components/message/MessageMarkdown.tsx | 93 ++++++++++--- .../src/components/message/MessageWrapper.tsx | 31 ++++- 7 files changed, 282 insertions(+), 87 deletions(-) diff --git a/packages/frontend/src/components/message/EmptyChatMessage.tsx b/packages/frontend/src/components/message/EmptyChatMessage.tsx index 6434ce30b..bfefc8e23 100644 --- a/packages/frontend/src/components/message/EmptyChatMessage.tsx +++ b/packages/frontend/src/components/message/EmptyChatMessage.tsx @@ -1,9 +1,10 @@ -import React from 'react' +import React, { useRef } from 'react' import { C } from '@deltachat/jsonrpc-client' import useTranslationFunction from '../../hooks/useTranslationFunction' import type { T } from '@deltachat/jsonrpc-client' +import { useRovingTabindex } from '../../contexts/RovingTabindex' type Props = { chat: T.FullChat @@ -12,6 +13,9 @@ type Props = { export default function EmptyChatMessage({ chat }: Props) { const tx = useTranslationFunction() + const ref = useRef(null) + const rovingTabindex = useRovingTabindex(ref) + let emptyChatMessage = tx('chat_new_one_to_one_hint', [chat.name, chat.name]) if (chat.chatType === C.DC_CHAT_TYPE_BROADCAST) { @@ -29,7 +33,13 @@ export default function EmptyChatMessage({ chat }: Props) { } return ( -
  • +
  • {emptyChatMessage}
    diff --git a/packages/frontend/src/components/message/Link.tsx b/packages/frontend/src/components/message/Link.tsx index 7ceda653f..30fa9aafc 100644 --- a/packages/frontend/src/components/message/Link.tsx +++ b/packages/frontend/src/components/message/Link.tsx @@ -42,9 +42,11 @@ function isDomainTrusted(domain: string): boolean { export const LabeledLink = ({ label, destination, + tabIndex, }: { label: string | JSX.Element | JSX.Element[] destination: LinkDestination + tabIndex: -1 | 0 }) => { const { openDialog } = useDialog() const openLinkSafely = useOpenLinkSafely() @@ -84,6 +86,7 @@ export const LabeledLink = ({ x-target-url={target} title={realUrl} onClick={onClick} + tabIndex={tabIndex} onContextMenu={ev => ((ev as any).t = ev.currentTarget)} > {label} @@ -164,7 +167,13 @@ function LabeledLinkConfirmationDialog( ) } -export const Link = ({ destination }: { destination: LinkDestination }) => { +export const Link = ({ + destination, + tabIndex, +}: { + destination: LinkDestination + tabIndex: -1 | 0 +}) => { const { openDialog } = useDialog() const openLinkSafely = useOpenLinkSafely() const accountId = selectedAccountId() @@ -193,7 +202,13 @@ export const Link = ({ destination }: { destination: LinkDestination }) => { } return ( - + {target} ) diff --git a/packages/frontend/src/components/message/Message.tsx b/packages/frontend/src/components/message/Message.tsx index 3ebc1abb5..549f8bf56 100644 --- a/packages/frontend/src/components/message/Message.tsx +++ b/packages/frontend/src/components/message/Message.tsx @@ -54,9 +54,11 @@ import type { PrivateReply } from '../../hooks/chat/usePrivateReply' const Avatar = ({ contact, onContactClick, + tabIndex, }: { contact: T.Contact onContactClick: (contact: T.Contact) => void + tabIndex: -1 | 0 }) => { const { profileImage, color, displayName } = contact @@ -64,7 +66,7 @@ const Avatar = ({ if (profileImage) { return ( -
    +
    {displayName}
    ) @@ -78,6 +80,7 @@ const Avatar = ({ className='author-avatar default' aria-label={displayName} onClick={onClick} + tabIndex={tabIndex} >
    {initial} @@ -91,10 +94,12 @@ const AuthorName = ({ contact, onContactClick, overrideSenderName, + tabIndex, }: { contact: T.Contact onContactClick: (contact: T.Contact) => void overrideSenderName: string | null + tabIndex: -1 | 0 }) => { const accountId = selectedAccountId() const { color, id } = contact @@ -120,6 +125,7 @@ const AuthorName = ({ className='author' style={{ color }} onClick={() => onContactClick(contact)} + tabIndex={tabIndex} > {getAuthorName(displayName, overrideSenderName)} @@ -132,12 +138,14 @@ const ForwardedTitle = ({ direction, conversationType, overrideSenderName, + tabIndex, }: { contact: T.Contact onContactClick: (contact: T.Contact) => void direction: 'incoming' | 'outgoing' conversationType: ConversationType overrideSenderName: string | null + tabIndex: -1 | 0 }) => { const tx = useTranslationFunction() @@ -152,6 +160,7 @@ const ForwardedTitle = ({ () => ( onContactClick(contact)} + tabIndex={tabIndex} key='displayname' style={{ color: color }} > @@ -160,7 +169,7 @@ const ForwardedTitle = ({ ) ) ) : ( - onContactClick(contact)}> + onContactClick(contact)} tabIndex={tabIndex}> {tx('forwarded_message')} )} @@ -343,8 +352,9 @@ export default function Message(props: { chat: T.FullChat message: T.Message conversationType: ConversationType + tabindexForInteractiveContents: -1 | 0 }) { - const { message, conversationType } = props + const { message, conversationType, tabindexForInteractiveContents } = props const { id, viewType, text, hasLocation, isSetupmessage, hasHtml } = message const direction = getDirection(message) const status = mapCoreMsgStatus2String(message.state) @@ -480,6 +490,9 @@ export default function Message(props: { id={String(message.id)} onContextMenu={showContextMenu} onClick={onClick} + // Using tabindex even when `!isInteractive` because it has + // a context menu. + tabIndex={tabindexForInteractiveContents} > {(isProtectionBrokenMsg || isProtectionEnabledMsg) && (
    - +
    joinVideoChat(accountId, id)} + tabIndex={tabindexForInteractiveContents} > {direction === 'incoming' ? tx('videochat_contact_invited_hint', message.sender.displayName) @@ -558,6 +576,7 @@ export default function Message(props: { padlock={message.showPadlock} onClickError={openMessageInfo.bind(null, openDialog, message)} viewType={'VideochatInvitation'} + // TODO tabIndex={tabindexForInteractiveContents} />
    @@ -568,7 +587,10 @@ export default function Message(props: { {message.isSetupmessage ? ( tx('autocrypt_asm_click_body') ) : text !== null ? ( - + ) : null}
    ) @@ -589,7 +611,10 @@ export default function Message(props: { {tx('downloading')} )} {(downloadState == 'Failure' || downloadState === 'Available') && ( - )} @@ -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 3d1c8c578..88c21098a 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 da960021a..ee0b2a5d8 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 582b6223a..ce2681413 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 d57803598..4bd62f84c 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. +
  • +
    • )