Toast focus management take 2 #6223
Conversation
|
Build successful! 🎉 |
|
Build successful! 🎉 |
|
Build successful! 🎉 |
|
Build successful! 🎉 |
|
Build successful! 🎉 |
|
Build successful! 🎉 |
|
@snowystinger With recent changes, focus is getting lost to the document.body when the top toast, the last item in the list gets removed and other toasts remain. |
|
|
||
| let titleId = useId(); | ||
| let descriptionId = useSlotId(); | ||
| let stringFormatter = useLocalizedStringFormatter(intlMessages, '@react-aria/toast'); | ||
|
|
||
| return { | ||
| toastProps: { | ||
| role: 'alert', | ||
| role: 'alertdialog', | ||
| 'aria-modal': 'false', |
There was a problem hiding this comment.
non-action, just interested that this is specifically set to false. I'm just used to omitting the attribute but I'm guessing this explicit false made a difference in the screen reader behavior?
There was a problem hiding this comment.
@majornista correct me if I'm wrong, but this is just to possibly future proof? since alertdialog would be considered a modal by default, and so we use aria-modal to communicate that it's not, in fact, a modal
There was a problem hiding this comment.
aria-modal="false" doesn't really make a difference for screen reader behavior; the default is for a dialog to be non-modal. However, the APG design pattern describes an alertdialog as modal, so I thought it best to be explicit:
An alert dialog is a modal dialog that interrupts the user's workflow to communicate an important message and acquire a response. Examples include action confirmation prompts and error message confirmations. The alertdialog role enables assistive technologies and browsers to distinguish alert dialogs from other dialogs so they have the option of giving alert dialogs special treatment, such as playing a system alert sound.
I use role="alertdialog", as a container for the toast message content, which has role="alert" to announce as a live region, and the button to dismiss the Toast, for a few reasons.
- A Toast is kind of a tiny non-modal dialog, so it didn't seem like much of a stretch.
- Using a dialog as a container helps disambiguate between the Dismiss buttons when more than one Toast is displayed.
- When we close a Toast, unlike the Dismiss button for an adjacent Toast, the dialog provides a uniquely labeled widget element to receive focus, and is easy to style using the React-Spectrum focus ring.
- Depending on the assistive technology being used, an alert dialog will open with an earcon, which I see as nice feature to distinguish a Toast from other content the screen reader may be announcing.
| @@ -39,6 +41,7 @@ export interface ToastAria { | |||
| * Provides the behavior and accessibility implementation for a toast component. | |||
| * Toasts display brief, temporary notifications of actions, errors, or other events in an application. | |||
| */ | |||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | |||
There was a problem hiding this comment.
Can we just remove the ref from the args even though it would be breaking? It is still in beta so might be ok?
There was a problem hiding this comment.
it's more of a, we've needed to add it back in later in other cases. I'd prefer to know we'll have access to it
| @@ -26,7 +26,7 @@ function Text(props: TextProps, ref: DOMRef) { | |||
| let domRef = useDOMRef(ref); | |||
|
|
|||
| return ( | |||
| <span {...filterDOMProps(otherProps)} {...styleProps} ref={domRef}> | |||
| <span role="none" {...filterDOMProps(otherProps)} {...styleProps} ref={domRef}> | |||
There was a problem hiding this comment.
I see this was added to prevent double announcements, but where do we use Text in toasts?
There was a problem hiding this comment.
Ah I see, the Button for the toast action. Will have to double check everywhere we use Text to see if there are any changes to the screen reader performance
There was a problem hiding this comment.
yeah, i haven't noticed anything yet
There was a problem hiding this comment.
Yeah, this was something I noticed in testing with VoiceOver. When a Toast included an action button, the button text was announced twice. This has to do with how live regions work, announcing both element "additions" and "text." VoiceOver seem to interpret the Text component's span element child of the Button as an addition, and announces the text more than once.
| if (removedToastIndex > -1) { | ||
| let i = 0; | ||
| let nextToast; | ||
| let prevToast; | ||
| while (i <= removedToastIndex) { | ||
| if (!removedToasts[i].isRemoved) { | ||
| prevToast = Math.max(0, i - 1); | ||
| } | ||
| i++; | ||
| } | ||
| while (i < removedToasts.length) { | ||
| if (!removedToasts[i].isRemoved) { | ||
| nextToast = i - 1; | ||
| break; | ||
| } | ||
| i++; | ||
| } |
There was a problem hiding this comment.
bit of a nit, but I found this a bit hard to follow, could we instead just slice the removedToasts list into two (one going from the 0 -> removedToastIndex and the other going from removedToastIndex -> end) and use findLastIndex with the same !removedToasts.isRemoved condition on the first slice to get the index of the prevToast and findIndex + the removedToastIndex on the second slice to get the nextToast index?
Thinking on this, it might just be the "removedToasts" naming that is tripping me up since that list contains all the toasts but just adds extra data to mark the removed ones.
There was a problem hiding this comment.
yeah, it was hard to write too
I've clarified with renames and some more comments
|
## API Changes
unknown top level export { type: 'any' } @react-aria/toastToastAria ToastAria {
closeButtonProps: AriaButtonProps
+ contentProps: DOMAttributes
descriptionProps: DOMAttributes
titleProps: DOMAttributes
toastProps: DOMAttributes
}it changed:
|
Closes
In association with #6011
There are still some known issues, they are listed in our GA ticket https://github.com/orgs/adobe/projects/19/views/4?pane=issue&itemId=55618129
✅ Pull Request Checklist:
📝 Test Instructions:
Test instructions in #6011 description
🧢 Your Project: