refactor alerts in direct share course tray

This is a preparatory commit for reusing the tray for a content share
import.

It also includes a minor change in behavior to the tray: when the copy
operation has started, the "copy" and "cancel" buttons are replaced with
a single "close" button.

The mechanism for doing these particular screen reader alerts has also
changed to something that should be more reliable. Instead of using the
static alert region on the page, this simply adds the screen reader
alert properties to the alert text. Modern screen readers seem to handle
this better than the static alert div.

refs ADMIN-2912
flag=direct_share

test plan:
- should still be able to copy a discussion from one course to another
  as before, with the changes to the button behavior specified above.
- screen reader alerts for the operation status should still work.

Change-Id: I46395b0c70cd111f455feac7f7d8d44b785552d1
Reviewed-on: https://gerrit.instructure.com/211843
Tested-by: Jenkins
Reviewed-by: Charley Kline <ckline@instructure.com>
QA-Review: Charley Kline <ckline@instructure.com>
Product-Review: Jon Willesen <jonw+gerrit@instructure.com>
This commit is contained in:
Jon Willesen 2019-10-02 10:46:08 -06:00 committed by Jon Willesen
parent a62a222a0c
commit 0d7bcdd077
7 changed files with 336 additions and 80 deletions

View File

@ -10,7 +10,7 @@ function appAndSpecFilesFor(path) {
// or if there is a folder of code that your team controls that you want
// to start ensuring conforms to prettier, add it to this array to opt-in
// now to conform to prettier.
const PRETTIER_WHITELIST = module.exports = [
const PRETTIER_WHITELIST = (module.exports = [
'./*.js',
'app/jsx/*.js',
'frontend_build/**/*.js',
@ -40,6 +40,7 @@ const PRETTIER_WHITELIST = module.exports = [
appAndSpecDirsFor('login'),
appAndSpecDirsFor('permissions'),
appAndSpecDirsFor('shared/components'),
appAndSpecDirsFor('shared/direct_share'),
appAndSpecDirsFor('speed_grader'),
appAndSpecDirsFor('theme_editor')
]
])

View File

@ -0,0 +1,51 @@
/*
* Copyright (C) 2019 - present Instructure, Inc.
*
* This file is part of Canvas.
*
* Canvas is free software: you can redistribute it and/or modify it under
* the terms of the GNU Affero General Public License as published by the Free
* Software Foundation, version 3 of the License.
*
* Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
* A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
* details.
*
* You should have received a copy of the GNU Affero General Public License along
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import React from 'react'
import {bool, node, oneOf} from 'prop-types'
import {Alert} from '@instructure/ui-alerts'
import {ScreenReaderContent} from '@instructure/ui-a11y-content'
CanvasInlineAlert.propTypes = {
liveAlert: bool,
screenReaderOnly: bool,
politeness: oneOf(['assertive', 'polite']),
children: node
}
export default function CanvasInlineAlert({
children,
liveAlert,
screenReaderOnly,
politeness = 'assertive',
...alertProps
}) {
let body = children
if (liveAlert || screenReaderOnly) {
body = (
<span role="alert" aria-live={politeness} aria-atomic>
{body}
</span>
)
}
if (screenReaderOnly) {
body = <ScreenReaderContent>{body}</ScreenReaderContent>
}
return <Alert {...alertProps}>{body}</Alert>
}

View File

@ -0,0 +1,41 @@
/*
* Copyright (C) 2019 - present Instructure, Inc.
*
* This file is part of Canvas.
*
* Canvas is free software: you can redistribute it and/or modify it under
*
* the terms of the GNU Affero General Public License as published by the Free
* Software Foundation, version 3 of the License.
*
* Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
* A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
* details.
*
* You should have received a copy of the GNU Affero General Public License along
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import React from 'react'
import {render} from '@testing-library/react'
import CanvasInlineAlert from '../CanvasInlineAlert'
describe('CanvasInlineAlert', () => {
it('renders a basic alert with no sr features', () => {
const {getByText} = render(<CanvasInlineAlert>alert message</CanvasInlineAlert>)
expect(getByText('alert message')).toBeInTheDocument()
expect(document.querySelector('[role="alert"]')).toBeNull()
})
it('renders an alert with sr features', () => {
render(<CanvasInlineAlert liveAlert>alert message</CanvasInlineAlert>)
expect(document.querySelector('[role="alert"]')).toBeInTheDocument()
})
it('renders an sr-only alert', () => {
render(<CanvasInlineAlert screenReaderOnly>alert message</CanvasInlineAlert>)
expect(document.querySelector('[role="alert"]')).toBeInTheDocument()
// There's no great way to tell if instui has done its ScreenReaderContent thing
})
})

View File

@ -21,14 +21,13 @@ import I18n from 'i18n!direct_share_course_panel'
import React, {useState} from 'react'
import {func, string} from 'prop-types'
import {Alert} from '@instructure/ui-alerts'
import {Button} from '@instructure/ui-buttons'
import {Flex} from '@instructure/ui-layout'
import {Spinner} from '@instructure/ui-elements'
import doFetchApi from 'jsx/shared/effects/doFetchApi'
import contentSelectionShape from 'jsx/shared/proptypes/contentSelection'
import ManagedCourseSelector from '../components/ManagedCourseSelector'
import DirectShareOperationStatus from './DirectShareOperationStatus'
// eventually this will have options for where to place the item in the new course.
// for now, it just has the selector plus some buttons
@ -39,79 +38,44 @@ DirectShareCoursePanel.propTypes = {
onCancel: func
}
function getLiveRegion() {
return document.querySelector('#flash_screenreader_holder')
}
export default function DirectShareCoursePanel({sourceCourseId, contentSelection, onCancel}) {
const [selectedCourse, setSelectedCourse] = useState(null)
const [postStatus, setPostStatus] = useState(null)
const [startCopyOperationPromise, setStartCopyOperationPromise] = useState(null)
function startCopyOperation() {
return doFetchApi({
method: 'POST',
path: `/api/v1/courses/${selectedCourse.id}/content_migrations`,
body: {
migration_type: 'course_copy_importer',
select: contentSelection,
settings: {source_course_id: sourceCourseId}
},
})
}
function handleStart() {
setPostStatus('starting')
startCopyOperation()
.then(() => {
setPostStatus('success')
})
.catch(err => {
console.error(err) // eslint-disable-line no-console
if (err.response) console.error(err.response) // eslint-disable-line no-console
setPostStatus('error')
})
}
let alert = null
const alertProps = {
margin: 'small 0',
liveRegion: getLiveRegion
}
if (postStatus === 'error') {
alert = (
<Alert variant="error" {...alertProps}>
{I18n.t('There was a problem starting the copy operation')}
</Alert>
)
} else if (postStatus === 'success') {
alert = (
<Alert variant="success" {...alertProps}>
{I18n.t('Copy operation started successfully')}
</Alert>
)
} else if (postStatus === 'starting') {
alert = (
<Alert variant="info" {...alertProps}>
{I18n.t('Starting copy operation')}
<Spinner renderTitle="" size="x-small" />
</Alert>
setStartCopyOperationPromise(
doFetchApi({
method: 'POST',
path: `/api/v1/courses/${selectedCourse.id}/content_migrations`,
body: {
migration_type: 'course_copy_importer',
select: contentSelection,
settings: {source_course_id: sourceCourseId}
}
})
)
}
const copyButtonDisabled = selectedCourse === null || postStatus !== null
const copyButton = startCopyOperationPromise ? null : (
<Button variant="primary" disabled={selectedCourse === null} onClick={startCopyOperation}>
{I18n.t('Copy')}
</Button>
)
return (
<>
{alert}
<DirectShareOperationStatus
promise={startCopyOperationPromise}
startingMsg={I18n.t('Starting copy operation')}
successMsg={I18n.t('Copy operation started successfully')}
errorMsg={I18n.t('There was a problem starting the copy operation')}
/>
<ManagedCourseSelector onCourseSelected={setSelectedCourse} />
<Flex justifyItems="end" padding="small 0 0 0">
<Flex.Item>
<Button variant="primary" disabled={copyButtonDisabled} onClick={handleStart}>
{I18n.t('Copy')}
</Button>
{copyButton}
<Button margin="0 0 0 x-small" onClick={onCancel}>
{I18n.t('Cancel')}
{startCopyOperationPromise ? I18n.t('Close') : I18n.t('Cancel')}
</Button>
</Flex.Item>
</Flex>

View File

@ -0,0 +1,75 @@
/*
* Copyright (C) 2019 - present Instructure, Inc.
*
* This file is part of Canvas.
*
* Canvas is free software: you can redistribute it and/or modify it under
* the terms of the GNU Affero General Public License as published by the Free
* Software Foundation, version 3 of the License.
*
* Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
* A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
* details.
*
* You should have received a copy of the GNU Affero General Public License along
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import React, {useRef, useState} from 'react'
import CanvasInlineAlert from 'jsx/shared/components/CanvasInlineAlert'
import {Spinner} from '@instructure/ui-elements'
export default function DirectShareOperationStatus({promise, startingMsg, successMsg, errorMsg}) {
const [operationStatus, setOperationStatus] = useState('starting')
const previousPromise = useRef(null)
if (previousPromise.current !== promise) {
previousPromise.current = promise
setOperationStatus('starting')
if (promise) {
promise
.then(() => {
if (promise === previousPromise.current) {
setOperationStatus('success')
}
})
.catch(err => {
if (promise === previousPromise.current) {
console.error(err) // eslint-disable-line no-console
if (err && err.response) console.error(err.response) // eslint-disable-line no-console
setOperationStatus('error')
}
})
}
}
let alert
const alertProps = {
liveAlert: true,
margin: 'small 0'
}
if (operationStatus === 'error') {
alert = (
<CanvasInlineAlert variant="error" {...alertProps}>
{errorMsg}
</CanvasInlineAlert>
)
} else if (operationStatus === 'success') {
alert = (
<CanvasInlineAlert variant="success" {...alertProps}>
{successMsg}
</CanvasInlineAlert>
)
} else if (operationStatus === 'starting') {
alert = (
<CanvasInlineAlert variant="info" {...alertProps}>
{startingMsg}
<Spinner renderTitle="" size="x-small" />
</CanvasInlineAlert>
)
}
return promise ? alert : null
}

View File

@ -87,21 +87,22 @@ describe('DirectShareCoursePanel', () => {
})
it('starts a copy operation and reports status', async () => {
fetchMock.postOnce(
'path:/api/v1/courses/abc/content_migrations',
{id: '8', workflow_state: 'running'}
)
const {getByText, getAllByText, getByLabelText} = render(
fetchMock.postOnce('path:/api/v1/courses/abc/content_migrations', {
id: '8',
workflow_state: 'running'
})
const {getByText, getByLabelText, queryByText} = render(
<DirectShareCoursePanel
sourceCourseId="42"
contentSelection={{discussion_topics: ['1123']}}
/>)
/>
)
const input = getByLabelText(/select a course/i)
fireEvent.click(input)
fireEvent.click(getByText('abc'))
const copyButton = getByText(/copy/i).closest('button')
fireEvent.click(copyButton)
expect(copyButton.getAttribute('disabled')).toBe('')
fireEvent.click(getByText(/copy/i))
expect(queryByText('Copy')).toBeNull()
expect(getByText('Close')).toBeInTheDocument()
const [, fetchOptions] = fetchMock.lastCall()
expect(fetchOptions.method).toBe('POST')
expect(JSON.parse(fetchOptions.body)).toMatchObject({
@ -109,10 +110,11 @@ describe('DirectShareCoursePanel', () => {
select: {discussion_topics: ['1123']},
settings: {source_course_id: '42'}
})
expect(getAllByText(/start/i)).toHaveLength(2)
expect(getByText(/start/i)).toBeInTheDocument()
await act(() => fetchMock.flush(true))
expect(getAllByText(/success/)).toHaveLength(2)
expect(copyButton.getAttribute('disabled')).toBe('')
expect(getByText(/success/)).toBeInTheDocument()
expect(queryByText('Copy')).toBeNull()
expect(getByText('Close')).toBeInTheDocument()
})
describe('errors', () => {
@ -126,15 +128,17 @@ describe('DirectShareCoursePanel', () => {
it('reports an error if the fetch fails', async () => {
fetchMock.postOnce('path:/api/v1/courses/abc/content_migrations', 400)
const {getByText, getAllByText, getByLabelText} = render(<DirectShareCoursePanel sourceCourseId="42" />)
const {getByText, getByLabelText, queryByText} = render(
<DirectShareCoursePanel sourceCourseId="42" />
)
const input = getByLabelText(/select a course/i)
fireEvent.click(input)
fireEvent.click(getByText('abc'))
const copyButton = getByText(/copy/i).closest('button')
fireEvent.click(copyButton)
fireEvent.click(getByText('Copy'))
await act(() => fetchMock.flush(true))
expect(getAllByText(/problem/i)).toHaveLength(2)
expect(copyButton.getAttribute('disabled')).toBe('')
expect(getByText(/problem/i)).toBeInTheDocument()
expect(queryByText('Copy')).toBeNull()
expect(getByText('Close')).toBeInTheDocument()
})
})
})

View File

@ -0,0 +1,120 @@
/*
* Copyright (C) 2019 - present Instructure, Inc.
*
* This file is part of Canvas.
*
* Canvas is free software: you can redistribute it and/or modify it under
*
* the terms of the GNU Affero General Public License as published by the Free
* Software Foundation, version 3 of the License.
*
* Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
* A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
* details.
*
* You should have received a copy of the GNU Affero General Public License along
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import React from 'react'
import {render} from '@testing-library/react'
import DirectShareOperationStatus from '../DirectShareOperationStatus'
describe('DirectShareOperationStatus', () => {
it('shows nothing if there is no promise', () => {
const {queryByText} = render(
<DirectShareOperationStatus
promise={null}
startingMsg="starting"
successMsg="success"
errorMsg="error"
/>
)
expect(queryByText(/starting|success|error/)).toBeNull()
})
it('shows the starting message when the promise is not resolved', () => {
const promise = new Promise(() => {})
const {getByText, queryByText} = render(
<DirectShareOperationStatus
promise={promise}
startingMsg="starting"
successMsg="success"
errorMsg="error"
/>
)
expect(getByText('starting')).toBeInTheDocument()
expect(queryByText(/success|error/)).toBeNull()
})
it('does an sr alert', () => {
const promise = new Promise(() => {})
render(
<DirectShareOperationStatus
promise={promise}
startingMsg="starting"
successMsg="success"
errorMsg="error"
/>
)
expect(document.querySelector('[role="alert"]')).toBeInTheDocument()
})
it('shows success when promise is fulfilled', async () => {
const promise = Promise.resolve()
const {findByText, queryByText} = render(
<DirectShareOperationStatus
promise={promise}
startingMsg="starting"
successMsg="success"
errorMsg="error"
/>
)
expect(await findByText('success')).toBeInTheDocument()
expect(queryByText(/starting|error/)).toBeNull()
})
describe('errors', () => {
beforeEach(() => jest.spyOn(console, 'error').mockImplementation(() => {}))
afterEach(() => console.error.mockRestore()) // eslint-disable-line no-console
it('shows error when promise is rejected', async () => {
const promise = Promise.reject()
const {findByText, queryByText} = render(
<DirectShareOperationStatus
promise={promise}
startingMsg="starting"
successMsg="success"
errorMsg="error"
/>
)
expect(await findByText('error')).toBeInTheDocument()
expect(queryByText(/starting|success/)).toBeNull()
expect(console.error).toHaveBeenCalled() // eslint-disable-line no-console
})
})
it('resets when it receives a new promise', async () => {
const promise = Promise.resolve()
const {findByText, getByText, rerender} = render(
<DirectShareOperationStatus
promise={promise}
startingMsg="starting"
successMsg="success"
errorMsg="error"
/>
)
expect(await findByText('success')).toBeInTheDocument()
const newPromise = new Promise(() => {})
rerender(
<DirectShareOperationStatus
promise={newPromise}
startingMsg="starting"
successMsg="success"
errorMsg="error"
/>
)
expect(getByText('starting')).toBeInTheDocument()
})
})