Guarantee content added from the tray goes to the right RCE

When RCEWrapper renders CanvasContentTray, the tray registers itself
with the bridge. Since the bridge is a singleton, it was stuck
on the last RCE in the page.  This commit changes Bridge so it keeps
a map of editor.id -> tray controller. Now it can find the right
controller for the active tray. As a side-effect, the CanvasContentTray
has to know its parent RCE's id, so that's a new prop.

I also added keys to RCEWrapper and CanvasContentTray. Focus management
can get wonky if there are >1 instance of a component on the page, and
w/o the key, React can't tell them apart.

I also changed the Bridge var in RCEWrapper to bridge (lower-case)
since it's an instance, not the class. Just fo clarity.

closes LS-895
flag = rce_enhancements

test plan:
  - see the ticket recreate scenario
  - it should work for inserting anything from the tray

Change-Id: I0b46d56113ce2a22ecd07831c0f7102d40abc25d
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/242423
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
This commit is contained in:
Ed Schiebel 2020-07-10 16:38:46 -04:00
parent 30ccc20142
commit 358e2d9871
9 changed files with 58 additions and 42 deletions

View File

@ -31,14 +31,15 @@ export default class Bridge {
this.trayProps = new WeakMap()
this._languages = []
this._controller = {}
}
get editorRendered() {
return this._editorRendered
}
get controller() {
return this._controller
controller(editorId) {
return this._controller[editorId]
}
activeEditor() {
@ -97,16 +98,16 @@ export default class Bridge {
}
}
attachController(controller) {
this._controller = controller
attachController(controller, editorId) {
this._controller[editorId] = controller
}
detachController() {
this._controller = null
detachController(editorId) {
delete this._controller[editorId]
}
showTrayForPlugin(plugin) {
this._controller && this._controller.showTrayForPlugin(plugin)
showTrayForPlugin(plugin, editorId) {
this._controller[editorId]?.showTrayForPlugin(plugin)
}
existingContentToLink() {
@ -134,7 +135,7 @@ export default class Bridge {
link.text = link.title || link.href
}
this.focusedEditor.insertLink(link)
this.controller?.hideTray()
this.controller(this.focusedEditor.id)?.hideTray()
} else {
console.warn('clicked sidebar link without a focused editor')
}
@ -155,9 +156,7 @@ export default class Bridge {
insertImage(image) {
if (this.focusedEditor) {
this.focusedEditor.insertImage(image)
if (this.controller) {
this.controller.hideTray()
}
this.controller(this.focusedEditor.id)?.hideTray()
} else {
console.warn('clicked sidebar image without a focused editor')
}
@ -216,18 +215,14 @@ export default class Bridge {
insertVideo = video => {
if (this.focusedEditor) {
this.focusedEditor.insertVideo(video)
}
if (this.controller) {
this.controller.hideTray()
this.controller(this.focusedEditor.id)?.hideTray()
}
}
insertAudio = audio => {
if (this.focusedEditor) {
this.focusedEditor.insertAudio(audio)
}
if (this.controller) {
this.controller.hideTray()
this.controller(this.focusedEditor.id)?.hideTray()
}
}
}

View File

@ -74,6 +74,7 @@ describe('Editor/Sidebar bridge', () => {
beforeEach(() => {
jest.spyOn(console, 'warn')
editor = {
id: 'editor_id',
addAlert: jest.fn(),
insertLink: jest.fn(),
insertVideo: jest.fn(),
@ -124,7 +125,8 @@ describe('Editor/Sidebar bridge', () => {
it('calls hideTray after inserting a link', () => {
const hideTray = jest.fn()
Bridge.attachController({hideTray})
Bridge.focusEditor({id: 'editor_id'})
Bridge.attachController({hideTray}, 'editor_id')
Bridge.focusEditor(editor)
Bridge.insertLink({})
expect(hideTray).toHaveBeenCalledTimes(1)
@ -176,7 +178,7 @@ describe('Editor/Sidebar bridge', () => {
let hideTray
beforeEach(() => {
hideTray = jest.fn()
Bridge.attachController({hideTray})
Bridge.attachController({hideTray}, 'editor_id')
Bridge.focusEditor(editor)
})

View File

@ -31,7 +31,7 @@ import formatMessage from '../format-message'
import * as contentInsertion from './contentInsertion'
import indicatorRegion from './indicatorRegion'
import indicate from '../common/indicate'
import Bridge from '../bridge'
import bridge from '../bridge'
import CanvasContentTray, {trayProps} from './plugins/shared/CanvasContentTray'
import StatusBar from './StatusBar'
import ShowOnFocusButton from './ShowOnFocusButton'
@ -171,6 +171,7 @@ class RCEWrapper extends React.Component {
defaultContent: PropTypes.string,
editorOptions: PropTypes.object,
handleUnmount: PropTypes.func,
id: PropTypes.string,
language: PropTypes.string,
onFocus: PropTypes.func,
onBlur: PropTypes.func,
@ -222,10 +223,13 @@ class RCEWrapper extends React.Component {
messages: [],
announcement: null,
confirmAutoSave: false,
autoSavedContent: ''
autoSavedContent: '',
id: this.props.id || this.props.textareaId || `${Date.now()}`
}
alertHandler.alertFunc = this.addAlert
this.handleContentTrayClosing = this.handleContentTrayClosing.bind(this)
}
// getCode and setCode naming comes from tinyMCE
@ -468,7 +472,7 @@ class RCEWrapper extends React.Component {
}
onRemove = () => {
Bridge.detachEditor(this)
bridge.detachEditor(this)
this.props.onRemove && this.props.onRemove(this)
}
@ -480,6 +484,10 @@ class RCEWrapper extends React.Component {
return this.getTextarea().value
}
get id() {
return this.state.id
}
toggle = () => {
if (this.isHidden()) {
this.setState({isHtmlView: false})
@ -524,7 +532,7 @@ class RCEWrapper extends React.Component {
handleFocus(_event) {
if (!this.state.focused) {
this.setState({focused: true})
Bridge.focusEditor(this)
bridge.focusEditor(this)
this._forceCloseFloatingToolbar()
this.props.onFocus && this.props.onFocus(this)
}
@ -532,7 +540,7 @@ class RCEWrapper extends React.Component {
contentTrayClosing = false
handleContentTrayClosing = isClosing => {
handleContentTrayClosing(isClosing) {
this.contentTrayClosing = isClosing
}
@ -986,8 +994,8 @@ class RCEWrapper extends React.Component {
brandColor: this.theme.canvasBrandColor,
...this.props.trayProps
}
Bridge.trayProps.set(editor, trayPropsWithColor)
Bridge.languages = this.props.languages
bridge.trayProps.set(editor, trayPropsWithColor)
bridge.languages = this.props.languages
if (typeof setupCallback === 'function') {
setupCallback(editor)
}
@ -1141,6 +1149,7 @@ class RCEWrapper extends React.Component {
return (
<div
key={this.id}
className={`${styles.root} rce-wrapper`}
ref={el => (this._elementRef = el)}
onFocus={this.handleFocusRCE}
@ -1186,7 +1195,9 @@ class RCEWrapper extends React.Component {
onA11yChecker={this.onA11yChecker}
/>
<CanvasContentTray
bridge={Bridge}
key={this.id}
bridge={bridge}
editor={this}
onTrayClosing={this.handleContentTrayClosing}
{...trayProps}
/>

View File

@ -30,7 +30,7 @@ tinymce.create('tinymce.plugins.InstructureDocumentsPlugin', {
// Register commands
ed.addCommand('mceInstructureDocuments', clickCallback.bind(this, ed, document))
ed.addCommand('instructureTrayForDocuments', (ui, plugin_key) => {
bridge.showTrayForPlugin(plugin_key)
bridge.showTrayForPlugin(plugin_key, ed.id)
})
// Register menu items

View File

@ -34,7 +34,7 @@ tinymce.create('tinymce.plugins.InstructureImagePlugin', {
// Register commands
editor.addCommand('mceInstructureImage', clickCallback.bind(this, editor, document))
editor.addCommand('instructureTrayForImages', (ui, plugin_key) => {
bridge.showTrayForPlugin(plugin_key)
bridge.showTrayForPlugin(plugin_key, editor.id)
})
// Register menu items

View File

@ -81,7 +81,7 @@ tinymce.create('tinymce.plugins.InstructureLinksPlugin', {
ed.addCommand('instructureLinkCreate', clickCallback.bind(this, ed, CREATE_LINK))
ed.addCommand('instructureLinkEdit', clickCallback.bind(this, ed, EDIT_LINK))
ed.addCommand('instructureTrayForLinks', (ui, plugin_key) => {
bridge.showTrayForPlugin(plugin_key)
bridge.showTrayForPlugin(plugin_key, ed.id)
})
ed.addCommand('instructureTrayToEditLink', (ui, editor) => {
trayController.showTrayForEditor(editor)

View File

@ -33,7 +33,7 @@ tinymce.create('tinymce.plugins.InstructureRecord', {
ed.addCommand('instructureRecord', clickCallback.bind(this, ed, document))
ed.addCommand('instructureTrayForMedia', (ui, plugin_key) => {
bridge.showTrayForPlugin(plugin_key)
bridge.showTrayForPlugin(plugin_key, ed.id)
})
// Register menu items

View File

@ -160,12 +160,13 @@ export default function CanvasContentTray(props) {
const [filterSettings, setFilterSettings] = useFilterSettings()
const onTrayClosing = props.onTrayClosing
const {bridge, editor, onTrayClosing} = {...props}
const handleDismissTray = useCallback(() => {
bridge.focusEditor(editor)
onTrayClosing && onTrayClosing(true) // tell RCEWrapper we're closing
setIsOpen(false)
}, [onTrayClosing])
}, [editor, bridge, onTrayClosing])
useEffect(() => {
const controller = {
@ -178,25 +179,27 @@ export default function CanvasContentTray(props) {
}
}
props.bridge.attachController(controller)
bridge.attachController(controller, editor.id)
return () => {
props.bridge.detachController()
bridge.detachController(editor.id)
}
}, [props.bridge, handleDismissTray, setFilterSettings])
// it's OK the setFilterSettings is not a dependency
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [editor.id, bridge, handleDismissTray])
function handleExitTray() {
props.onTrayClosing && props.onTrayClosing(true) // tell RCEWrapper we're closing
onTrayClosing && onTrayClosing(true) // tell RCEWrapper we're closing
}
function handleCloseTray() {
props.bridge.focusActiveEditor(false)
bridge.focusActiveEditor(false)
// increment a counter that's used a the key when rendering
// this gets us a new instance everytime, which is necessary
// to get the queries run so we have up to date data.
setOpenCount(openCount + 1)
setHasOpened(false)
props.onTrayClosing && props.onTrayClosing(false) // tell RCEWrapper we're closed
onTrayClosing && onTrayClosing(false) // tell RCEWrapper we're closed
}
function handleFilterChange(newFilter, onChangeContext) {
@ -238,7 +241,10 @@ export default function CanvasContentTray(props) {
onDismiss={handleDismissTray}
onClose={handleCloseTray}
onExit={handleExitTray}
onOpen={() => setHasOpened(true)}
onOpen={() => {
bridge.focusEditor(editor)
setHasOpened(true)
}}
>
{isOpen && hasOpened ? (
<Flex direction="column" display="block" height="100vh" overflowY="hidden">
@ -308,6 +314,7 @@ export const trayProps = shape(trayPropsMap)
CanvasContentTray.propTypes = {
bridge: instanceOf(Bridge).isRequired,
editor: shape({id: string}).isRequired,
onTrayClosing: func, // called with true when the tray starts closing, false once closed
...trayPropsMap
}

View File

@ -34,6 +34,7 @@ describe('RCE Plugins > CanvasContentTray', () => {
function getProps(override = {}) {
props = {
bridge: new Bridge(),
editor: {id: 'editor_id'},
containingContext: {type: 'course', contextId: '1201', userId: '17'},
contextId: '1201',
contextType: 'course',
@ -58,7 +59,7 @@ describe('RCE Plugins > CanvasContentTray', () => {
async function showTrayForPlugin(plugin) {
act(() => {
props.bridge.controller.showTrayForPlugin(plugin)
props.bridge.showTrayForPlugin(plugin, 'editor_id')
})
await wait(getTray, {timeout: 19500})
}