-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[WIP] Chat history accessibility navigation pattern update #5638
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
compulim
wants to merge
48
commits into
main
Choose a base branch
from
feat-chat-history-accessibility
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 9 commits
Commits
Show all changes
48 commits
Select commit
Hold shift + click to select a range
718ab9e
Build a blueprint
compulim b98f4ff
Add focus trap
compulim 8ab4310
Simple styling
compulim 373f0e8
Change to <section>
compulim 35cd62e
Add comment
compulim b0b2073
Remove bodyId
compulim 454380b
Remove role="none"
compulim 6e0e886
Add CSS comments
compulim 62e9278
Remove commented code
compulim ca02096
Add tests for ideal scenario
compulim 0e453a9
Add tests
compulim 6be4380
Remove deprecated test
compulim b6b2f44
Add tests
compulim bb46a22
Clean up
compulim f9cfb6d
Clean up
compulim 1c15961
Refactor id
compulim ad0343e
Use ID to save active message
compulim c624252
Keep position after message added
compulim 42bffe3
Longer timeout
compulim c0dede7
Add ALT+A
compulim dc82423
Move to roving tab index
compulim dfa3758
Simplify ChatMessage API
compulim 7ab539c
Comments
compulim 90296bd
Fix ESCAPE to send box
compulim 39e5e33
Remove console
compulim 8fc4b72
Simplify unnecessary deps
compulim 5422c7a
No need roving tab index
compulim 0b37b6b
Clean up
compulim 2b3d585
Use useId()
compulim 8d223a4
TAB and SHIFT-TAB will send focus into focus trap
compulim 63654f0
Add @ts-ignore on useId
compulim cc296a6
Unify keydown
compulim 2bc5bcb
Clean up
compulim 71e68db
Clean up some code
compulim b2b26ef
Use jumpToRelativeMessage to reduce reliance on focusedMessageId
compulim 45530c2
Comments
compulim fe0185b
Add comments
compulim b5e1b2c
Add comments
compulim 1dbe157
Fix clause
compulim 0dc30cb
Add Adaptive Card
compulim 1184a71
Remove unused id
compulim e74e37d
Add attachment label
compulim e97edf0
Add Windows Narrator quirks comment
compulim 35eca65
Add interact mode 2
compulim ed94d8b
Reduce reading in mode 2
compulim 61bf2c2
Arrow to move between chat history and send box
compulim 5ed93d3
Better CSS
compulim 82b54c7
Clean up comments
compulim File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,348 @@ | ||
| <!doctype html> | ||
| <html lang="en-US"> | ||
| <head> | ||
|
Check failure on line 3 in __tests__/html2/accessibility/scanMode/ideal.html
|
||
| <link href="/assets/index.css" rel="stylesheet" type="text/css" /> | ||
| <style type="text/css"> | ||
| body { | ||
| font-family: 'Segoe UI', Tahoma, Geneva, Verdana, sans-serif; | ||
| } | ||
|
|
||
| .chat-history__body { | ||
| display: flex; | ||
| flex-direction: column; | ||
| gap: 12px; | ||
| padding: 6px; | ||
| } | ||
|
|
||
| /* Can we eliminate .chat-history:focus? It doesn't follow BEM. */ | ||
| .chat-history:focus .chat-message__is-active { | ||
| outline: dashed 2px black; | ||
| outline-offset: 2px; | ||
| } | ||
|
|
||
| .chat-message { | ||
| background-color: White; | ||
| border: solid 1px #ddd; | ||
| border-radius: 8px; | ||
| box-shadow: 0 0 10px rgba(0, 0, 0, 0.05); | ||
| padding: 8px; | ||
| } | ||
|
|
||
| .chat-message__header { | ||
| color: #999; | ||
| font-size: smaller; | ||
| font-weight: lighter; | ||
| margin-block-start: 0; | ||
| margin-block-end: 0.2em; | ||
| } | ||
|
|
||
| .chat-message__content > .focus-trap > p:first-of-type { | ||
| margin-block-start: 0; | ||
| } | ||
|
|
||
| .chat-message__content > .focus-trap > p:last-of-type { | ||
| margin-block-end: 0; | ||
| } | ||
|
|
||
| .chat-message__content, | ||
| .focus-trap { | ||
| /* These elements are meaningless in CSS world, adding "display: contents" to save some CPUs. */ | ||
| display: contents; | ||
| } | ||
|
|
||
| .send-box { | ||
| margin: 6px; | ||
| } | ||
| </style> | ||
| </head> | ||
| <body> | ||
| <main></main> | ||
| <script type="importmap"> | ||
| { | ||
| "imports": { | ||
| "classnames": "https://esm.sh/classnames", | ||
| "jest-mock": "https://esm.sh/jest-mock", | ||
| "react": "https://esm.sh/react", | ||
| "react-dom": "https://esm.sh/react-dom" | ||
| } | ||
| } | ||
| </script> | ||
| <script crossorigin="anonymous" src="https://esm.sh/tsx" type="module"></script> | ||
| <script type="text/babel"> | ||
| import cx from 'classnames'; | ||
| import { useRefFrom } from 'https://esm.sh/use-ref-from'; | ||
| import { useStateWithRef } from 'https://esm.sh/use-state-with-ref'; | ||
| import { forwardRef, useCallback, useEffect, useMemo, useRef, useState } from 'react'; | ||
| import { createRoot } from 'react-dom/client'; | ||
|
|
||
| const FOCUSABLE_SELECTOR_QUERY = [ | ||
| 'a[href]', | ||
| 'button:not([disabled])', | ||
| 'textarea:not([disabled])', | ||
| 'input:not([disabled])', | ||
| 'select:not([disabled])', | ||
| '[tabindex]:not([tabindex="-1"])' | ||
| ].join(','); | ||
|
|
||
| function usePrevious(value) { | ||
| const previousRef = useRef(); | ||
|
|
||
| useEffect(() => { | ||
| previousRef.current = value; | ||
| }); | ||
|
|
||
| return previousRef.current; | ||
| } | ||
|
|
||
| // TODO: Use our own implementation of <FocusTrap>, we have better UX: | ||
| // - Save last focus | ||
| // - When an element become non-focusable | ||
| // However, this implementation is better at: | ||
| // - Handle "inert" attribute | ||
| // - Handle invisible element (element without `offsetParent`) | ||
| function FocusTrap({ children, onLeave }) { | ||
| const onLeaveRef = useRefFrom(onLeave); | ||
| const rootRef = useRef(); | ||
|
|
||
| const handleKeyDown = useCallback( | ||
| event => { | ||
| const container = rootRef.current; | ||
|
|
||
| if (!container) { | ||
| return; | ||
| } | ||
|
|
||
| if (event.key === 'Tab') { | ||
| const focusables = Array.from(container.querySelectorAll(FOCUSABLE_SELECTOR_QUERY)).filter( | ||
| element => !element.closest('[inert]') && element.offsetParent | ||
| ); | ||
|
|
||
| if (focusables.length === 0) { | ||
| return; | ||
| } | ||
|
|
||
| const firstElement = focusables[0]; | ||
| const lastElement = focusables.at(-1); | ||
|
|
||
| if (event.shiftKey && document.activeElement === firstElement) { | ||
| event.preventDefault(); | ||
| event.stopPropagation(); | ||
|
|
||
| lastElement.focus(); | ||
| } else if (!event.shiftKey && document.activeElement === lastElement) { | ||
| event.preventDefault(); | ||
| event.stopPropagation(); | ||
|
|
||
| firstElement.focus(); | ||
| } | ||
| } else if (event.key === 'Escape') { | ||
| event.stopPropagation(); | ||
|
|
||
| onLeaveRef.current?.(); | ||
| } | ||
| }, | ||
| [onLeaveRef] | ||
| ); | ||
|
|
||
| return ( | ||
| <div className="focus-trap" onKeyDown={handleKeyDown} ref={rootRef}> | ||
| {children} | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| function ChatMessage({ abstract, activeMode, children, index, onLeave, onRequestFocus }) { | ||
| const bodyRef = useRef(); | ||
| const contentId = useMemo(() => crypto.randomUUID(), []); | ||
| const headerId = useMemo(() => crypto.randomUUID(), []); | ||
| const indexRef = useRefFrom(index); | ||
| const onLeaveRef = useRefFrom(onLeave); | ||
| const onRequestFocusRef = useRefFrom(onRequestFocus); | ||
|
|
||
| const isFocused = activeMode === 'focus'; | ||
| const wasFocused = usePrevious(isFocused); | ||
| const becomingFocused = !wasFocused && isFocused; | ||
|
|
||
| const isFocusedRef = useRefFrom(isFocused); | ||
|
|
||
| const handleKeyDown = useCallback( | ||
| event => { | ||
| if (isFocusedRef.current) { | ||
| event.stopPropagation(); | ||
| } | ||
|
|
||
| if (event.key === 'Escape') { | ||
| event.stopPropagation(); | ||
|
|
||
| onLeaveRef.current?.(); | ||
| } | ||
| }, | ||
| [onLeaveRef] | ||
| ); | ||
|
|
||
| useEffect(() => { | ||
| if (becomingFocused) { | ||
| bodyRef.current?.focus(); | ||
| } | ||
| }, [becomingFocused]); | ||
|
|
||
| const handleHeaderClick = useCallback( | ||
| event => onRequestFocusRef.current?.(indexRef.current), | ||
| [indexRef, onRequestFocusRef] | ||
| ); | ||
|
|
||
| return ( | ||
| <article // Required: children of role="feed" must be role="article". | ||
| aria-labelledby={headerId} // Required: we just want screen reader to narrate header. Without this, it will narrate the whole content. | ||
| className={cx('chat-message', { 'chat-message__is-active': activeMode === 'active' })} | ||
| id={`chat-message__index-${index}`} | ||
| > | ||
| <h4 | ||
| className="chat-message__header" | ||
| id={headerId} | ||
| // Narrator UX: in scan mode, when user press ENTER, we will get onClick and we can use it to focus into the message. | ||
| // However, this item should be hidden as we want to prevent mouse clicks. | ||
| onClick={handleHeaderClick} | ||
| > | ||
| {abstract} | ||
| </h4> | ||
| <div | ||
| aria-labelledby={contentId} // Narrator quirks: without aria-labelledby, after pressing ENTER and focus on this element, Narrator will say nothing. | ||
| className="chat-message__body" | ||
| inert={!isFocused} // Required: if the element is not focused, its contents should not be tabbable. | ||
| onKeyDown={handleKeyDown} | ||
| ref={bodyRef} | ||
| role={isFocused ? 'document' : undefined} // Required: as instructed by C+AI accessibility team: after pressing ENTER, add role="document" and focus on the element, screen reader should change to scan/browse mode. | ||
| tabindex={isFocused ? -1 : undefined} // Required: as instructed by C+AI accessibility team: after pressing ENTER, add role="document" and focus on the element, screen reader should change to scan/browse mode. | ||
| > | ||
| <div className="chat-message__content" id={contentId}> | ||
| <FocusTrap onLeave={onLeave}>{children}</FocusTrap> | ||
| </div> | ||
| </div> | ||
| </article> | ||
| ); | ||
| } | ||
|
|
||
| function ChatHistory({ onLeave }) { | ||
| const [activeMessageIndex, setActiveMessageIndex] = useState(0); | ||
| const [isFocused, setIsFocused, isFocusedRef] = useStateWithRef(false); | ||
| const onLeaveRef = useRefFrom(onLeave); | ||
| const rootRef = useRef(); | ||
|
|
||
| const handleKeyDown = useCallback( | ||
| event => { | ||
| if (event.key === 'ArrowUp') { | ||
| event.stopPropagation(); | ||
|
|
||
| setActiveMessageIndex(index => Math.max(0, index - 1)); | ||
| } else if (event.key === 'ArrowDown') { | ||
| event.stopPropagation(); | ||
|
|
||
| setActiveMessageIndex(index => Math.min(1, index + 1)); | ||
| } else if (event.key === 'Enter') { | ||
| event.stopPropagation(); | ||
|
|
||
| setIsFocused(true); | ||
| } else if (event.key === 'Escape') { | ||
| // We like this, when pressing ESCAPE key on chat history, send the focus to send box. | ||
| setIsFocused(false); | ||
|
|
||
| onLeaveRef.current?.(); | ||
| } | ||
| }, | ||
| [isFocusedRef, onLeaveRef, setActiveMessageIndex, setIsFocused] | ||
| ); | ||
|
|
||
| const handleMessageLeave = useCallback(() => { | ||
| rootRef.current?.focus(); | ||
|
|
||
| setIsFocused(false); | ||
| }, [rootRef, setIsFocused]); | ||
|
|
||
| const handleMessageRequestFocus = useCallback( | ||
| index => { | ||
| setActiveMessageIndex(index); | ||
| setIsFocused(true); | ||
| }, | ||
| [setActiveMessageIndex, setIsFocused] | ||
| ); | ||
|
|
||
| return ( | ||
| <div | ||
| aria-activedescendant={`chat-message__index-${activeMessageIndex}`} // Matter of taste: we are using active descendant to control focus, instead of roving tab index. | ||
| className="chat-history" | ||
| onKeyDown={handleKeyDown} | ||
| ref={rootRef} | ||
| role="group" // Required: aria-activedescendant is only available for role="group". | ||
| tabindex="0" // Required: container of the active descendant must be focusable. | ||
| > | ||
| <section | ||
| className="chat-history__body" | ||
| role="feed" // Required: we are using role="feed/article" to represent the chat thread. | ||
| > | ||
| <ChatMessage | ||
| abstract="Bot said: Hello, World!" // Matter of taste on how to abstract the text: this is for screen reader user pressing H key to quickly jump between messages. | ||
| activeMode={activeMessageIndex === 0 ? (isFocused ? 'focus' : 'active') : undefined} | ||
| index={0} | ||
| onLeave={activeMessageIndex === 0 ? handleMessageLeave : undefined} | ||
| onRequestFocus={handleMessageRequestFocus} | ||
| > | ||
| <p>Hello, World!</p> | ||
| <p> | ||
| Click <a href="https://bing.com/">this link</a> for more details. | ||
| </p> | ||
| </ChatMessage> | ||
| <ChatMessage | ||
| abstract="You said: Aloha!" | ||
| activeMode={activeMessageIndex === 1 ? (isFocused ? 'focus' : 'active') : undefined} | ||
| index={1} | ||
| onLeave={activeMessageIndex === 1 ? handleMessageLeave : undefined} | ||
| onRequestFocus={handleMessageRequestFocus} | ||
| > | ||
| <p>Aloha!</p> | ||
| </ChatMessage> | ||
| </section> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| const SendBox = forwardRef(function SendBox(_, ref) { | ||
| const handleSubmit = useCallback(event => { | ||
| event.preventDefault(); | ||
| }, []); | ||
|
|
||
| return ( | ||
| <form className="send-box" onSubmit={handleSubmit}> | ||
| <textarea className="send-box__text-box" placeholder="Type a message" ref={ref} /> | ||
| </form> | ||
| ); | ||
| }); | ||
|
|
||
| function ChatApp() { | ||
| const sendBoxRef = useRef(); | ||
|
|
||
| const handleChatHistoryLeave = useCallback(() => { | ||
| sendBoxRef.current?.focus(); | ||
| }, [sendBoxRef]); | ||
|
|
||
| return ( | ||
| <div | ||
| className="chat-app" | ||
| role="application" // Required: role="document" will only work when its container has role="application". | ||
| > | ||
| <ChatHistory onLeave={handleChatHistoryLeave} /> | ||
| <SendBox ref={sendBoxRef} /> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| const mainElement = document.querySelector('main'); | ||
|
|
||
| mainElement && createRoot(mainElement).render(<ChatApp />); | ||
|
|
||
| setTimeout(() => { | ||
| document.querySelector('textarea')?.focus(); | ||
| }, 100); | ||
| </script> | ||
| </body> | ||
| </html> | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if markdown contains headings? Or this is negotiated by using document role?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can tune the Markdown headings but TBH not sure what is the best
<h*>to use.<h1>is usually reserved for page title, so we could start with<h2>and have Markdown/contents starts with<h3>. What do you think?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm okay. I don't like adjusting authored content like so, but seems we don't have a better way 🤣