From 69a0780f1f808665b03b2e600d1b8d5681124d47 Mon Sep 17 00:00:00 2001 From: WofWca Date: Thu, 31 Oct 2024 14:50:00 +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. --- .../frontend/scss/message/_message-list.scss | 18 ++- .../frontend/src/components/Avatar/index.tsx | 12 +- .../src/components/chat/ChatListItem.tsx | 2 +- .../src/components/composer/Composer.tsx | 2 +- .../components/message/EmptyChatMessage.tsx | 14 +- .../frontend/src/components/message/Link.tsx | 19 ++- .../src/components/message/Message.tsx | 139 +++++++++++++++--- .../src/components/message/MessageBody.tsx | 11 +- .../src/components/message/MessageList.tsx | 130 +++++++++------- .../components/message/MessageMarkdown.tsx | 93 ++++++++++-- 10 files changed, 340 insertions(+), 100 deletions(-) diff --git a/packages/frontend/scss/message/_message-list.scss b/packages/frontend/scss/message/_message-list.scss index fe7e97e842..da7f58dd87 100644 --- a/packages/frontend/scss/message/_message-list.scss +++ b/packages/frontend/scss/message/_message-list.scss @@ -123,10 +123,26 @@ scroll-margin-bottom: $margin-bottom; // And this is purely cosmetic. For scrolling to a message // that is above the visible area. - scroll-margin-top: math.div($margin-bottom, 2); + $scroll-margin-top: math.div($margin-bottom, 2); + scroll-margin-top: $scroll-margin-top; min-width: 200px; + .roving-tabindex { + // The contents of the `
  • ` are focusable (see `useRovingTabindex`). + // Focusing an element inside a scrollable region makes the browser + // scroll the focused element into view, so let's also apply the same + // margins for it. + // This is not as important though, but still nice. + // + // But ideally we'd probably want to refactor this in such a way that + // the focusabe element and the element that we `scrollIntoView()` + // when `jumpToMessage` is the same element, so only one + // needs `scroll-margin`. + scroll-margin-bottom: $margin-bottom; + scroll-margin-top: $scroll-margin-top; + } + &::after { visibility: hidden; display: block; diff --git a/packages/frontend/src/components/Avatar/index.tsx b/packages/frontend/src/components/Avatar/index.tsx index 21173fba88..0ee90d0227 100644 --- a/packages/frontend/src/components/Avatar/index.tsx +++ b/packages/frontend/src/components/Avatar/index.tsx @@ -43,6 +43,7 @@ export function Avatar(props: { wasSeenRecently?: boolean style?: htmlDivProps['style'] onClick?: () => void + tabIndex?: -1 | 0 className?: string }) { const { @@ -54,6 +55,7 @@ export function Avatar(props: { small, wasSeenRecently, onClick, + tabIndex, className, } = props @@ -73,6 +75,7 @@ export function Avatar(props: { className )} onClick={onClick} + tabIndex={tabIndex} > {content} @@ -80,11 +83,15 @@ export function Avatar(props: { } export function AvatarFromContact( - props: { contact: Type.Contact; onClick?: (contact: Type.Contact) => void }, + props: { + contact: Type.Contact + onClick?: (contact: Type.Contact) => void + tabIndex?: -1 | 0 + }, large?: boolean, small?: boolean ) { - const { contact, onClick } = props + const { contact, onClick, tabIndex } = props return ( onClick && onClick(contact)} + tabIndex={tabIndex} /> ) } diff --git a/packages/frontend/src/components/chat/ChatListItem.tsx b/packages/frontend/src/components/chat/ChatListItem.tsx index c4d5688bf9..0636e1b4b5 100644 --- a/packages/frontend/src/components/chat/ChatListItem.tsx +++ b/packages/frontend/src/components/chat/ChatListItem.tsx @@ -126,7 +126,7 @@ const Message = React.memo< }} /> )} - {message2React(summaryText2 || '', true)} + {message2React(summaryText2 || '', true, -1)} {isContactRequest && (
    diff --git a/packages/frontend/src/components/composer/Composer.tsx b/packages/frontend/src/components/composer/Composer.tsx index 45ca670004..67347a4e13 100644 --- a/packages/frontend/src/components/composer/Composer.tsx +++ b/packages/frontend/src/components/composer/Composer.tsx @@ -339,7 +339,7 @@ const Composer = forwardRef<
    {draftState.quote !== null && (
    - +
    )} diff --git a/packages/frontend/src/components/message/EmptyChatMessage.tsx b/packages/frontend/src/components/message/EmptyChatMessage.tsx index 6434ce30b1..bfefc8e237 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 7ceda653fb..30fa9aafcb 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 25243b5f0c..8ab7297629 100644 --- a/packages/frontend/src/components/message/Message.tsx +++ b/packages/frontend/src/components/message/Message.tsx @@ -1,4 +1,10 @@ -import React, { useCallback, useContext, useEffect, useState } from 'react' +import React, { + useCallback, + useContext, + useEffect, + useRef, + useState, +} from 'react' import reactStringReplace from 'react-string-replace' import classNames from 'classnames' import { C, T } from '@deltachat/jsonrpc-client' @@ -51,13 +57,16 @@ import styles from './styles.module.scss' import type { OpenDialog } from '../../contexts/DialogContext' import type { PrivateReply } from '../../hooks/chat/usePrivateReply' import { mouseEventToPosition } from '../../utils/mouseEventToPosition' +import { useRovingTabindex } from '../../contexts/RovingTabindex' const Avatar = ({ contact, onContactClick, + tabIndex, }: { contact: T.Contact onContactClick: (contact: T.Contact) => void + tabIndex: -1 | 0 }) => { const { profileImage, color, displayName } = contact @@ -65,7 +74,7 @@ const Avatar = ({ if (profileImage) { return ( -
    +
    {displayName}
    ) @@ -79,6 +88,7 @@ const Avatar = ({ className='author-avatar default' aria-label={displayName} onClick={onClick} + tabIndex={tabIndex} >
    {initial} @@ -92,10 +102,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 @@ -121,6 +133,7 @@ const AuthorName = ({ className='author' style={{ color }} onClick={() => onContactClick(contact)} + tabIndex={tabIndex} > {getAuthorName(displayName, overrideSenderName)} @@ -133,12 +146,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() @@ -153,6 +168,7 @@ const ForwardedTitle = ({ () => ( onContactClick(contact)} + tabIndex={tabIndex} key='displayname' style={{ color: color }} > @@ -161,7 +177,7 @@ const ForwardedTitle = ({ ) ) ) : ( - onContactClick(contact)}> + onContactClick(contact)} tabIndex={tabIndex}> {tx('forwarded_message')} )} @@ -344,6 +360,7 @@ export default function Message(props: { chat: T.FullChat message: T.Message conversationType: ConversationType + // tabindexForInteractiveContents: -1 | 0 }) { const { message, conversationType } = props const { id, viewType, text, hasLocation, isSetupmessage, hasHtml } = message @@ -435,6 +452,26 @@ export default function Message(props: { ] ) + const ref = useRef(null) + const rovingTabindex = useRovingTabindex(ref) + const rovingTabindexAttrs = { + ref, + tabIndex: rovingTabindex.tabIndex, + onKeyDown: rovingTabindex.onKeydown, + onFocus: rovingTabindex.setAsActiveElement, + } + // When the message is not the active one + // `rovingTabindex.tabIndex === -1`, we need to set `tabindex="-1"` + // to all its interactive (otherwise "Tabbable to") elements, + // such as links, attachments, "view reactions" button, etc. + // Only the contents of the "active" (selected) message + // should have tab stops. + // See https://github.com/deltachat/deltachat-desktop/issues/2141 + // WhatsApp appears to behave similarly. + // The implementation is similar to the "Grid" pattern: + // https://www.w3.org/WAI/ARIA/apg/patterns/grid/#gridNav_inside + const tabindexForInteractiveContents = rovingTabindex.tabIndex + // Info Message if (message.isInfo) { const isWebxdcInfo = message.systemMessageType === 'WebxdcInfoMessage' @@ -480,11 +517,13 @@ export default function Message(props: { className={classNames( 'info-message', isWebxdcInfo && 'webxdc-info', - isInteractive && 'interactive' + isInteractive && 'interactive', + rovingTabindex.className )} id={String(message.id)} onContextMenu={showContextMenu} onClick={onClick} + {...rovingTabindexAttrs} > {(isProtectionBrokenMsg || isProtectionEnabledMsg) && ( +
    - +
    joinVideoChat(accountId, id)} + tabIndex={tabindexForInteractiveContents} > {direction === 'incoming' ? tx('videochat_contact_invited_hint', message.sender.displayName) @@ -563,6 +611,7 @@ export default function Message(props: { padlock={message.showPadlock} onClickError={openMessageInfo.bind(null, openDialog, message)} viewType={'VideochatInvitation'} + // TODO tabIndex={tabindexForInteractiveContents} />
    @@ -573,7 +622,10 @@ export default function Message(props: { {message.isSetupmessage ? ( tx('autocrypt_asm_click_body') ) : text !== null ? ( - + ) : null}
    ) @@ -594,7 +646,10 @@ export default function Message(props: { {tx('downloading')} )} {(downloadState == 'Failure' || downloadState === 'Available') && ( - )} @@ -618,17 +673,31 @@ export default function Message(props: { return (
    {showAuthor && direction === 'incoming' && ( - + )}
    )} {!message.isForwarded && ( @@ -656,6 +726,7 @@ export default function Message(props: { contact={message.sender} onContactClick={onContactClick} overrideSenderName={message.overrideSenderName} + tabIndex={tabindexForInteractiveContents} />
    )} @@ -664,9 +735,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' && ( @@ -687,6 +766,7 @@ export default function Message(props: {
    {tx('show_full_message')}
    @@ -707,9 +787,13 @@ export default function Message(props: { padlock={message.showPadlock} onClickError={openMessageInfo.bind(null, openDialog, message)} viewType={message.viewType} + // TODO tabIndex={tabindexForInteractiveContents} /> {message.reactions && !isSetupmessage && ( - + )}
    @@ -727,9 +811,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() @@ -759,6 +845,7 @@ export const Quote = ({ scrollIntoViewArg: { block: 'nearest' }, }) }} + tabIndex={tabIndex} >
    )} @@ -829,7 +917,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 @@ -846,6 +940,8 @@ function WebxdcMessageContent({ message }: { message: T.Message }) { src={runtime.getWebxdcIconURL(selectedAccountId(), message.id)} alt={`icon of ${info.name}`} onClick={() => openWebxdc(message)} + // Not setting `tabIndex={tabindexForInteractiveContents}` here + // because there is a button below that does the same />
    openWebxdc(message)} + tabIndex={tabindexForInteractiveContents} > {tx('start_app')} diff --git a/packages/frontend/src/components/message/MessageBody.tsx b/packages/frontend/src/components/message/MessageBody.tsx index c798b3b19a..f25e242f5f 100644 --- a/packages/frontend/src/components/message/MessageBody.tsx +++ b/packages/frontend/src/components/message/MessageBody.tsx @@ -10,6 +10,7 @@ function MessageBody({ text, disableJumbomoji, nonInteractiveContent = false, + tabindexForInteractiveContents, }: { text: string disableJumbomoji?: boolean @@ -18,6 +19,10 @@ function MessageBody({ * display them as regular text. */ nonInteractiveContent?: boolean + /** + * Has no effect when {@link nonInteractiveContent} === true. + */ + tabindexForInteractiveContents?: -1 | 0 }): JSX.Element { if (text.length >= UPPER_LIMIT_FOR_PARSED_MESSAGES) { return <>{text} @@ -34,7 +39,11 @@ function MessageBody({ ) } } - return message2React(emojifiedText, nonInteractiveContent) + return message2React( + emojifiedText, + nonInteractiveContent, + 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 91c4c2db41..81bf8180d7 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 @@ -795,64 +799,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} +
      -
      - ) + ) + } } - } - })} + })} +
    ) @@ -926,9 +935,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 cecc22866e..7083b7b7c4 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'} @@ -147,13 +178,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 }) @@ -161,7 +204,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() @@ -179,13 +228,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}"` @@ -199,13 +249,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() @@ -281,7 +337,12 @@ function BotCommandSuggestion({ suggestion }: { suggestion: string }) { } return ( - + {suggestion} )