diff --git a/services/web/frontend/js/features/file-tree/components/modals/file-tree-modal-create-folder.js b/services/web/frontend/js/features/file-tree/components/modals/file-tree-modal-create-folder.js index 51e4f8df35..47eb776fa3 100644 --- a/services/web/frontend/js/features/file-tree/components/modals/file-tree-modal-create-folder.js +++ b/services/web/frontend/js/features/file-tree/components/modals/file-tree-modal-create-folder.js @@ -5,6 +5,8 @@ import { Button, Modal } from 'react-bootstrap' import { useTranslation } from 'react-i18next' import { useRefWithAutoFocus } from '../../../../infrastructure/auto-focus' +import AccessibleModal from '../../../../shared/components/accessible-modal' + import { useFileTreeActionable } from '../../contexts/file-tree-actionable' import { DuplicateFilenameError } from '../../errors' @@ -44,7 +46,7 @@ function FileTreeModalCreateFolder() { } return ( - + {t('new_folder')} @@ -95,7 +97,7 @@ function FileTreeModalCreateFolder() { )} - + ) } diff --git a/services/web/frontend/js/features/file-tree/components/modals/file-tree-modal-delete.js b/services/web/frontend/js/features/file-tree/components/modals/file-tree-modal-delete.js index 6d86af7028..4b807de94a 100644 --- a/services/web/frontend/js/features/file-tree/components/modals/file-tree-modal-delete.js +++ b/services/web/frontend/js/features/file-tree/components/modals/file-tree-modal-delete.js @@ -2,6 +2,8 @@ import React from 'react' import { Button, Modal } from 'react-bootstrap' import { useTranslation } from 'react-i18next' +import AccessibleModal from '../../../../shared/components/accessible-modal' + import { useFileTreeActionable } from '../../contexts/file-tree-actionable' function FileTreeModalDelete() { @@ -27,7 +29,7 @@ function FileTreeModalDelete() { } return ( - + {t('delete')} @@ -60,7 +62,7 @@ function FileTreeModalDelete() { )} - + ) } diff --git a/services/web/frontend/js/shared/components/accessible-modal.js b/services/web/frontend/js/shared/components/accessible-modal.js new file mode 100644 index 0000000000..52a46b3885 --- /dev/null +++ b/services/web/frontend/js/shared/components/accessible-modal.js @@ -0,0 +1,36 @@ +import React, { useCallback } from 'react' +import PropTypes from 'prop-types' +import { Modal } from 'react-bootstrap' + +// a bootstrap Modal with its `aria-hidden` attribute removed. Visisble modals +// should not have their `aria-hidden` attribute set but that's a bug in our +// version of react-bootstrap. +function AccessibleModal({ show, ...otherProps }) { + // use a callback ref to track the modal. This will re-run the function + // when the element node or any of the dependencies are updated + const setModalRef = useCallback( + element => { + if (!element) return + + const modalNode = element._modal && element._modal.modalNode + if (!modalNode) return + + if (show) { + modalNode.removeAttribute('aria-hidden') + } else { + modalNode.setAttribute('aria-hidden', 'true') + } + }, + // `show` is necessary as a dependency, but eslint thinks it is not + // eslint-disable-next-line react-hooks/exhaustive-deps + [show] + ) + + return +} + +AccessibleModal.propTypes = { + show: PropTypes.bool +} + +export default AccessibleModal diff --git a/services/web/test/frontend/features/file-tree/flows/create-folder.test.js b/services/web/test/frontend/features/file-tree/flows/create-folder.test.js index 1a78fbb5e1..d37a016689 100644 --- a/services/web/test/frontend/features/file-tree/flows/create-folder.test.js +++ b/services/web/test/frontend/features/file-tree/flows/create-folder.test.js @@ -1,7 +1,7 @@ import { expect } from 'chai' import React from 'react' import sinon from 'sinon' -import { screen, render, fireEvent } from '@testing-library/react' +import { screen, render, fireEvent, waitFor } from '@testing-library/react' import fetchMock from 'fetch-mock' import MockedSocket from 'socket.io-mock' @@ -56,7 +56,7 @@ describe('FileTree Create Folder Flow', function() { } fetchMock.post(matcher, response) - fireCreateFolder(newFolderName) + await fireCreateFolder(newFolderName) const lastCallBody = JSON.parse(fetchMock.lastCall(matcher)[1].body) expect(lastCallBody.name).to.equal(newFolderName) @@ -114,7 +114,7 @@ describe('FileTree Create Folder Flow', function() { } fetchMock.post(matcher, response) - fireCreateFolder(newFolderName) + await fireCreateFolder(newFolderName) const lastCallBody = JSON.parse(fetchMock.lastCall(matcher)[1].body) expect(lastCallBody.name).to.equal(newFolderName) @@ -178,7 +178,7 @@ describe('FileTree Create Folder Flow', function() { } fetchMock.post(matcher, response) - fireCreateFolder(newFolderName) + await fireCreateFolder(newFolderName) const lastCallBody = JSON.parse(fetchMock.lastCall(matcher)[1].body) expect(lastCallBody.name).to.equal(newFolderName) @@ -239,7 +239,7 @@ describe('FileTree Create Folder Flow', function() { }) }) - function fireCreateFolder(name) { + async function fireCreateFolder(name) { const createFolderButton = screen.getByRole('button', { name: 'New Folder' }) @@ -247,14 +247,12 @@ describe('FileTree Create Folder Flow', function() { setFolderName(name) - const modalCreateButton = getModalCreateButton() + const modalCreateButton = await getModalCreateButton() fireEvent.click(modalCreateButton) } function setFolderName(name) { - const input = screen.getByRole('textbox', { - hidden: true // FIXME: modal should not be hidden but it has the aria-hidden label due to a react-bootstrap bug - }) + const input = screen.getByRole('textbox') fireEvent.change(input, { target: { value: name } }) } @@ -264,10 +262,7 @@ describe('FileTree Create Folder Flow', function() { .replace(/0\./, 'random-test-id-') } - function getModalCreateButton() { - return screen.getAllByRole('button', { - name: 'Create', - hidden: true // FIXME: modal should not be hidden but it has the aria-hidden label due to a react-bootstrap bug - })[0] // the first matched button is the toolbar button + async function getModalCreateButton() { + return waitFor(() => screen.getByRole('button', { name: 'Create' })) } }) diff --git a/services/web/test/frontend/features/file-tree/flows/delete-entity.test.js b/services/web/test/frontend/features/file-tree/flows/delete-entity.test.js index 407e3373c4..88ca65bea2 100644 --- a/services/web/test/frontend/features/file-tree/flows/delete-entity.test.js +++ b/services/web/test/frontend/features/file-tree/flows/delete-entity.test.js @@ -55,7 +55,7 @@ describe('FileTree Delete Entity Flow', function() { const fetchMatcher = /\/project\/\w+\/doc\/\w+/ fetchMock.delete(fetchMatcher, 204) - const modalDeleteButton = getModalDeleteButton() + const modalDeleteButton = await getModalDeleteButton() fireEvent.click(modalDeleteButton) window._ide.socket.socketClient.emit('removeEntity', '456def') @@ -82,7 +82,7 @@ describe('FileTree Delete Entity Flow', function() { it('continues delete on 404s', async function() { fetchMock.delete(/\/project\/\w+\/doc\/\w+/, 404) - const modalDeleteButton = getModalDeleteButton() + const modalDeleteButton = await getModalDeleteButton() fireEvent.click(modalDeleteButton) window._ide.socket.socketClient.emit('removeEntity', '456def') @@ -107,7 +107,7 @@ describe('FileTree Delete Entity Flow', function() { const fetchMatcher = /\/project\/\w+\/doc\/\w+/ fetchMock.delete(fetchMatcher, 500) - const modalDeleteButton = getModalDeleteButton() + const modalDeleteButton = await getModalDeleteButton() fireEvent.click(modalDeleteButton) // The modal should still be open, but the file should not be deleted @@ -150,7 +150,7 @@ describe('FileTree Delete Entity Flow', function() { const fetchMatcher = /\/project\/\w+\/(doc|file)\/\w+/ fetchMock.delete(fetchMatcher, 204) - const modalDeleteButton = getModalDeleteButton() + const modalDeleteButton = await getModalDeleteButton() fireEvent.click(modalDeleteButton) window._ide.socket.socketClient.emit('removeEntity', '456def') @@ -181,10 +181,7 @@ describe('FileTree Delete Entity Flow', function() { }) }) - function getModalDeleteButton() { - return screen.getAllByRole('button', { - name: 'Delete', - hidden: true // FIXME: modal should not be hidden but it has the aria-hidden label due to a react-bootstrap bug - })[1] // the first matched button is the toolbar button + async function getModalDeleteButton() { + return waitFor(() => screen.getByRole('button', { name: 'Delete' })) } })