diff --git a/client/src/templates/Challenges/classic/editor.tsx b/client/src/templates/Challenges/classic/editor.tsx index 1b10f17d26f..61f4e34297a 100644 --- a/client/src/templates/Challenges/classic/editor.tsx +++ b/client/src/templates/Challenges/classic/editor.tsx @@ -123,19 +123,33 @@ export interface EditorProps { showIndependentLowerJaw?: boolean; } -// TODO: this is grab bag of unrelated properties. There's no need for them to -// all be in the same object. -interface EditorProperties { +interface MonacoInstances { editor?: editor.IStandaloneCodeEditor; model?: editor.ITextModel; - descriptionZoneId: string; - insideEditDecId: string; - descriptionZoneTop: number; - outputZoneTop: number; - outputZoneId: string; - descriptionNode?: HTMLDivElement; - descriptionWidget?: editor.IContentWidget; - outputWidget?: editor.IOverlayWidget; +} + +interface DescriptionZoneState { + zoneId: string; + top: number; + node?: HTMLDivElement; + widget?: editor.IContentWidget; +} + +interface OutputZoneState { + zoneId: string; + top: number; + widget?: editor.IOverlayWidget; +} + +interface EditableRegionState { + decorationId: string; +} + +interface EditorState { + monaco: MonacoInstances; + descriptionZone: DescriptionZoneState; + outputZone: OutputZoneState; + editableRegion: EditableRegionState; } const mapStateToProps = createSelector( @@ -242,13 +256,20 @@ const defineMonacoThemes = ( }); }; -const initialData: EditorProperties = { - descriptionZoneId: '', - insideEditDecId: '', - descriptionZoneTop: 0, - outputZoneId: '', - outputZoneTop: 0 -}; +const createInitialEditorState = (): EditorState => ({ + monaco: {}, + descriptionZone: { + zoneId: '', + top: 0 + }, + outputZone: { + zoneId: '', + top: 0 + }, + editableRegion: { + decorationId: '' + } +}); const Editor = (props: EditorProps): JSX.Element => { const { t } = useTranslation(); @@ -266,9 +287,12 @@ const Editor = (props: EditorProps): JSX.Element => { // only take effect during the next render, which is too late. We could use // plain objects here, but useRef is shared between instances, so avoids // unnecessary object creation. + // Each Editor instance (e.g. per file in MultifileEditor) must get its own + // nested state; a shallow copy would share refs across instances and cause + // "element was detached" and wrong-editor behavior. const monacoRef: MutableRefObject = useRef(null); - const dataRef = useRef({ ...initialData }); + const dataRef = useRef(createInitialEditorState()); const [lowerJawContainer, setLowerJawContainer] = React.useState(null); @@ -400,12 +424,12 @@ const Editor = (props: EditorProps): JSX.Element => { } const model = - dataRef.current.model || + dataRef.current.monaco.model || createModel( challengeFile?.contents ?? '', modeMap[challengeFile?.ext ?? 'html'] ); - dataRef.current.model = model; + dataRef.current.monaco.model = model; if (player.current.shouldPlay && !player.current.sampler) { void import('tone').then(tone => { @@ -427,7 +451,7 @@ const Editor = (props: EditorProps): JSX.Element => { // changes coming from outside the editor (such as code resets). const resetEditorValues = () => { const { challengeFiles, fileKey } = props; - const { model } = dataRef.current; + const { model } = dataRef.current.monaco; const initialContents = challengeFiles?.find( challengeFile => challengeFile.fileKey === fileKey @@ -443,7 +467,10 @@ const Editor = (props: EditorProps): JSX.Element => { // reacts to the Tab key. Setting it to true allows the user to tab // out of the editor. False keeps it inside the editor and creates a tab. const setMonacoTabTrapped = (trapped: boolean) => - dataRef.current.editor?.createContextKey('editorTabMovesFocus', !trapped); + dataRef.current.monaco.editor?.createContextKey( + 'editorTabMovesFocus', + !trapped + ); const editorDidMount = ( editor: editor.IStandaloneCodeEditor, @@ -452,7 +479,7 @@ const Editor = (props: EditorProps): JSX.Element => { const { isMobileLayout, isUsingKeyboardInTablist } = props; // TODO this should *probably* be set on focus editorRef.current = editor; - dataRef.current.editor = editor; + dataRef.current.monaco.editor = editor; if (hasEditableRegion()) { initializeRegions(); @@ -746,7 +773,7 @@ const Editor = (props: EditorProps): JSX.Element => { const descriptionZoneCallback = ( changeAccessor: editor.IViewZoneChangeAccessor ) => { - const editor = dataRef.current.editor; + const editor = dataRef.current.monaco.editor; if (!editor) return; const domNode = createDescription(editor); @@ -766,15 +793,15 @@ const Editor = (props: EditorProps): JSX.Element => { // This is called when the editor dimensions change and AFTER the // text in the editor has shifted. onDomNodeTop: () => { - dataRef.current.descriptionZoneTop = + dataRef.current.descriptionZone.top = editor.getTopForLineNumber(getLineBeforeEditableRegion() + 1) - domNode.offsetHeight; - if (dataRef.current.descriptionWidget) - editor.layoutContentWidget(dataRef.current.descriptionWidget); + if (dataRef.current.descriptionZone.widget) + editor.layoutContentWidget(dataRef.current.descriptionZone.widget); } }; - dataRef.current.descriptionZoneId = changeAccessor.addZone(viewZone); + dataRef.current.descriptionZone.zoneId = changeAccessor.addZone(viewZone); }; function tryToExecuteChallenge() { @@ -796,26 +823,27 @@ const Editor = (props: EditorProps): JSX.Element => { // position of the overlayWidget (i.e. trigger it via onComputedHeight). If // not the editor may report the wrong value for position of the lines. editor.changeViewZones(changeAccessor => { - changeAccessor.removeZone(dataRef.current.outputZoneId); + changeAccessor.removeZone(dataRef.current.outputZone.zoneId); const viewZone = { afterLineNumber: getLastLineOfEditableRegion(), heightInPx: lowerJawContainer.offsetHeight, domNode: document.createElement('div'), onComputedHeight: () => - dataRef.current.outputWidget && - editor.layoutOverlayWidget(dataRef.current.outputWidget), + dataRef.current.outputZone.widget && + editor.layoutOverlayWidget(dataRef.current.outputZone.widget), onDomNodeTop: (top: number) => { - dataRef.current.outputZoneTop = top; - if (dataRef.current.outputWidget) - editor.layoutOverlayWidget(dataRef.current.outputWidget); + dataRef.current.outputZone.top = top; + if (dataRef.current.outputZone.widget) + editor.layoutOverlayWidget(dataRef.current.outputZone.widget); } }; - dataRef.current.outputZoneId = changeAccessor.addZone(viewZone); + dataRef.current.outputZone.zoneId = changeAccessor.addZone(viewZone); }); }; function createDescription(editor: editor.IStandaloneCodeEditor) { - if (dataRef.current.descriptionNode) return dataRef.current.descriptionNode; + if (dataRef.current.descriptionZone.node) + return dataRef.current.descriptionZone.node; const { description, title, isChallengeCompleted } = props; const jawHeading = isChallengeCompleted ? document.createElement('div') @@ -869,7 +897,7 @@ const Editor = (props: EditorProps): JSX.Element => { domNode.style.width = `${getEditorContentWidth(editor)}px`; domNode.style.top = getDescriptionZoneTop(); - dataRef.current.descriptionNode = domNode; + dataRef.current.descriptionZone.node = domNode; return domNode; } @@ -904,7 +932,8 @@ const Editor = (props: EditorProps): JSX.Element => { } function resetMarginDecorations() { - const { model, insideEditDecId } = dataRef.current; + const { model } = dataRef.current.monaco; + const { decorationId: insideEditDecId } = dataRef.current.editableRegion; const range = model?.getDecorationRange(insideEditDecId); if (range) { updateEditableRegion(range, { model }); @@ -1008,7 +1037,7 @@ const Editor = (props: EditorProps): JSX.Element => { options: editor.IModelDecorationOptions = {} ) { const { model } = modelContext; - const { insideEditDecId } = dataRef.current; + const { decorationId: insideEditDecId } = dataRef.current.editableRegion; const oldOptions = model?.getDecorationOptions(insideEditDecId); const lineDecoration = { @@ -1022,23 +1051,23 @@ const Editor = (props: EditorProps): JSX.Element => { } function getDescriptionZoneTop() { - return `${dataRef.current.descriptionZoneTop}px`; + return `${dataRef.current.descriptionZone.top}px`; } function getOutputZoneTop() { - return `${dataRef.current.outputZoneTop}px`; + return `${dataRef.current.outputZone.top}px`; } function getLineBeforeEditableRegion() { - const range = dataRef.current.model?.getDecorationRange( - dataRef.current.insideEditDecId + const range = dataRef.current.monaco.model?.getDecorationRange( + dataRef.current.editableRegion.decorationId ); return range ? range.startLineNumber - 1 : 1; } function getLastLineOfEditableRegion() { - const range = dataRef.current.model?.getDecorationRange( - dataRef.current.insideEditDecId + const range = dataRef.current.monaco.model?.getDecorationRange( + dataRef.current.editableRegion.decorationId ); return range ? range.endLineNumber : 1; } @@ -1046,7 +1075,8 @@ const Editor = (props: EditorProps): JSX.Element => { // This Range covers all the text in the editable region, const getLinesCoveringEditableRegion = () => { const monaco = monacoRef.current; - const { model, insideEditDecId } = dataRef.current; + const { model } = dataRef.current.monaco; + const { decorationId: insideEditDecId } = dataRef.current.editableRegion; // TODO: this is a little low-level, but we should bail if there is no // editable region defined. if (!insideEditDecId || !model || !monaco) { @@ -1074,7 +1104,7 @@ const Editor = (props: EditorProps): JSX.Element => { } function focusIfTargetEditor() { - const { editor } = dataRef.current; + const { editor } = dataRef.current.monaco; const { canFocusOnMountRef } = props; if (!editor || !canFocusOnMountRef.current) return; if (!props.usesMultifileEditor) { @@ -1088,7 +1118,7 @@ const Editor = (props: EditorProps): JSX.Element => { } function initializeRegions() { - const { model, editor } = dataRef.current; + const { model, editor } = dataRef.current.monaco; const monaco = monacoRef.current; if (!model || !monaco || !editor) return; const editableRegion = getEditableRegionFromRedux(); @@ -1097,10 +1127,13 @@ const Editor = (props: EditorProps): JSX.Element => { editableRegion[1] - 1 ]); - dataRef.current.insideEditDecId = initializeEditableRegion(editableRange, { - monaco, - model - })[0]; + dataRef.current.editableRegion.decorationId = initializeEditableRegion( + editableRange, + { + monaco, + model + } + )[0]; } const createWidget = ( @@ -1138,15 +1171,15 @@ const Editor = (props: EditorProps): JSX.Element => { }; function addWidgetsToRegions() { - const editor = dataRef.current.editor; + const editor = dataRef.current.monaco.editor; if (!editor) return; const descriptionNode = createDescription(editor); const lowerJawNode = createLowerJawContainer(editor); - if (!dataRef.current.descriptionWidget) { - dataRef.current.descriptionWidget = createWidget( + if (!dataRef.current.descriptionZone.widget) { + dataRef.current.descriptionZone.widget = createWidget( editor, 'description.widget', descriptionNode, @@ -1155,36 +1188,36 @@ const Editor = (props: EditorProps): JSX.Element => { // this order (add widget, change zone) is necessary, since the zone // relies on the domnode being in the DOM to calculate its height - that // doesn't happen until the widget is added. - editor.addContentWidget(dataRef.current.descriptionWidget); + editor.addContentWidget(dataRef.current.descriptionZone.widget); editor.changeViewZones(descriptionZoneCallback); // Now that the description zone is in place, the browser knows its height // and we can use that to calculate the top of the output zone. If we do // not do this the output zone will be on top of the description zone, // initially. - dataRef.current.outputZoneTop = editor.getTopForLineNumber( + dataRef.current.outputZone.top = editor.getTopForLineNumber( getLastLineOfEditableRegion() + 1 ); } - if (!dataRef.current.outputWidget) { - dataRef.current.outputWidget = createWidget( + if (!dataRef.current.outputZone.widget) { + dataRef.current.outputZone.widget = createWidget( editor, 'output.widget', lowerJawNode, getOutputZoneTop ); - editor.addOverlayWidget(dataRef.current.outputWidget); + editor.addOverlayWidget(dataRef.current.outputZone.widget); } editor.onDidScrollChange(() => { - if (dataRef.current.descriptionWidget) - editor.layoutContentWidget(dataRef.current.descriptionWidget); - if (dataRef.current.outputWidget) - editor.layoutOverlayWidget(dataRef.current.outputWidget); + if (dataRef.current.descriptionZone.widget) + editor.layoutContentWidget(dataRef.current.descriptionZone.widget); + if (dataRef.current.outputZone.widget) + editor.layoutOverlayWidget(dataRef.current.outputZone.widget); }); } function addContentChangeListener() { - const { model } = dataRef.current; + const { model } = dataRef.current.monaco; const monaco = monacoRef.current; if (!monaco) return; @@ -1263,7 +1296,7 @@ const Editor = (props: EditorProps): JSX.Element => { useEffect(() => { // If a challenge is reset, it needs to communicate that change to the // editor. - const { editor } = dataRef.current; + const { editor } = dataRef.current.monaco; if (props.isResetting) { // NOTE: this looks a lot like a race condition, since stopResetting gets @@ -1302,7 +1335,8 @@ const Editor = (props: EditorProps): JSX.Element => { }, [props.previewOpen]); useEffect(() => { - const { model, insideEditDecId } = dataRef.current; + const { model } = dataRef.current.monaco; + const { decorationId: insideEditDecId } = dataRef.current.editableRegion; const isChallengeComplete = challengeIsComplete(); const range = model?.getDecorationRange(insideEditDecId); if (range && isChallengeComplete) { @@ -1317,7 +1351,7 @@ const Editor = (props: EditorProps): JSX.Element => { }, [props.tests]); useEffect(() => { - const editor = dataRef.current.editor; + const editor = dataRef.current.monaco.editor; editor?.layout(); // layout() resets the monaco tab trapping back to default (true), so we // need to untrap it if the user had it set to false. @@ -1327,9 +1361,9 @@ const Editor = (props: EditorProps): JSX.Element => { }, [props.dimensions]); function updateDescriptionZone() { - const editor = dataRef.current.editor; + const editor = dataRef.current.monaco.editor; editor?.changeViewZones(changeAccessor => { - changeAccessor.removeZone(dataRef.current.descriptionZoneId); + changeAccessor.removeZone(dataRef.current.descriptionZone.zoneId); descriptionZoneCallback(changeAccessor); }); } @@ -1395,7 +1429,10 @@ const Editor = (props: EditorProps): JSX.Element => { tryToSubmitChallenge={tryToSubmitChallenge} isSignedIn={props.isSignedIn} updateContainer={() => - updateOutputViewZone(lowerJawContainer, dataRef.current.editor) + updateOutputViewZone( + lowerJawContainer, + dataRef.current.monaco.editor + ) } />, lowerJawContainer