Dropdown menus - option selection : add aria-hidden on icons #1036
+82
−35
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
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 |
|---|---|---|
|
|
@@ -45,7 +45,9 @@ test.describe('Doc Header', () => { | |
|
|
||
| await expect(card.getByText('Owner ·')).toBeVisible(); | ||
| await expect(page.getByRole('button', { name: 'Share' })).toBeVisible(); | ||
| await expect( | ||
| page.getByRole('button', { name: 'Export the document' }), | ||
| ).toBeVisible(); | ||
| await expect( | ||
| page.getByRole('button', { name: 'Open the document options' }), | ||
| ).toBeVisible(); | ||
|
|
@@ -116,7 +118,9 @@ test.describe('Doc Header', () => { | |
|
|
||
| await goToGridDoc(page); | ||
|
|
||
| await expect( | ||
| page.getByRole('button', { name: 'Export the document' }), | ||
| ).toBeVisible(); | ||
|
|
||
| await page.getByLabel('Open the document options').click(); | ||
|
|
||
|
|
@@ -139,7 +143,9 @@ test.describe('Doc Header', () => { | |
| const invitationRole = invitationCard.getByLabel('doc-role-dropdown'); | ||
| await expect(invitationRole).toBeVisible(); | ||
|
|
||
| await invitationCard | ||
| .getByRole('button', { name: 'Open the invitation options' }) | ||
| .click(); | ||
|
|
||
| await page.getByRole('menuitem', { name: 'Remove access' }).click(); | ||
| await expect(invitationCard).toBeHidden(); | ||
|
|
@@ -154,8 +160,13 @@ test.describe('Doc Header', () => { | |
|
|
||
| await roles.click(); | ||
| await expect( | ||
| memberCard.getByRole('button', { name: 'Open the member options' }), | ||
| ).toBeVisible(); | ||
| await memberCard | ||
| .getByRole('button', { name: 'Open the member options' }) | ||
| .click(); | ||
|
|
||
| await expect(page.getByLabel('Delete')).toBeEnabled(); | ||
| }); | ||
|
|
||
| test('it checks the options available if editor', async ({ page }) => { | ||
|
|
@@ -186,7 +197,9 @@ test.describe('Doc Header', () => { | |
|
|
||
| await goToGridDoc(page); | ||
|
|
||
| await expect( | ||
| page.getByRole('button', { name: 'Export the document' }), | ||
| ).toBeVisible(); | ||
| await page.getByLabel('Open the document options').click(); | ||
|
|
||
| await expect(page.getByLabel('Delete document')).toBeDisabled(); | ||
|
|
@@ -207,14 +220,16 @@ test.describe('Doc Header', () => { | |
| ).toBeVisible(); | ||
| await expect(invitationCard.getByLabel('doc-role-text')).toBeVisible(); | ||
| await expect( | ||
| invitationCard.getByRole('button', { | ||
| name: 'Open the invitation options', | ||
| }), | ||
| ).toBeHidden(); | ||
|
|
||
| const memberCard = shareModal.getByLabel('List members card'); | ||
| await expect(memberCard.getByText('[email protected]')).toBeVisible(); | ||
| await expect(memberCard.getByLabel('doc-role-text')).toBeVisible(); | ||
| await expect( | ||
| memberCard.getByRole('button', { name: 'Open the member options' }), | ||
|
There was a problem hiding this comment. Same as above, extract into a variable if you'd like. |
||
| ).toBeHidden(); | ||
| }); | ||
|
|
||
|
|
@@ -246,7 +261,9 @@ test.describe('Doc Header', () => { | |
|
|
||
| await goToGridDoc(page); | ||
|
|
||
| await expect( | ||
| page.getByRole('button', { name: 'Export the document' }), | ||
| ).toBeVisible(); | ||
| await page.getByLabel('Open the document options').click(); | ||
|
|
||
| await expect(page.getByLabel('Delete document')).toBeDisabled(); | ||
|
|
@@ -267,14 +284,16 @@ test.describe('Doc Header', () => { | |
| ).toBeVisible(); | ||
| await expect(invitationCard.getByLabel('doc-role-text')).toBeVisible(); | ||
| await expect( | ||
| invitationCard.getByRole('button', { | ||
| name: 'Open the invitation options', | ||
| }), | ||
| ).toBeHidden(); | ||
|
|
||
| const memberCard = shareModal.getByLabel('List members card'); | ||
| await expect(memberCard.getByText('[email protected]')).toBeVisible(); | ||
| await expect(memberCard.getByLabel('doc-role-text')).toBeVisible(); | ||
| await expect( | ||
| memberCard.getByRole('button', { name: 'Open the member options' }), | ||
| ).toBeHidden(); | ||
| }); | ||
|
|
||
|
|
||
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 |
|---|---|---|
|
|
@@ -184,9 +184,6 @@ test.describe('Document list members', () => { | |
|
|
||
| const emailMyself = `user@${browserName}.test`; | ||
| const mySelf = list.getByTestId(`doc-share-member-row-${emailMyself}`); | ||
|
|
||
| const userOwnerEmail = await addNewMember(page, 0, 'Owner'); | ||
| const userOwner = list.getByTestId( | ||
|
|
@@ -201,20 +198,31 @@ test.describe('Document list members', () => { | |
| const userReader = list.getByTestId( | ||
| `doc-share-member-row-${userReaderEmail}`, | ||
| ); | ||
|
|
||
| await expect(mySelf).toBeVisible(); | ||
| await expect(userOwner).toBeVisible(); | ||
| await expect(userReader).toBeVisible(); | ||
|
|
||
| await expect( | ||
| userOwner.getByRole('button', { | ||
| name: 'Open the member options', | ||
| }), | ||
| ).toBeVisible(); | ||
|
|
||
| await userReader | ||
| .getByRole('button', { | ||
| name: 'Open the member options', | ||
| }) | ||
| .click(); | ||
| await page.getByLabel('Delete').click(); | ||
| await expect(userReader).toBeHidden(); | ||
|
|
||
| await mySelf | ||
| .getByRole('button', { | ||
| name: 'Open the member options', | ||
| }) | ||
| .click(); | ||
| await page.getByLabel('Delete').click(); | ||
| await expect( | ||
| page.getByText('Insufficient access rights to view the document.'), | ||
| ).toBeVisible(); | ||
|
|
||
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 |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import { HorizontalSeparator } from '@gouvfr-lasuite/ui-kit'; | ||
| import { Fragment, PropsWithChildren, useRef, useState } from 'react'; | ||
| import { useTranslation } from 'react-i18next'; | ||
| import { css } from 'styled-components'; | ||
|
|
||
| import { Box, BoxButton, BoxProps, DropButton, Icon, Text } from '@/components'; | ||
|
|
@@ -45,6 +46,7 @@ export const DropdownMenu = ({ | |
| const { spacingsTokens, colorsTokens } = useCunninghamTheme(); | ||
| const [isOpen, setIsOpen] = useState(false); | ||
| const blockButtonRef = useRef<HTMLDivElement>(null); | ||
| const { t } = useTranslation(); | ||
|
|
||
| const onOpenChange = (isOpen: boolean) => { | ||
| setIsOpen(isOpen); | ||
|
|
@@ -162,6 +164,7 @@ export const DropdownMenu = ({ | |
| $direction="row" | ||
| $align="center" | ||
| $gap={spacingsTokens['base']} | ||
| aria-selected={option.isSelected ? true : undefined} | ||
| > | ||
| {option.icon && ( | ||
| <Icon | ||
|
|
@@ -177,7 +180,14 @@ export const DropdownMenu = ({ | |
| </Box> | ||
| {(option.isSelected || | ||
| selectedValues?.includes(option.value ?? '')) && ( | ||
| <Icon | ||
| role="img" | ||
| iconName="check" | ||
| $size="20px" | ||
| $theme="greyscale" | ||
| aria-label={t('Checked')} | ||
| aria-hidden="false" | ||
| /> | ||
| )} | ||
| </BoxButton> | ||
| {option.showSeparator && ( | ||
|
|
||
|
There was a problem hiding this comment. Just a quick note: It might be cleaner to accept explicit Happy to discuss it further if neededfeel free to reach out! I’ll suggest a snippet below feel free to use it or adapt it as you see fit. |
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 | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -14,6 +14,7 @@ export const Icon = ({ | |||||||||||||||||||
| }: IconProps) => { | ||||||||||||||||||||
|
There was a problem hiding this comment. Make the in the IconProps Type :
Suggested change
There was a problem hiding this comment.
Suggested change
|
||||||||||||||||||||
| return ( | ||||||||||||||||||||
| <Text | ||||||||||||||||||||
| aria-hidden={!!!textProps['aria-label']} | ||||||||||||||||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||||||||||||||||
| {...textProps} | ||||||||||||||||||||
| className={clsx('--docs--icon-bg', textProps.className, { | ||||||||||||||||||||
| 'material-icons-filled': variant === 'filled', | ||||||||||||||||||||
|
|
||||||||||||||||||||
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 |
|---|---|---|
|
|
@@ -252,9 +252,10 @@ export const DocToolBox = ({ doc }: DocToolBoxProps) => { | |
| setIsModalExportOpen(true); | ||
| }} | ||
| size={isSmallMobile ? 'small' : 'medium'} | ||
| aria-label={t('Export the document')} | ||
| /> | ||
| )} | ||
| <DropdownMenu options={options} label={t('Open the document options')}> | ||
| <IconOptions | ||
| isHorizontal | ||
| $theme="primary" | ||
|
There was a problem hiding this comment. Since the I'd suggest removing it and letting the menu handle accessibility. |
||
|
|
||
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,7 +2,7 @@ import { Button } from '@openfun/cunningham-react'; | |||||
| import { useTranslation } from 'react-i18next'; | ||||||
| import { css } from 'styled-components'; | ||||||
|
|
||||||
| import { Box, HorizontalSeparator, Icon } from '@/components'; | ||||||
| import { Doc, useCopyDocLink } from '@/docs/doc-management'; | ||||||
|
|
||||||
| import { DocVisibility } from './DocVisibility'; | ||||||
|
|
@@ -41,7 +41,7 @@ export const DocShareModalFooter = ({ | |||||
| fullWidth={false} | ||||||
| onClick={copyDocLink} | ||||||
| color="tertiary" | ||||||
| icon={<Icon iconName="add_link" />} | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| > | ||||||
| {t('Copy link')} | ||||||
| </Button> | ||||||
|
|
||||||
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 |
|---|---|---|
|
|
@@ -5,7 +5,7 @@ import { | |
| } from '@openfun/cunningham-react'; | ||
| import { useTranslation } from 'react-i18next'; | ||
|
|
||
| import { DropdownMenu, DropdownMenuOption, IconOptions } from '@/components'; | ||
| import { | ||
| Doc, | ||
| KEY_LIST_DOC, | ||
|
|
@@ -93,10 +93,10 @@ export const DocsGridActions = ({ | |
|
|
||
| return ( | ||
| <> | ||
| <DropdownMenu options={options} label={t('Open the document options')}> | ||
| <IconOptions | ||
| isHorizontal | ||
| data-testid={`docs-grid-actions-button-${doc.id}`} | ||
| $theme="primary" | ||
| $variation="600" | ||
| /> | ||
|
|
||
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 |
|---|---|---|
|
|
@@ -61,6 +61,7 @@ | |
| "Beautify": "Verschönern", | ||
| "Can't load this page, please check your internet connection.": "Diese Seite kann nicht geladen werden. Bitte überprüfen Sie Ihre Internetverbindung.", | ||
| "Cancel": "Abbrechen", | ||
| "Checked": "Angehakt", | ||
| "Close the modal": "Pop up schliessen", | ||
| "Collaborate": "Zusammenarbeiten", | ||
| "Collaborate and write in real time, without layout constraints.": "In Echtzeit und ohne Layout-Beschränkungen schreiben und zusammenarbeiten.", | ||
|
|
@@ -247,6 +248,7 @@ | |
| "Callout": "Destacado", | ||
| "Can't load this page, please check your internet connection.": "No se puede cargar esta página, por favor compruebe su conexión a Internet.", | ||
| "Cancel": "Cancelar", | ||
| "Checked": "Marcado", | ||
| "Close the modal": "Cerrar modal", | ||
| "Collaborate": "Colabora", | ||
| "Collaborate and write in real time, without layout constraints.": "Colaborar y escribir en tiempo real, sin restricciones de diseño.", | ||
|
|
@@ -433,6 +435,7 @@ | |
| "Callout": "Alerte", | ||
| "Can't load this page, please check your internet connection.": "Impossible de charger cette page, veuillez vérifier votre connexion Internet.", | ||
| "Cancel": "Annuler", | ||
| "Checked": "Coché", | ||
| "Close the modal": "Fermer la modale", | ||
| "Collaborate": "Collaborer", | ||
| "Collaborate and write in real time, without layout constraints.": "Collaborez et rédigez en temps réel, sans contrainte de mise en page.", | ||
|
|
@@ -648,6 +651,7 @@ | |
| "Beautify": "Filtro bellezza", | ||
| "Can't load this page, please check your internet connection.": "Impossibile caricare questa pagina, controlla la tua connessione internet.", | ||
| "Cancel": "Cancella", | ||
| "Checked": "Selezionato", | ||
| "Collaborate": "Collabora", | ||
| "Collaborate and write in real time, without layout constraints.": "Collaborare e scrivere in tempo reale, senza vincoli di layout.", | ||
| "Collaborative writing, Simplified.": "Scrittura collaborativa, semplificata.", | ||
|
|
@@ -798,6 +802,7 @@ | |
| "Beautify": "Maak mooier", | ||
| "Can't load this page, please check your internet connection.": "Kan deze pagina niet laden. Controleer je internetverbinding.", | ||
| "Cancel": "Breek af", | ||
| "Checked": "Aangevinkt", | ||
| "Close the modal": "Sluit het venster", | ||
| "Collaborate": "Samenwerken", | ||
| "Collaborate and write in real time, without layout constraints.": "Samenwerken en schrijven in realtime, zonder lay-out beperkingen.", | ||
|
|
@@ -963,6 +968,7 @@ | |
| "Anonymous": "Anonym", | ||
| "Beautify": "Försköna", | ||
| "Cancel": "Avbryt", | ||
| "Checked": "Markerad", | ||
| "Close the modal": "Stäng fönstret", | ||
| "Convert Markdown": "Konvertera Markdown", | ||
| "Correct": "Korrekt", | ||
|
|
@@ -999,6 +1005,7 @@ | |
| "Banner image": "Afiş görseli", | ||
| "Beautify": "Güzelleştir", | ||
| "Cancel": "İptal", | ||
| "Checked": "İşaretli", | ||
| "Collaborate": "İşbirliği Yapın", | ||
| "Collaborate and write in real time, without layout constraints.": "Düzen kısıtlamaları olmadan gerçek zamanlı olarak işbirliği yapın ve yazın.", | ||
| "Collaborative writing, Simplified.": "İş birliğiyle yazmak çok daha Kolay.", | ||
|
|
@@ -1086,6 +1093,7 @@ | |
| "Callout": "标注", | ||
| "Can't load this page, please check your internet connection.": "无法加载此页面，请检查您的网络连接。", | ||
| "Cancel": "取消", | ||
| "Checked": "已勾选", | ||
| "Close the modal": "关闭对话框", | ||
| "Collaborate": "协作", | ||
| "Collaborate and write in real time, without layout constraints.": "实时协作和写入，不受布局限制。", | ||
|
|
||
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.
Totally optional: you could create a variable like
const exportButtonLabel = 'Export the document'to avoid repeating the string and make future changes easier.
Not a blocker at all, just a small DX improvement suggestion :)