-
Notifications
You must be signed in to change notification settings - Fork 464
♿(frontend) improve screen reader support in DocShare modal with aria… #1628
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -208,7 +208,7 @@ test.describe('Doc Header', () => { | |
| await expect( | ||
| invitationCard.getByText('[email protected]').first(), | ||
| ).toBeVisible(); | ||
| const invitationRole = invitationCard.getByLabel('doc-role-dropdown'); | ||
| const invitationRole = invitationCard.getByTestId('doc-role-dropdown'); | ||
| await expect(invitationRole).toBeVisible(); | ||
|
|
||
| await invitationRole.click(); | ||
|
|
@@ -217,7 +217,7 @@ test.describe('Doc Header', () => { | |
| await expect(invitationCard).toBeHidden(); | ||
|
|
||
| const memberCard = shareModal.getByLabel('List members card'); | ||
| const roles = memberCard.getByLabel('doc-role-dropdown'); | ||
| const roles = memberCard.getByTestId('doc-role-dropdown'); | ||
| await expect(memberCard).toBeVisible(); | ||
| await expect( | ||
| memberCard.getByText('[email protected]').first(), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -76,6 +76,7 @@ export const DocShareModal = ({ doc, onClose, isRootDoc = true }: Props) => { | |
| const [selectedUsers, setSelectedUsers] = useState<User[]>([]); | ||
| const [userQuery, setUserQuery] = useState(''); | ||
| const [inputValue, setInputValue] = useState(''); | ||
| const [liveAnnouncement, setLiveAnnouncement] = useState(''); | ||
|
|
||
| const [listHeight, setListHeight] = useState<string>('400px'); | ||
| const canShare = doc.abilities.accesses_manage && isRootDoc; | ||
|
|
@@ -88,6 +89,19 @@ export const DocShareModal = ({ doc, onClose, isRootDoc = true }: Props) => { | |
| setSelectedUsers((prev) => [...prev, user]); | ||
| setUserQuery(''); | ||
| setInputValue(''); | ||
|
|
||
| // Announce to screen readers | ||
| const userName = user.full_name || user.email; | ||
| setLiveAnnouncement( | ||
| t( | ||
| '{{name}} added to invite list. Add more members or press Tab to select role and invite.', | ||
| { | ||
| name: userName, | ||
| }, | ||
| ), | ||
| ); | ||
| // Clear announcement after it's been read | ||
| setTimeout(() => setLiveAnnouncement(''), 100); | ||
| }; | ||
|
Comment on lines
+93
to
105
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I find a bit strange to do the announcement, we have a notification system, but I think we decided to not display a notification for that, because the invitation list will display the user that you just add, it is already like a announcement. |
||
|
|
||
| const { data: membersQuery } = useDocAccesses({ | ||
|
|
@@ -114,6 +128,16 @@ export const DocShareModal = ({ doc, onClose, isRootDoc = true }: Props) => { | |
| } | ||
| const newArray = [...prevState]; | ||
| newArray.splice(index, 1); | ||
|
|
||
| // Announce to screen readers | ||
| const userName = row.full_name || row.email; | ||
| setLiveAnnouncement( | ||
| t('{{name}} removed from invite list', { | ||
| name: userName, | ||
| }), | ||
| ); | ||
| setTimeout(() => setLiveAnnouncement(''), 100); | ||
|
|
||
| return newArray; | ||
| }); | ||
| }; | ||
|
|
@@ -175,12 +199,29 @@ export const DocShareModal = ({ doc, onClose, isRootDoc = true }: Props) => { | |
| <ButtonCloseModal | ||
| aria-label={t('Close the share modal')} | ||
| onClick={onClose} | ||
| tabIndex={-1} | ||
| /> | ||
| </Box> | ||
| } | ||
| hideCloseButton | ||
| > | ||
| <ShareModalStyle /> | ||
| {/* Screen reader announcements */} | ||
| <div | ||
| role="status" | ||
| aria-live="polite" | ||
| aria-atomic="true" | ||
| className="sr-only" | ||
| style={{ | ||
| position: 'absolute', | ||
| left: '-10000px', | ||
| width: '1px', | ||
| height: '1px', | ||
| overflow: 'hidden', | ||
| }} | ||
| > | ||
|
Comment on lines
+215
to
+222
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The style attach with |
||
| {liveAnnouncement} | ||
| </div> | ||
| <Box | ||
| $height="auto" | ||
| $maxHeight={canViewAccesses ? modalContentHeight : 'none'} | ||
|
|
||
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.
During rebase, this part will have to go under
Unrelease