ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

validate icon before embedding 

stop letting people create invisible icons.
ensure they have something visible selected,
and if not, prompt them to do so

closes MAT-652
flag=buttons_and_icons_root_account

test plan:
-enable the ff and find an RCE
-try to insert an IM icon with the default
 settings
>observe that the icon is not embedded in the
 RCE
>observe that an alert appears that informs the
 user that they need to take action to make their
 icon visible
-now select a color for the icon
>observe the error message disappears
-submit the icon again by pressing apply
>observe the icon is embedded in the RCE
-edit the inserted icon by removing the color and
 setting it to 'none'
>try to insert and observe the behavior is in line
 with what is outlined above when creating a new
 icon

Change-Id: Iea8f9219272f5bf8bab36d8e96d625136970155e
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/288965
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Joe Hernandez <joe.hernandez@instructure.com>
QA-Review: Joe Hernandez <joe.hernandez@instructure.com>
Product-Review: David Lyons <lyons@instructure.com>
This commit is contained in:
jake.oeding 2022-04-05 16:29:43 -05:00 committed by Jake Oeding
parent 4b77031eed
commit f3125ee8f8
4 changed files with 259 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
packages/canvas-rce/src/rce/plugins/instructure_buttons/components/ButtonsTray.js
View File

@ -22,6 +22,7 @@ import {CloseButton} from '@instructure/ui-buttons'
import {Heading} from '@instructure/ui-heading'
import {Flex} from '@instructure/ui-flex'
import {View} from '@instructure/ui-view'
import {Alert} from '@instructure/ui-alerts'
import {Preview} from './CreateButtonForm/Preview'
import {CreateButtonForm} from './CreateButtonForm'
import {Footer} from './CreateButtonForm/Footer'
@ -32,10 +33,26 @@ import {FixedContentTray} from '../../shared/FixedContentTray'
import {useStoreProps} from '../../shared/StoreContext'
import formatMessage from '../../../../format-message'
import buildDownloadUrl from '../../shared/buildDownloadUrl'
import {validIcon} from '../utils/iconValidation'
function renderHeader(title, settings, setIsOpen, onKeyDown) {
const INVALID_MESSAGE = formatMessage(
'One of the following styles must be added to save an icon: Icon Color, Outline Size, Icon Text, or Image'
)
function renderHeader(title, settings, setIsOpen, onKeyDown, isInvalid, onAlertDismissal) {
return (
<View as="div" background="primary">
{isInvalid && (
<Alert
variant="error"
margin="small"
timeout={10000}
onDismiss={onAlertDismissal}
renderCloseButtonLabel="Close"
>
{INVALID_MESSAGE}
</Alert>
)}
<Flex direction="column">
<Flex.Item padding="medium medium small">
<Flex direction="row">
@ -96,6 +113,7 @@ export function ButtonsTray({editor, onUnmount, editing, rcsConfig}) {
const nameRef = useRef()
const applyRef = useRef()
const [isInvalid, setIsInvalid] = useState(false)
const [isOpen, setIsOpen] = useState(true)
const [replaceAll, setReplaceAll] = useState(false)
@ -117,9 +135,30 @@ export function ButtonsTray({editor, onUnmount, editing, rcsConfig}) {
setReplaceAll(false)
}, [settings.name])
useEffect(() => {
if (validIcon(settings)) {
setIsInvalid(false)
setStatus(statuses.IDLE)
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [
settings.color,
settings.textColor,
settings.text,
settings.encodedImage,
settings.outlineColor,
settings.outlineSize
])
const handleSubmit = ({replaceFile = false}) => {
setStatus(statuses.LOADING)
if (!validIcon(settings)) {
setIsInvalid(true)
setStatus(statuses.ERROR)
return
}
const svg = buildSvg(settings, {isPreview: false})
buildStylesheet()
.then(stylesheet => {
@ -163,13 +202,17 @@ export function ButtonsTray({editor, onUnmount, editing, rcsConfig}) {
setStatus(settingsStatus)
}, [settingsStatus])
const handleAlertDismissal = () => setIsInvalid(false)
return (
<FixedContentTray
title={title}
isOpen={isOpen}
onDismiss={onClose}
onUnmount={onUnmount}
renderHeader={() => renderHeader(title, settings, setIsOpen, onKeyDown)}
renderHeader={() =>
renderHeader(title, settings, setIsOpen, onKeyDown, isInvalid, handleAlertDismissal)
}
renderBody={() =>
renderBody(settings, dispatch, editor, editing, !replaceAll, nameRef, rcsConfig)
}

75
packages/canvas-rce/src/rce/plugins/instructure_buttons/components/__tests__/ButtonsTray.test.js
View File

@ -49,6 +49,11 @@ const editor = {
insertContent: jest.fn()
}
const setIconColor = hex => {
const input = screen.getByRole('textbox', {name: /icon color color picker/i})
fireEvent.input(input, {target: {value: hex}})
}
describe('RCE "Buttons and Icons" Plugin > ButtonsTray', () => {
const defaults = {
editor,
@ -90,7 +95,23 @@ describe('RCE "Buttons and Icons" Plugin > ButtonsTray', () => {
await waitFor(() => expect(onUnmount).toHaveBeenCalled())
})
describe('when the close button is focused', () => {
describe('when the user has not created a valid icon', () => {
beforeEach(() => {
render(<ButtonsTray {...defaults} />)
userEvent.click(screen.getByRole('button', {name: /apply/i}))
})
it('does not fire off the icon upload callback', () => {
expect(startButtonsAndIconsUpload).not.toHaveBeenCalled()
})
it('shows an error message', () => {
const alertMessage = screen.getByText(/one of the following styles/i)
expect(alertMessage).toBeInTheDocument()
})
})
describe('focus management', () => {
let focusedElement, originalFocus
beforeAll(() => {
@ -105,33 +126,29 @@ describe('RCE "Buttons and Icons" Plugin > ButtonsTray', () => {
afterEach(() => (window.HTMLElement.prototype.focus = originalFocus))
describe('and the user does a forward tab', () => {
const event = {key: 'Tab', keyCode: 9}
describe('when the close button is focused', () => {
describe('and the user does a forward tab', () => {
const event = {key: 'Tab', keyCode: 9}
it('moves focus to the "name" input', async () => {
const {findByTestId} = render(<ButtonsTray {...defaults} />)
const closeButton = await findByTestId('icon-maker-close-button')
const expectedElement = await findByTestId('button-name')
fireEvent.keyDown(closeButton, event)
expect(focusedElement).toEqual(expectedElement)
it('moves focus to the "name" input', async () => {
const {findByTestId} = render(<ButtonsTray {...defaults} />)
const closeButton = await findByTestId('icon-maker-close-button')
const expectedElement = await findByTestId('button-name')
fireEvent.keyDown(closeButton, event)
expect(focusedElement).toEqual(expectedElement)
})
})
})
describe('and the user does a reverse tab', () => {
const event = {key: 'Tab', keyCode: 9, shiftKey: true}
describe('and the user does a reverse tab', () => {
const event = {key: 'Tab', keyCode: 9, shiftKey: true}
it('moves focus to the apply button', async () => {
const {findByTestId} = render(<ButtonsTray {...defaults} />)
const closeButton = await findByTestId('icon-maker-close-button')
const expectedElement = await findByTestId('create-icon-button')
fireEvent.keyDown(closeButton, event)
expect(focusedElement).toEqual(expectedElement)
it('moves focus to the apply button', async () => {
const {findByTestId} = render(<ButtonsTray {...defaults} />)
const closeButton = await findByTestId('icon-maker-close-button')
const expectedElement = await findByTestId('create-icon-button')
fireEvent.keyDown(closeButton, event)
expect(focusedElement).toEqual(expectedElement)
})
})
})
})
@ -140,6 +157,7 @@ describe('RCE "Buttons and Icons" Plugin > ButtonsTray', () => {
it('with correct content', async () => {
render(<ButtonsTray {...defaults} />)
setIconColor('#000000')
userEvent.click(screen.getByRole('button', {name: /apply/i}))
let firstCall
await waitFor(() => {
@ -160,7 +178,7 @@ describe('RCE "Buttons and Icons" Plugin > ButtonsTray', () => {
xmlns="http://www.w3.org/2000/svg"
>
<metadata>
{"type":"image/svg+xml-icon-maker-icons","alt":"","shape":"square","size":"small","color":null,"outlineColor":null,"outlineSize":"small","text":"","textSize":"small","textColor":"#000000","textBackgroundColor":null,"textPosition":"middle","encodedImage":"","encodedImageType":"","encodedImageName":"","x":"50%","y":"50%","translateX":-54,"translateY":-54,"width":108,"height":108,"transform":"translate(-54,-54)"}
{"type":"image/svg+xml-icon-maker-icons","alt":"","shape":"square","size":"small","color":"#000000","outlineColor":null,"outlineSize":"small","text":"","textSize":"small","textColor":"#000000","textBackgroundColor":null,"textPosition":"middle","encodedImage":"","encodedImageType":"","encodedImageName":"","x":"50%","y":"50%","translateX":-54,"translateY":-54,"width":108,"height":108,"transform":"translate(-54,-54)"}
</metadata>
<svg
fill="none"
@ -170,7 +188,7 @@ describe('RCE "Buttons and Icons" Plugin > ButtonsTray', () => {
x="0"
>
<g
fill="none"
fill="#000000"
>
<clippath
id="clip-path-for-embed"
@ -210,6 +228,8 @@ describe('RCE "Buttons and Icons" Plugin > ButtonsTray', () => {
it('with overwrite if "replace all" is checked', async () => {
const {getByTestId, getByRole} = render(<ButtonsTray {...defaults} editing />)
setIconColor('#000000')
act(() => {
getByTestId('cb-replace-all').click()
})
@ -230,6 +250,7 @@ describe('RCE "Buttons and Icons" Plugin > ButtonsTray', () => {
render(<ButtonsTray {...defaults} />)
fireEvent.change(document.querySelector('#button-alt-text'), {target: {value: 'banana'}})
setIconColor('#000000')
userEvent.click(screen.getByRole('button', {name: /apply/i}))
await waitFor(() => expect(editor.insertContent).toHaveBeenCalled())
expect(editor.insertContent.mock.calls[0]).toMatchInlineSnapshot(`
@ -244,6 +265,7 @@ describe('RCE "Buttons and Icons" Plugin > ButtonsTray', () => {
it('writes the content to the editor without alt attribute', async () => {
render(<ButtonsTray {...defaults} />)
setIconColor('#000000')
userEvent.click(screen.getByRole('button', {name: /apply/i}))
await waitFor(() => expect(editor.insertContent).toHaveBeenCalled())
expect(editor.insertContent.mock.calls[0]).toMatchInlineSnapshot(`
@ -280,6 +302,7 @@ describe('RCE "Buttons and Icons" Plugin > ButtonsTray', () => {
it('disables footer while submiting', async () => {
render(<ButtonsTray {...defaults} />)
setIconColor('#000000')
const button = screen.getByRole('button', {name: /apply/i})
userEvent.click(button)

124
packages/canvas-rce/src/rce/plugins/instructure_buttons/utils/__tests__/iconValidation.test.js Normal file
View File

@ -0,0 +1,124 @@
/*
* Copyright (C) 2022 - 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 {
hasBackgroundColor,
hasVisibleText,
hasImage,
hasVisibleOutline,
validIcon
} from '../iconValidation'
describe('icon validation', () => {
let settings
beforeEach(() => {
settings = {
color: null,
encodedImage: '',
outlineColor: null,
outlineSize: 'none',
text: '',
textColor: null
}
})
describe('hasBackgroundColor', () => {
it('is false if the icon has no background color', () => {
expect(hasBackgroundColor(settings)).toBe(false)
})
it('is true if the icon has a background color', () => {
settings.color = '#000000'
expect(hasBackgroundColor(settings)).toBe(true)
})
})
describe('hasVisibleText', () => {
it('is false if the icon has no text color', () => {
expect(hasVisibleText(settings)).toBe(false)
})
it('is false if the icon has text color but no text', () => {
settings.textColor = '#000000'
expect(hasVisibleText(settings)).toBe(false)
})
it('is true if the icon has text color and text', () => {
settings.textColor = '#000000'
settings.text = 'blah'
expect(hasVisibleText(settings)).toBe(true)
})
})
describe('hasImage', () => {
it('is false if the icon has no image', () => {
expect(hasImage(settings)).toBe(false)
})
it('is true if the icon has an image', () => {
settings.encodedImage = 'data:image/svg+xml;base64,PHN2ZyB3aWR...'
expect(hasImage(settings)).toBe(true)
})
})
describe('hasVisibleOutline', () => {
it('is false if the icon has no outline color', () => {
expect(hasVisibleOutline(settings)).toBe(false)
})
it('is false if the icon has an outline color but outline size is none', () => {
settings.outlineColor = '#000000'
expect(hasVisibleOutline(settings)).toBe(false)
})
it('is true if the icon has outline color and outline size', () => {
settings.outlineColor = '#000000'
settings.outlineSize = 'small'
expect(hasVisibleOutline(settings)).toBe(true)
})
})
describe('validIcon', () => {
it('is false if none of the criteria are met', () => {
expect(validIcon(settings)).toBe(false)
})
it('is true if the icon has a background color', () => {
settings.color = '#000000'
expect(validIcon(settings)).toBe(true)
})
it('is true if the icon has text color and text', () => {
settings.textColor = '#000000'
settings.text = 'blah'
expect(validIcon(settings)).toBe(true)
})
it('is true if the icon has an image', () => {
settings.encodedImage = 'data:image/svg+xml;base64,PHN2ZyB3aWR...'
expect(validIcon(settings)).toBe(true)
})
it('is true if the icon has outline color and outline size', () => {
settings.outlineColor = '#000000'
settings.outlineSize = 'small'
expect(validIcon(settings)).toBe(true)
})
})
})

41
packages/canvas-rce/src/rce/plugins/instructure_buttons/utils/iconValidation.js Normal file
View File

@ -0,0 +1,41 @@
/*
* Copyright (C) 2022 - 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/>.
*/
export const hasBackgroundColor = settings => {
return !!settings.color
}
export const hasVisibleText = settings => {
return !!settings.textColor && settings.text.length > 0
}
export const hasImage = settings => {
return settings.encodedImage.length > 0
}
export const hasVisibleOutline = settings => {
return !!settings.outlineColor && settings.outlineSize !== 'none'
}
export const validIcon = settings => {
return [hasBackgroundColor, hasVisibleText, hasImage, hasVisibleOutline].some(func =>
func(settings)
)
}
export default validIcon