From 04fe2556c38d0bac2a99e99b5bc95edb0cb50f44 Mon Sep 17 00:00:00 2001 From: Ed Schiebel Date: Tue, 6 Jul 2021 17:31:32 -0400 Subject: [PATCH] Reorganize some of the CanvasRce properties closes MAT-328 flag=rce_enhancements It was very confusing having 2 components named CanvasRce, so the one in the canvas-rce package was renamed to simply RCE. That the props for menu, toolbar, and plugins were not consistent between canvas' CanvasRce and the RCE's CanvasRce was a source of confusion (and just wrong). This moves them into the editorOptions property where are all the way through the component hierarchy. test plan: - from the canvas-rce directory run yarn demo:dev > expect the custom toolbar and menu options to work > expect the readonly prop to work (something was broken there too) - enable the site_admin feature "Assignment Enhancements - Student" - create a text entry assignment - as a student, go to the assignment and submit you work > expect the page to work like before Change-Id: I982c7c54750e7a3d9e230e0ab6d16bd078ce9030 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/268505 Tested-by: Service Cloud Jenkins Reviewed-by: Nate Armstrong QA-Review: Nate Armstrong Product-Review: Ed Schiebel --- jest.config.js | 5 +- jest/jest-setup.js | 8 +- packages/canvas-rce/demo/app.js | 14 +- .../src/rce/{CanvasRce.js => RCE.js} | 38 +--- packages/canvas-rce/src/rce/RCEWrapper.js | 54 +++-- packages/canvas-rce/src/rce/StatusBar.js | 7 +- .../src/rce/__mocks__/tinymceReact.js | 7 +- .../{CanvasRce.test.js => RCE.test.js} | 10 +- .../canvas-rce/test/module/RCEWrapper.test.js | 3 +- ui/shared/rce/react/CanvasRce.js | 16 +- .../rce/react/__tests__/CanvasRce.test.js | 201 ++++++++++++++++++ ui/shared/rce/tinymce.config.js | 26 ++- 12 files changed, 306 insertions(+), 83 deletions(-) rename packages/canvas-rce/src/rce/{CanvasRce.js => RCE.js} (85%) rename packages/canvas-rce/src/rce/__tests__/{CanvasRce.test.js => RCE.test.js} (86%) create mode 100644 ui/shared/rce/react/__tests__/CanvasRce.test.js diff --git a/jest.config.js b/jest.config.js index 61105a656b47..6c1fde3de2bd 100644 --- a/jest.config.js +++ b/jest.config.js @@ -27,10 +27,9 @@ module.exports = { '^Backbone$': '/public/javascripts/Backbone.js', // jest can't import the icons '@instructure/ui-icons/es/svg': '/packages/canvas-rce/src/rce/__tests__/_mockIcons.js', - // redirect import from es/rce/CanvasRce to lib - '@instructure/canvas-rce/es/rce/CanvasRce': - '/packages/canvas-rce/lib/rce/CanvasRce.js', + // redirect imports from es/rce to lib '@instructure/canvas-rce/es/rce/tinyRCE': '/packages/canvas-rce/lib/rce/tinyRCE.js', + '@instructure/canvas-rce/es/rce/RCE': '/packages/canvas-rce/lib/rce/RCE.js', // mock the tinymce-react Editor react component '@tinymce/tinymce-react': '/packages/canvas-rce/src/rce/__mocks__/tinymceReact.js' }, diff --git a/jest/jest-setup.js b/jest/jest-setup.js index 6545324c49be..9ff6380412bf 100644 --- a/jest/jest-setup.js +++ b/jest/jest-setup.js @@ -25,7 +25,9 @@ filterUselessConsoleMessages(console) require('jest-fetch-mock').enableFetchMocks() window.scroll = () => {} -window.ENV = {} +window.ENV = { + use_rce_enhancements: true +} Enzyme.configure({adapter: new Adapter()}) @@ -75,8 +77,8 @@ if (process.env.DEPRECATION_SENTRY_DSN) { } }).install() - const setupRavenConsoleLoggingPlugin = require('../app/jsx/shared/helpers/setupRavenConsoleLoggingPlugin') - .default + const setupRavenConsoleLoggingPlugin = + require('../ui/boot/initializers/setupRavenConsoleLoggingPlugin').default setupRavenConsoleLoggingPlugin(Raven, {loggerName: 'console-jest'}) } diff --git a/packages/canvas-rce/demo/app.js b/packages/canvas-rce/demo/app.js index a827c4ded100..4df0e2e75d73 100644 --- a/packages/canvas-rce/demo/app.js +++ b/packages/canvas-rce/demo/app.js @@ -18,7 +18,7 @@ import React, {useEffect, useRef, useState} from 'react' import ReactDOM from 'react-dom' -import CanvasRce from '../src/rce/CanvasRce' +import RCE from '../src/rce/RCE' import DemoOptions from './DemoOptions' import {Button} from '@instructure/ui-buttons' import {View} from '@instructure/ui-view' @@ -183,18 +183,20 @@ function Demo() { return ( <>
- { setCurrentContent(editor.getContent()) }} diff --git a/packages/canvas-rce/src/rce/CanvasRce.js b/packages/canvas-rce/src/rce/RCE.js similarity index 85% rename from packages/canvas-rce/src/rce/CanvasRce.js rename to packages/canvas-rce/src/rce/RCE.js index 9d3b1fd8ec5b..f6216590752d 100644 --- a/packages/canvas-rce/src/rce/CanvasRce.js +++ b/packages/canvas-rce/src/rce/RCE.js @@ -17,9 +17,9 @@ */ import React, {forwardRef, useState} from 'react' -import {arrayOf, bool, func, number, object, objectOf, oneOfType, shape, string} from 'prop-types' +import {arrayOf, bool, func, number, objectOf, oneOfType, shape, string} from 'prop-types' import formatMessage from '../format-message' -import RCEWrapper, {toolbarPropType, menuPropType, ltiToolsPropType} from './RCEWrapper' +import RCEWrapper, {editorOptionsPropType, ltiToolsPropType} from './RCEWrapper' import {trayPropTypes} from './plugins/shared/CanvasContentTray' import editorLanguage from './editorLanguage' import normalizeLocale from './normalizeLocale' @@ -40,7 +40,7 @@ if (!process?.env?.BUILD_LOCALE) { // forward rceRef to it refs the RCEWrapper where clients can call getCode etc. on it. // You probably shouldn't use it until onInit has been called. Until then tinymce // is not initialized. -const CanvasRce = forwardRef(function CanvasRce(props, rceRef) { +const RCE = forwardRef(function RCE(props, rceRef) { const { autosave, defaultContent, @@ -51,13 +51,10 @@ const CanvasRce = forwardRef(function CanvasRce(props, rceRef) { language, liveRegion, mirroredAttrs, // attributes to transfer from the original textarea to the one created by tinymce - menu, - plugins, readOnly, textareaId, textareaClassName, rcsProps, - toolbar, use_rce_pretty_html_editor, use_rce_buttons_and_icons, onFocus, @@ -96,23 +93,15 @@ const CanvasRce = forwardRef(function CanvasRce(props, rceRef) { instRecordDisabled, language: normalizeLocale(language), liveRegion, - menu, - plugins, textareaId, textareaClassName, trayProps: rcsProps, - toolbar, use_rce_pretty_html_editor, use_rce_buttons_and_icons, editorOptions: Object.assign(editorOptions, editorOptions, { selector: `#${textareaId}`, height, - language: editorLanguage(props.language), - toolbar: props.toolbar, - menu: props.menu, - menubar: props.menu ? Object.keys(props.menu).join(' ') : undefined, - plugins: props.plugins, - readonly: readOnly + language: editorLanguage(props.language) }) } wrapInitCb(mirroredAttrs, iProps.editorOptions) @@ -139,9 +128,9 @@ const CanvasRce = forwardRef(function CanvasRce(props, rceRef) { } }) -export default CanvasRce +export default RCE -CanvasRce.propTypes = { +RCE.propTypes = { // do you want the rce to autosave content to localStorage, and // how long should it be until it's deleted. // If autosave is enabled, call yourRef.RCEClosed() if the user @@ -149,8 +138,9 @@ CanvasRce.propTypes = { autosave: shape({enabled: bool, maxAge: number}), // the initial content defaultContent: string, - // tinymce configuration. See defaultTinymceConfig for the basics - editorOptions: object, + // tinymce configuration. See defaultTinymceConfig for all the defaults + // and RCEWrapper.editorOptionsPropType for stuff you may want to include + editorOptions: editorOptionsPropType, // height of the RCE. if a number, in px height: oneOfType([number, string]), // array of URLs to high-contrast css @@ -179,10 +169,6 @@ CanvasRce.propTypes = { // name:value pairs of attributes to add to the textarea // tinymce creates as the backing store of the RCE mirroredAttrs: objectOf(string), - // additional menu items that get merged into the default menubar - menu: menuPropType, - // additional plugins that get merged into the default list of plugins - plugins: arrayOf(string), // is this RCE readonly? readOnly: bool, // id put on the generated textarea @@ -192,8 +178,6 @@ CanvasRce.propTypes = { // properties necessary for the RCE to us the RCS // if missing, RCE features that require the RCS are omitted rcsProps: trayPropTypes, - // additional toolbar items that get merged into the default toolbars - toolbar: toolbarPropType, // enable the pretty html editor (temporary until the feature is forced on) use_rce_pretty_html_editor: bool, // enable the custom buttons feature (temporary until the feature is forced on) @@ -202,10 +186,10 @@ CanvasRce.propTypes = { onFocus: func, // f(RCEWrapper component) onBlur: func, // f(event) onInit: func, // f(tinymce_editor) - onContentChange: func // f(content), don't mistake this as an indication CanvasRce is a controlled component + onContentChange: func // f(content), don't mistake this as an indication RCE is a controlled component } -CanvasRce.defaultProps = { +RCE.defaultProps = { autosave: {enabled: false, maxAge: 3600000}, defaultContent: '', editorOptions: {...defaultTinymceConfig}, diff --git a/packages/canvas-rce/src/rce/RCEWrapper.js b/packages/canvas-rce/src/rce/RCEWrapper.js index 4c2bbb0c592b..97b80d33ec67 100644 --- a/packages/canvas-rce/src/rce/RCEWrapper.js +++ b/packages/canvas-rce/src/rce/RCEWrapper.js @@ -61,6 +61,10 @@ const toolbarPropType = PropTypes.arrayOf( PropTypes.shape({ // name of the toolbar the items are added to // if this toolbar doesn't exist, it is created + // tinymce toolbar config does not + // include a key to identify the individual toolbars, just a name + // which is translated. This toolbar's name must be translated + // in order to be merged correctly. name: PropTypes.string.isRequired, // items added to the toolbar // each is the name of the button some plugin has @@ -70,10 +74,11 @@ const toolbarPropType = PropTypes.arrayOf( ) const menuPropType = PropTypes.objectOf( - // the key is the name of the menu item some plugin has - // registered with tinymce + // the key is the name of the menu item a plugin has + // registered with tinymce. If it does not exist in the + // default menubar, it will be added. PropTypes.shape({ - // if this is a new menu in the menubar,title it's label. + // if this is a new menu in the menubar, title is it's label. // if these are items being merged into an existing menu, title is ignored title: PropTypes.string, // items is a space separated list it menu_items @@ -90,6 +95,26 @@ const ltiToolsPropType = PropTypes.arrayOf( }) ) +export const editorOptionsPropType = PropTypes.shape({ + // height of the RCE. + // if a number interpreted as pixels. + // if a string as a CSS value. + height: PropTypes.oneOfType([PropTypes.number, PropTypes.string]), + // entries you want merged into the toolbar. See toolBarPropType above. + toolbar: toolbarPropType, + // entries you want merged into to the menus. See menuPropType above. + // If an entry defines a new menu, tinymce's menubar config option will + // be updated for you. In fact, if you provide an editorOptions.menubar value + // it will be overwritten. + menu: menuPropType, + // additional plugins that get merged into the default list of plugins + // it is up to you to import the plugin's definition which will + // register it and any related toolbar or menu entries with tinymce. + plugins: PropTypes.arrayOf(PropTypes.string), + // is this RCE readonly? + readonly: PropTypes.bool +}) + // we `require` instead of `import` because the ui-themeable babel require hook only works with `require` // 2021-04-21: This is no longer true, but I didn't want to make a gratutious change when I found this out. // see https://gerrit.instructure.com/c/canvas-lms/+/263299/2/packages/canvas-rce/src/rce/RCEWrapper.js#50 @@ -216,7 +241,7 @@ class RCEWrapper extends React.Component { maxAge: PropTypes.number }), defaultContent: PropTypes.string, - editorOptions: PropTypes.object, + editorOptions: editorOptionsPropType, handleUnmount: PropTypes.func, editorView: PropTypes.oneOf([WYSIWYG_VIEW, PRETTY_HTML_EDITOR_VIEW, RAW_HTML_EDITOR_VIEW]), id: PropTypes.string, @@ -941,6 +966,10 @@ class RCEWrapper extends React.Component { // first view this.setEditorView(this.state.editorView) + // readonly should have been handled via the init property passed + // to , but it's not. + editor.mode.set(this.props.readOnly ? 'readonly' : 'design') + this.props.onInitted?.(editor) } @@ -1303,9 +1332,15 @@ class RCEWrapper extends React.Component { canvasPlugins.push('instructure_buttons') } + const possibleNewMenubarItems = this.props.editorOptions.menu + ? Object.keys(this.props.editorOptions.menu).join(' ') + : undefined + const wrappedOpts = { ...options, + readonly: this.props.readOnly, + theme: 'silver', // some older code specified 'modern', which doesn't exist any more height: options.height || DEFAULT_RCE_HEIGHT, @@ -1343,7 +1378,7 @@ class RCEWrapper extends React.Component { content_css: options.content_css || [], content_style: contentCSS, - menubar: mergeMenuItems('edit view insert format tools table', options.menubar), + menubar: mergeMenuItems('edit view insert format tools table', possibleNewMenubarItems), // default menu options listed at https://www.tiny.cloud/docs/configure/editor-appearance/#menu // tinymce's default edit and table menus are fine // we include all the canvas specific items in the menu and toolbar @@ -1433,14 +1468,13 @@ class RCEWrapper extends React.Component { 'instructure_media_embed', 'instructure_external_tools', 'a11y_checker', - 'wordcount' + 'wordcount', + ...canvasPlugins ], sanitizePlugins(options.plugins) ) } - wrappedOpts.plugins.splice(wrappedOpts.plugins.length, 0, ...canvasPlugins) - if (this.props.trayProps) { wrappedOpts.canvas_rce_user_context = { type: this.props.trayProps.contextType, @@ -1740,10 +1774,6 @@ function mergeMenu(standard, custom) { // returns: the merged result by mutating the incoming standard arg. // It will add commands to existing toolbars, or add a new toolbar // if the custom one does not exist -// This is a little awkward in that tinymce toolbar config does not -// include a key to identify the individual toolbars, just a name -// which is translated. The custom toolbar's name must be translated -// in order to be merged correctly. function mergeToolbar(standard, custom) { if (!custom) return standard // merge given toolbar data into the default toolbar diff --git a/packages/canvas-rce/src/rce/StatusBar.js b/packages/canvas-rce/src/rce/StatusBar.js index 20e1b7a0a2bc..730c0cb1dad3 100644 --- a/packages/canvas-rce/src/rce/StatusBar.js +++ b/packages/canvas-rce/src/rce/StatusBar.js @@ -21,10 +21,9 @@ import ReactDOM from 'react-dom' import {arrayOf, bool, func, number, oneOf, string} from 'prop-types' import {StyleSheet, css} from 'aphrodite' import keycode from 'keycode' -import {Button, CondensedButton, IconButton} from '@instructure/ui-buttons' +import {CondensedButton, IconButton} from '@instructure/ui-buttons' import {Flex} from '@instructure/ui-flex' import {View} from '@instructure/ui-view' -import {ScreenReaderContent} from '@instructure/ui-a11y-content' import {Text} from '@instructure/ui-text' import {SVGIcon} from '@instructure/ui-svg-images' @@ -112,9 +111,11 @@ export default function StatusBar(props) { } // adding a delay before including the HTML Editor description to wait the focus moves to the RCE // and prevent JAWS from reading the aria-describedby element when switching back to RCE view - setTimeout(() => { + const timerid = setTimeout(() => { setIncludeEdtrDesc(props.use_rce_pretty_html_editor && !isHtmlView()) }, 100) + + return () => clearTimeout(timerid) }, [props.editorView]) // eslint-disable-line react-hooks/exhaustive-deps function preferredHtmlEditor() { diff --git a/packages/canvas-rce/src/rce/__mocks__/tinymceReact.js b/packages/canvas-rce/src/rce/__mocks__/tinymceReact.js index 8bd4cc612636..720c3960325b 100644 --- a/packages/canvas-rce/src/rce/__mocks__/tinymceReact.js +++ b/packages/canvas-rce/src/rce/__mocks__/tinymceReact.js @@ -28,9 +28,10 @@ import React, {useEffect, useRef} from 'react' class FakeEditor { - constructor(textareaId) { + constructor(props) { + this.props = props this.hidden = true - this._textareaId = textareaId + this._textareaId = props.id this.readonly = undefined this._eventHandlers = {} } @@ -94,7 +95,7 @@ class FakeEditor { export function Editor(props) { const editorRef = useRef(null) const textareaRef = useRef(null) - const tinymceEditor = useRef(new FakeEditor(props.id)) + const tinymceEditor = useRef(new FakeEditor(props)) useEffect(() => { window.tinymce.editors[0] = tinymceEditor.current diff --git a/packages/canvas-rce/src/rce/__tests__/CanvasRce.test.js b/packages/canvas-rce/src/rce/__tests__/RCE.test.js similarity index 86% rename from packages/canvas-rce/src/rce/__tests__/CanvasRce.test.js rename to packages/canvas-rce/src/rce/__tests__/RCE.test.js index c49b874f5783..491cf5c47b2d 100644 --- a/packages/canvas-rce/src/rce/__tests__/CanvasRce.test.js +++ b/packages/canvas-rce/src/rce/__tests__/RCE.test.js @@ -1,5 +1,5 @@ /* - * Copyright (C) 2018 - present Instructure, Inc. + * Copyright (C) 2021 - present Instructure, Inc. * * This file is part of Canvas. * @@ -18,10 +18,10 @@ import React, {createRef} from 'react' import {render, waitFor} from '@testing-library/react' -import CanvasRce from '../CanvasRce' +import RCE from '../RCE' import bridge from '../../bridge' -describe('CanvasRce', () => { +describe('RCE', () => { let target beforeEach(() => { @@ -38,13 +38,13 @@ describe('CanvasRce', () => { }) it('bridges newly rendered editors', async () => { - render(, target) + render(, target) await waitFor(() => expect(bridge.activeEditor().constructor.displayName).toEqual('RCEWrapper')) }) it('supports getCode() and setCode() on its ref', async () => { const rceRef = createRef(null) - render(, target) + render(, target) await waitFor(() => expect(rceRef.current).not.toBeNull()) diff --git a/packages/canvas-rce/test/module/RCEWrapper.test.js b/packages/canvas-rce/test/module/RCEWrapper.test.js index 0388b9eef59c..c65363fb1f72 100644 --- a/packages/canvas-rce/test/module/RCEWrapper.test.js +++ b/packages/canvas-rce/test/module/RCEWrapper.test.js @@ -91,7 +91,8 @@ function defaultProps() { highContrastCSS: [], languages: [{id: 'en', label: 'English'}], autosave: {enabled: false}, - ltiTools: [] + ltiTools: [], + editorOptions: {} } } diff --git a/ui/shared/rce/react/CanvasRce.js b/ui/shared/rce/react/CanvasRce.js index e046378b85b2..673a8d5fed96 100644 --- a/ui/shared/rce/react/CanvasRce.js +++ b/ui/shared/rce/react/CanvasRce.js @@ -19,7 +19,7 @@ import React, {forwardRef, useEffect, useState} from 'react' import {bool, func, number, object, objectOf, oneOfType, string} from 'prop-types' import {createChainedFunction} from '@instructure/ui-utils' -import TheRealRce from '@instructure/canvas-rce/es/rce/CanvasRce' +import RCE from '@instructure/canvas-rce/es/rce/RCE' import getRCSProps from '../getRCSProps' import closedCaptionLanguages from '@canvas/util/closedCaptionLanguages' import EditorConfig from '../tinymce.config' @@ -67,10 +67,12 @@ const CanvasRce = forwardRef(function CanvasRce(props, rceRef) { // tinymce is a global by now via import of CanvasRce importing tinyRCE const editorConfig = new EditorConfig(tinymce, window.INST, textareaId) const config = {...editorConfig.defaultConfig(), ...editorOptions} - config.init_instance_callback = createChainedFunction( - config.init_instance_callback, - editorOptions.init_instance_callback - ) + if (editorOptions.init_instance_callback) { + config.init_instance_callback = createChainedFunction( + config.init_instance_callback, + editorOptions.init_instance_callback + ) + } return config }) const [autosave_] = useState({ @@ -86,7 +88,7 @@ const CanvasRce = forwardRef(function CanvasRce(props, rceRef) { }, [rceRef]) return ( - . + */ + +import React, {createRef} from 'react' +import {render, waitFor} from '@testing-library/react' +import CanvasRce from '../CanvasRce' + +describe('CanvasRce', () => { + let target + + beforeEach(() => { + const div = document.createElement('div') + div.id = 'fixture' + div.innerHTML = '