-
Notifications
You must be signed in to change notification settings - Fork 72
[terra-modal manager] Implemented focus lock to confine focus within the modal popup. #2014
Conversation
… AH106586-modal-manager-focus-lock
This reverts commit afc75c2.
…m/cerner/terra-framework into AH106586-modal-manager-focus-lock
We had added focus trap to this element and UX had us remove it. @scottwilmarth Can you chime in with why we don't want focus trap in modal manager? |
… AH106586-modal-manager-focus-lock
As suggested in the PR, the user focus when navigating using the keyboard is confined inside the modal dialog and the focus is to be navigated in cyclic order within the dialog. However, the issue is that tabindex="0" added to the modal dialog container, causing it to receive focus every time the user navigates within the modal dialog in cyclic order. It is recommended to add tabindex="-1" to the modal dialog container , to set the initial focus on the modal dialog container when it is opened.
|
… AH106586-modal-manager-focus-lock
Implemented. Thanks. |
const modalContent = ( | ||
<div | ||
{...customProps} | ||
tabIndex={platformIsiOS || isCalledFromNotificationDialog ? '-1' : '0'} |
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.
Does container receive initial focus without tabIndex ?
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.
Yes, the primary focus is on the container.PFA
modal_popup.mov
{...customProps} | ||
aria-label={ariaLabel} | ||
aria-labelledby={ariaLabelledBy} | ||
aria-describedby={ariaDescribedBy} |
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 need to validate with screen readers to ensure values provided to above aria attributes are read when the focus is on modal container. Since there is no tabIndex on this div, it may get ignored by screen readers
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.
Hi @supreethmr , when the focus is on modal container all the aria attributes are working as expected. Please find the attached video for reference.
test.mov
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.
Please verify on JAWS as well
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.
In JAWS as well It is announcing all the aria attributes PFA video.
Screen.Recording.2024-02-15.at.2.53.45.PM.mov
+1 |
Summary
Fixed- Focus is continuing to go outside of the Modal Manager when navigating through the component.
What was changed:
Added FocusLock to confine the focus within the modal popup
Why it was changed:
Pressing the TAB key causes the focus to shift away from the modal popup.
Testing
This change was tested using:
Reviews
In addition to engineering reviews, this PR needs:
Additional Details
This PR resolves:
UXPLATFORM-10090
UXPLATFORM-10169
Thank you for contributing to Terra.
@cerner/terra