fix(various): IFD accessibility call-outs#595
Conversation
|
Preview: https://chatbot-pr-chatbot-595.surge.sh A11y report: https://chatbot-pr-chatbot-595-a11y.surge.sh |
packages/module/patternfly-docs/content/extensions/chatbot/examples/UI/ChatbotHeaderDrawer.tsx
Show resolved
Hide resolved
rebeccaalpert
force-pushed
the
a11y-ifd
branch
3 times, most recently
from
8c01ced to
93e8e7c
Compare
|
Surge preview here since PR was pre-token-refresh: http://enchanted-basil.surge.sh/ |
thatblindgeye
left a comment
thatblindgeye
left a comment
There was a problem hiding this comment.
Also for the items from the issue:
- "Close button activation should not disrupt expected focus order" - not sure if I'm noticing this issue, but if we aren't explicitly handling focus in examples (or component code) to move focus from the convo history drawer close button back to the convo history menu toggle, we should do that
- "“New chat” button should not disrupt expected focus order" - same as above here + just some documentation that what to focus upon clicking "new chat" may depend, i.e. maybe the expectation is that focus goes to the chat text input so you can quickly start typing without having to Tab around
Was a little confused by these two, though, as it seems like sometimes focus does go where I'd expect, sometimes not.
packages/module/patternfly-docs/content/extensions/chatbot/examples/UI/ChatbotHeaderDrawer.tsx
Show resolved
Hide resolved
packages/module/src/ChatbotConversationHistoryNav/ChatbotConversationHistoryNav.tsx
Outdated
Show resolved
Hide resolved
| {...drawerPanelContentProps} | ||
| > | ||
| {announcement && ( | ||
| <div className="pf-chatbot__filter-announcement" aria-live="polite"> |
There was a problem hiding this comment.
We should consider removing either the arialive on this new div, or on DrawerPanelContent.
We should also move this div to come after the search input in the DOM. Rihgt now the traversal is a bit weird because I can have announced this live region text from VO before I even get to the search input, and the context of this announcement would be more beneficial immediately after the input.
| totalCount === 1 | ||
| ? `${totalCount} conversation matches "${value}"` | ||
| : `${totalCount} conversations match "${value}"`; | ||
| setAnnouncement(newAnnouncement); |
There was a problem hiding this comment.
More of a nit since it's example code, but it might be good to maybe have a debounce when setting this announcement text. Granted it may be due to the dual live regions in the component code as well, but when trying to search "red" in the example, VO stops to tell me the results for just "r"
There was a problem hiding this comment.
I gave this a go! Let me know what you think.
packages/module/src/ChatbotConversationHistoryNav/ChatbotConversationHistoryNav.tsx
Outdated
Show resolved
Hide resolved
…ous a11y fixes Adjusted default text to match visible text and provide more context. Jump buttons now also allow more customization via prop passage. The history drawer can also make announcements when content changes.
|
Pushed PR feedback to http://enchanted-basil.surge.sh/. |
|
🎉 This PR is included in version 6.4.0-prerelease.10 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Adjusted default aria-labels on history button, search box label, jump buttons. Jump buttons now also allow more customization via prop passage. The history drawer can also make announcements when content changes (demo included).
3 participants
This hits some of the accessibility issues flagged by IFD. Some don't apply to us and a couple (jump link behavior) would require conversation with design I think, since the intended behavior is to have them disappear at bottom and top respectively. You can still access both via the rotor if visible even if they aren't easy to tab to.