From fe0d19f5f6b499f55ec09172f4ca5566100afc35 Mon Sep 17 00:00:00 2001 From: Vishawdeep Singh <vishawdeepsingh29@gmail.com> Date: Tue, 24 Dec 2024 15:18:57 +0530 Subject: [PATCH 01/14] fix Exiting Multi-Line Cursor Doesnt Return to the Original Cursor --- client/modules/IDE/components/Editor/index.jsx | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/client/modules/IDE/components/Editor/index.jsx b/client/modules/IDE/components/Editor/index.jsx index 4ef69d2a44..b7a4846fd8 100644 --- a/client/modules/IDE/components/Editor/index.jsx +++ b/client/modules/IDE/components/Editor/index.jsx @@ -193,6 +193,18 @@ class Editor extends React.Component { if (/^[a-z]$/i.test(e.key) && (mode === 'css' || mode === 'javascript')) { showHint(_cm, this.props.autocompleteHinter, this.props.fontSize); } + if (e.key === 'Escape') { + console.log(this._cm); + e.preventDefault(); + const selections = this._cm.listSelections(); + if (selections.length > 1) { + const firstPos = selections[0].head || selections[0].anchor; + this._cm.setSelection(firstPos); + this._cm.scrollIntoView(firstPos); + } else { + this._cm.getInputField().blur(); + } + } }); this._cm.getWrapperElement().style[ From ce374151edc0d4cab31d372dc2434c95ca371478 Mon Sep 17 00:00:00 2001 From: raclim <raclim@nyu.edu> Date: Thu, 6 Feb 2025 19:19:14 -0500 Subject: [PATCH 02/14] remove extra console log --- client/modules/IDE/components/Editor/index.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/modules/IDE/components/Editor/index.jsx b/client/modules/IDE/components/Editor/index.jsx index b7a4846fd8..bd38c96668 100644 --- a/client/modules/IDE/components/Editor/index.jsx +++ b/client/modules/IDE/components/Editor/index.jsx @@ -194,9 +194,9 @@ class Editor extends React.Component { showHint(_cm, this.props.autocompleteHinter, this.props.fontSize); } if (e.key === 'Escape') { - console.log(this._cm); e.preventDefault(); const selections = this._cm.listSelections(); + if (selections.length > 1) { const firstPos = selections[0].head || selections[0].anchor; this._cm.setSelection(firstPos); From c1102d9531d57860d9a3f8bb06ea58c04b1d07a7 Mon Sep 17 00:00:00 2001 From: Connie Ye <constanceye1316@gmail.com> Date: Thu, 13 Feb 2025 21:29:04 -0800 Subject: [PATCH 03/14] pull out codemirror test --- .../IDE/components/Editor/codemirror.js | 175 ++++++++++++++++++ .../modules/IDE/components/Editor/index.jsx | 168 ++--------------- 2 files changed, 191 insertions(+), 152 deletions(-) create mode 100644 client/modules/IDE/components/Editor/codemirror.js diff --git a/client/modules/IDE/components/Editor/codemirror.js b/client/modules/IDE/components/Editor/codemirror.js new file mode 100644 index 0000000000..56e02984fe --- /dev/null +++ b/client/modules/IDE/components/Editor/codemirror.js @@ -0,0 +1,175 @@ +import CodeMirror from 'codemirror'; +import 'codemirror/mode/css/css'; +import 'codemirror/mode/clike/clike'; +import 'codemirror/addon/selection/active-line'; +import 'codemirror/addon/lint/lint'; +import 'codemirror/addon/lint/javascript-lint'; +import 'codemirror/addon/lint/css-lint'; +import 'codemirror/addon/lint/html-lint'; +import 'codemirror/addon/fold/brace-fold'; +import 'codemirror/addon/fold/comment-fold'; +import 'codemirror/addon/fold/foldcode'; +import 'codemirror/addon/fold/foldgutter'; +import 'codemirror/addon/fold/indent-fold'; +import 'codemirror/addon/fold/xml-fold'; +import 'codemirror/addon/comment/comment'; +import 'codemirror/keymap/sublime'; +import 'codemirror/addon/search/searchcursor'; +import 'codemirror/addon/search/matchesonscrollbar'; +import 'codemirror/addon/search/match-highlighter'; +import 'codemirror/addon/search/jump-to-line'; +import 'codemirror/addon/edit/matchbrackets'; +import 'codemirror/addon/edit/closebrackets'; +import 'codemirror/addon/selection/mark-selection'; +import 'codemirror-colorpicker'; + +import { debounce } from 'lodash'; +import emmet from '@emmetio/codemirror-plugin'; + +import { metaKey } from '../../../../utils/metaKey'; +import { showHint } from './hinter'; +import tidyCode from './tidier'; + +const INDENTATION_AMOUNT = 2; + +emmet(CodeMirror); + +function setupCodeMirrorHooks( + cmInstance, + { + setUnsavedChanges, + hideRuntimeErrorWarning, + updateFileContent, + file, + autorefresh, + isPlaying, + clearConsole, + startSketch, + autocompleteHinter, + fontSize + }, + updateLineNumber +) { + cmInstance.on( + 'change', + debounce(() => { + setUnsavedChanges(true); + hideRuntimeErrorWarning(); + updateFileContent(file.id, cmInstance.getValue()); + if (autorefresh && isPlaying) { + clearConsole(); + startSketch(); + } + }, 1000) + ); + + cmInstance.on('keyup', () => { + const lineNumber = parseInt(cmInstance.getCursor().line + 1, 10); + updateLineNumber(lineNumber); + }); + + cmInstance.on('keydown', (_cm, e) => { + // Show hint + const mode = cmInstance.getOption('mode'); + if (/^[a-z]$/i.test(e.key) && (mode === 'css' || mode === 'javascript')) { + showHint(_cm, autocompleteHinter, fontSize); + } + if (e.key === 'Escape') { + e.preventDefault(); + const selections = cmInstance.listSelections(); + + if (selections.length > 1) { + const firstPos = selections[0].head || selections[0].anchor; + cmInstance.setSelection(firstPos); + cmInstance.scrollIntoView(firstPos); + } else { + cmInstance.getInputField().blur(); + } + } + }); + + cmInstance.getWrapperElement().style['font-size'] = `${fontSize}px`; +} + +export default function setupCodeMirror( + container, + props, + onUpdateLinting, + docs, + updateLineNumber +) { + const { theme, lineNumbers, linewrap, autocloseBracketsQuotes } = props; + const cm = CodeMirror(container, { + theme: `p5-${theme}`, + lineNumbers, + styleActiveLine: true, + inputStyle: 'contenteditable', + lineWrapping: linewrap, + fixedGutter: false, + foldGutter: true, + foldOptions: { widget: '\u2026' }, + gutters: ['CodeMirror-foldgutter', 'CodeMirror-lint-markers'], + keyMap: 'sublime', + highlightSelectionMatches: true, // highlight current search match + matchBrackets: true, + emmet: { + preview: ['html'], + markTagPairs: true, + autoRenameTags: true + }, + autoCloseBrackets: autocloseBracketsQuotes, + styleSelectedText: true, + lint: { + onUpdateLinting, + options: { + asi: true, + eqeqeq: false, + '-W041': false, + esversion: 11 + } + }, + colorpicker: { + type: 'sketch', + mode: 'edit' + } + }); + + delete cm.options.lint.options.errors; + + const replaceCommand = + metaKey === 'Ctrl' ? `${metaKey}-H` : `${metaKey}-Option-F`; + cm.setOption('extraKeys', { + Tab: (tabCm) => { + if (!tabCm.execCommand('emmetExpandAbbreviation')) return; + // might need to specify and indent more? + const selection = tabCm.doc.getSelection(); + if (selection.length > 0) { + tabCm.execCommand('indentMore'); + } else { + tabCm.replaceSelection(' '.repeat(INDENTATION_AMOUNT)); + } + }, + Enter: 'emmetInsertLineBreak', + Esc: 'emmetResetAbbreviation', + [`Shift-Tab`]: false, + [`${metaKey}-Enter`]: () => null, + [`Shift-${metaKey}-Enter`]: () => null, + [`${metaKey}-F`]: 'findPersistent', + [`Shift-${metaKey}-F`]: () => tidyCode(cm), + [`${metaKey}-G`]: 'findPersistentNext', + [`Shift-${metaKey}-G`]: 'findPersistentPrev', + [replaceCommand]: 'replace', + // Cassie Tarakajian: If you don't set a default color, then when you + // choose a color, it deletes characters inline. This is a + // hack to prevent that. + [`${metaKey}-K`]: (metaCm, event) => + metaCm.state.colorpicker.popup_color_picker({ length: 0 }), + [`${metaKey}-.`]: 'toggleComment' // Note: most adblockers use the shortcut ctrl+. + }); + + setupCodeMirrorHooks(cm, props, updateLineNumber); + + cm.swapDoc(docs[props.file.id]); + + return cm; +} diff --git a/client/modules/IDE/components/Editor/index.jsx b/client/modules/IDE/components/Editor/index.jsx index bd38c96668..a3e8d7c7a3 100644 --- a/client/modules/IDE/components/Editor/index.jsx +++ b/client/modules/IDE/components/Editor/index.jsx @@ -3,32 +3,8 @@ import PropTypes from 'prop-types'; import React from 'react'; import CodeMirror from 'codemirror'; -import emmet from '@emmetio/codemirror-plugin'; import { withTranslation } from 'react-i18next'; import StackTrace from 'stacktrace-js'; -import 'codemirror/mode/css/css'; -import 'codemirror/mode/clike/clike'; -import 'codemirror/addon/selection/active-line'; -import 'codemirror/addon/lint/lint'; -import 'codemirror/addon/lint/javascript-lint'; -import 'codemirror/addon/lint/css-lint'; -import 'codemirror/addon/lint/html-lint'; -import 'codemirror/addon/fold/brace-fold'; -import 'codemirror/addon/fold/comment-fold'; -import 'codemirror/addon/fold/foldcode'; -import 'codemirror/addon/fold/foldgutter'; -import 'codemirror/addon/fold/indent-fold'; -import 'codemirror/addon/fold/xml-fold'; -import 'codemirror/addon/comment/comment'; -import 'codemirror/keymap/sublime'; -import 'codemirror/addon/search/searchcursor'; -import 'codemirror/addon/search/matchesonscrollbar'; -import 'codemirror/addon/search/match-highlighter'; -import 'codemirror/addon/search/jump-to-line'; -import 'codemirror/addon/edit/matchbrackets'; -import 'codemirror/addon/edit/closebrackets'; -import 'codemirror/addon/selection/mark-selection'; -import 'codemirror-colorpicker'; import classNames from 'classnames'; import { debounce } from 'lodash'; @@ -37,7 +13,6 @@ import { bindActionCreators } from 'redux'; import MediaQuery from 'react-responsive'; import '../../../../utils/htmlmixed'; import '../../../../utils/p5-javascript'; -import { metaKey } from '../../../../utils/metaKey'; import '../../../../utils/codemirror-search'; import beepUrl from '../../../../sounds/audioAlert.mp3'; @@ -62,13 +37,10 @@ import { EditorContainer, EditorHolder } from './MobileEditor'; import { FolderIcon } from '../../../../common/icons'; import IconButton from '../../../../common/IconButton'; -import { showHint, hideHinter } from './hinter'; +import { hideHinter } from './hinter'; import getFileMode from './utils'; import tidyCode from './tidier'; - -emmet(CodeMirror); - -const INDENTATION_AMOUNT = 2; +import setupCodeMirror from './codemirror'; class Editor extends React.Component { constructor(props) { @@ -96,121 +68,18 @@ class Editor extends React.Component { componentDidMount() { this.beep = new Audio(beepUrl); - // this.widgets = []; - this._cm = CodeMirror(this.codemirrorContainer, { - theme: `p5-${this.props.theme}`, - lineNumbers: this.props.lineNumbers, - styleActiveLine: true, - inputStyle: 'contenteditable', - lineWrapping: this.props.linewrap, - fixedGutter: false, - foldGutter: true, - foldOptions: { widget: '\u2026' }, - gutters: ['CodeMirror-foldgutter', 'CodeMirror-lint-markers'], - keyMap: 'sublime', - highlightSelectionMatches: true, // highlight current search match - matchBrackets: true, - emmet: { - preview: ['html'], - markTagPairs: true, - autoRenameTags: true - }, - autoCloseBrackets: this.props.autocloseBracketsQuotes, - styleSelectedText: true, - lint: { - onUpdateLinting: (annotations) => { - this.updateLintingMessageAccessibility(annotations); - }, - options: { - asi: true, - eqeqeq: false, - '-W041': false, - esversion: 11 - } - }, - colorpicker: { - type: 'sketch', - mode: 'edit' - } - }); - - delete this._cm.options.lint.options.errors; - - const replaceCommand = - metaKey === 'Ctrl' ? `${metaKey}-H` : `${metaKey}-Option-F`; - this._cm.setOption('extraKeys', { - Tab: (cm) => { - if (!cm.execCommand('emmetExpandAbbreviation')) return; - // might need to specify and indent more? - const selection = cm.doc.getSelection(); - if (selection.length > 0) { - cm.execCommand('indentMore'); - } else { - cm.replaceSelection(' '.repeat(INDENTATION_AMOUNT)); - } - }, - Enter: 'emmetInsertLineBreak', - Esc: 'emmetResetAbbreviation', - [`Shift-Tab`]: false, - [`${metaKey}-Enter`]: () => null, - [`Shift-${metaKey}-Enter`]: () => null, - [`${metaKey}-F`]: 'findPersistent', - [`Shift-${metaKey}-F`]: () => tidyCode(this._cm), - [`${metaKey}-G`]: 'findPersistentNext', - [`Shift-${metaKey}-G`]: 'findPersistentPrev', - [replaceCommand]: 'replace', - // Cassie Tarakajian: If you don't set a default color, then when you - // choose a color, it deletes characters inline. This is a - // hack to prevent that. - [`${metaKey}-K`]: (cm, event) => - cm.state.colorpicker.popup_color_picker({ length: 0 }), - [`${metaKey}-.`]: 'toggleComment' // Note: most adblockers use the shortcut ctrl+. - }); - this.initializeDocuments(this.props.files); - this._cm.swapDoc(this._docs[this.props.file.id]); - this._cm.on( - 'change', - debounce(() => { - this.props.setUnsavedChanges(true); - this.props.hideRuntimeErrorWarning(); - this.props.updateFileContent(this.props.file.id, this._cm.getValue()); - if (this.props.autorefresh && this.props.isPlaying) { - this.props.clearConsole(); - this.props.startSketch(); - } - }, 1000) + this._cm = setupCodeMirror( + this.codemirrorContainer, + this.props, + (annotations) => { + this.updateLintingMessageAccessibility(annotations); + }, + this._docs, + (lineNumber) => this.setState({ currentLine: lineNumber }) ); - if (this._cm) { - this._cm.on('keyup', this.handleKeyUp); - } - - this._cm.on('keydown', (_cm, e) => { - // Show hint - const mode = this._cm.getOption('mode'); - if (/^[a-z]$/i.test(e.key) && (mode === 'css' || mode === 'javascript')) { - showHint(_cm, this.props.autocompleteHinter, this.props.fontSize); - } - if (e.key === 'Escape') { - e.preventDefault(); - const selections = this._cm.listSelections(); - - if (selections.length > 1) { - const firstPos = selections[0].head || selections[0].anchor; - this._cm.setSelection(firstPos); - this._cm.scrollIntoView(firstPos); - } else { - this._cm.getInputField().blur(); - } - } - }); - - this._cm.getWrapperElement().style[ - 'font-size' - ] = `${this.props.fontSize}px`; - this.props.provideController({ tidyCode: () => tidyCode(this._cm), showFind: this.showFind, @@ -341,11 +210,6 @@ class Editor extends React.Component { return updatedFile; } - handleKeyUp = () => { - const lineNumber = parseInt(this._cm.getCursor().line + 1, 10); - this.setState({ currentLine: lineNumber }); - }; - showFind() { this._cm.execCommand('findPersistent'); } @@ -488,7 +352,7 @@ Editor.propTypes = { ).isRequired, updateLintMessage: PropTypes.func.isRequired, clearLintMessage: PropTypes.func.isRequired, - updateFileContent: PropTypes.func.isRequired, + // updateFileContent: PropTypes.func.isRequired, fontSize: PropTypes.number.isRequired, file: PropTypes.shape({ name: PropTypes.string.isRequired, @@ -498,9 +362,9 @@ Editor.propTypes = { url: PropTypes.string }).isRequired, setUnsavedChanges: PropTypes.func.isRequired, - startSketch: PropTypes.func.isRequired, - autorefresh: PropTypes.bool.isRequired, - isPlaying: PropTypes.bool.isRequired, + // startSketch: PropTypes.func.isRequired, + // autorefresh: PropTypes.bool.isRequired, + // isPlaying: PropTypes.bool.isRequired, theme: PropTypes.string.isRequired, unsavedChanges: PropTypes.bool.isRequired, files: PropTypes.arrayOf( @@ -514,8 +378,8 @@ Editor.propTypes = { collapseSidebar: PropTypes.func.isRequired, closeProjectOptions: PropTypes.func.isRequired, expandSidebar: PropTypes.func.isRequired, - clearConsole: PropTypes.func.isRequired, - hideRuntimeErrorWarning: PropTypes.func.isRequired, + // clearConsole: PropTypes.func.isRequired, + // hideRuntimeErrorWarning: PropTypes.func.isRequired, runtimeErrorWarningVisible: PropTypes.bool.isRequired, provideController: PropTypes.func.isRequired, t: PropTypes.func.isRequired, From 2c917a2dbb048996d523ebfd798c4b33b54ef225 Mon Sep 17 00:00:00 2001 From: Connie Ye <constanceye1316@gmail.com> Date: Fri, 28 Feb 2025 08:36:44 -0800 Subject: [PATCH 04/14] Convert editor file to functional --- .../IDE/components/Editor/codemirror.js | 37 +- .../modules/IDE/components/Editor/index.jsx | 498 +++++++++--------- client/utils/usePrevious.js | 15 + 3 files changed, 305 insertions(+), 245 deletions(-) create mode 100644 client/utils/usePrevious.js diff --git a/client/modules/IDE/components/Editor/codemirror.js b/client/modules/IDE/components/Editor/codemirror.js index 56e02984fe..6ef2056391 100644 --- a/client/modules/IDE/components/Editor/codemirror.js +++ b/client/modules/IDE/components/Editor/codemirror.js @@ -93,12 +93,26 @@ function setupCodeMirrorHooks( export default function setupCodeMirror( container, - props, + { + theme, + lineNumbers, + linewrap, + autocloseBracketsQuotes, + setUnsavedChanges, + hideRuntimeErrorWarning, + updateFileContent, + file, + autorefresh, + isPlaying, + clearConsole, + startSketch, + autocompleteHinter, + fontSize + }, onUpdateLinting, docs, updateLineNumber ) { - const { theme, lineNumbers, linewrap, autocloseBracketsQuotes } = props; const cm = CodeMirror(container, { theme: `p5-${theme}`, lineNumbers, @@ -167,9 +181,24 @@ export default function setupCodeMirror( [`${metaKey}-.`]: 'toggleComment' // Note: most adblockers use the shortcut ctrl+. }); - setupCodeMirrorHooks(cm, props, updateLineNumber); + setupCodeMirrorHooks( + cm, + { + setUnsavedChanges, + hideRuntimeErrorWarning, + updateFileContent, + file, + autorefresh, + isPlaying, + clearConsole, + startSketch, + autocompleteHinter, + fontSize + }, + updateLineNumber + ); - cm.swapDoc(docs[props.file.id]); + cm.swapDoc(docs[file.id]); return cm; } diff --git a/client/modules/IDE/components/Editor/index.jsx b/client/modules/IDE/components/Editor/index.jsx index a3e8d7c7a3..7e00bc345e 100644 --- a/client/modules/IDE/components/Editor/index.jsx +++ b/client/modules/IDE/components/Editor/index.jsx @@ -1,7 +1,7 @@ // TODO: convert to functional component import PropTypes from 'prop-types'; -import React from 'react'; +import React, { useCallback, useEffect, useRef, useState } from 'react'; import CodeMirror from 'codemirror'; import { withTranslation } from 'react-i18next'; import StackTrace from 'stacktrace-js'; @@ -41,122 +41,205 @@ import { hideHinter } from './hinter'; import getFileMode from './utils'; import tidyCode from './tidier'; import setupCodeMirror from './codemirror'; +import usePrevious from '../../../../utils/usePrevious'; -class Editor extends React.Component { - constructor(props) { - super(props); - this.state = { - currentLine: 1 - }; - this._cm = null; +function Editor({ + provideController, + files, + file, + theme, + linewrap, + lineNumbers, + closeProjectOptions, + setSelectedFile, + unsavedChanges, + setUnsavedChanges, + lintMessages, + lintWarning, + clearLintMessage, + updateLintMessage, + updateFileContent, + autorefresh, + isPlaying, + clearConsole, + startSketch, + autocompleteHinter, + autocloseBracketsQuotes, + fontSize, + consoleEvents, + hideRuntimeErrorWarning, + runtimeErrorWarningVisible, + expandConsole, + isExpanded, + t, + collapseSidebar, + expandSidebar +}) { + const [currentLine, setCurrentLine] = useState(1); + const cm = useRef(); + const beep = useRef(); + const docs = useRef(); + const previous = usePrevious({ file, unsavedChanges, consoleEvents }); - this.updateLintingMessageAccessibility = debounce((annotations) => { - this.props.clearLintMessage(); - annotations.forEach((x) => { - if (x.from.line > -1) { - this.props.updateLintMessage(x.severity, x.from.line + 1, x.message); - } - }); - if (this.props.lintMessages.length > 0 && this.props.lintWarning) { - this.beep.play(); + const updateLintingMessageAccessibility = debounce((annotations) => { + clearLintMessage(); + annotations.forEach((x) => { + if (x.from.line > -1) { + updateLintMessage(x.severity, x.from.line + 1, x.message); } - }, 2000); - this.showFind = this.showFind.bind(this); - this.showReplace = this.showReplace.bind(this); - this.getContent = this.getContent.bind(this); - } - - componentDidMount() { - this.beep = new Audio(beepUrl); - this.initializeDocuments(this.props.files); - - this._cm = setupCodeMirror( - this.codemirrorContainer, - this.props, - (annotations) => { - this.updateLintingMessageAccessibility(annotations); + }); + if (lintMessages.length > 0 && lintWarning) { + beep.play(); + } + }, 2000); + + const getContent = () => { + const content = cm.current.getValue(); + const updatedFile = Object.assign({}, file, { content }); + return updatedFile; + }; + + const showFind = () => { + cm.current.execCommand('findPersistent'); + }; + + const showReplace = () => { + cm.current.execCommand('replace'); + }; + + const initializeDocuments = () => { + docs.current = {}; + files.forEach((currentFile) => { + if (currentFile.name !== 'root') { + docs.current[currentFile.id] = CodeMirror.Doc( + currentFile.content, + getFileMode(currentFile.name) + ); + } + }); + }; + + // Component did mount + const onContainerMounted = useCallback((containerNode) => { + beep.current = new Audio(beepUrl); + + // We need to do this first so docs exists. + initializeDocuments(); + + cm.current = setupCodeMirror( + containerNode, + { + theme, + lineNumbers, + linewrap, + autocloseBracketsQuotes, + setUnsavedChanges, + hideRuntimeErrorWarning, + updateFileContent, + file, + autorefresh, + isPlaying, + clearConsole, + startSketch, + autocompleteHinter, + fontSize }, - this._docs, - (lineNumber) => this.setState({ currentLine: lineNumber }) + updateLintingMessageAccessibility, + docs.current, + setCurrentLine ); + }, []); - this.props.provideController({ - tidyCode: () => tidyCode(this._cm), - showFind: this.showFind, - showReplace: this.showReplace, - getContent: this.getContent + // Component did mount + useEffect(() => { + provideController({ + tidyCode: () => tidyCode(cm.current), + showFind, + showReplace, + getContent }); - } - componentWillUpdate(nextProps) { - // check if files have changed - if (this.props.files[0].id !== nextProps.files[0].id) { - // then need to make CodeMirror documents - this.initializeDocuments(nextProps.files); - } - if (this.props.files.length !== nextProps.files.length) { - this.initializeDocuments(nextProps.files); - } - } - - componentDidUpdate(prevProps) { - if (this.props.file.id !== prevProps.file.id) { - const fileMode = getFileMode(this.props.file.name); - if (fileMode === 'javascript') { - // Define the new Emmet configuration based on the file mode - const emmetConfig = { - preview: ['html'], - markTagPairs: false, - autoRenameTags: true - }; - this._cm.setOption('emmet', emmetConfig); - } - const oldDoc = this._cm.swapDoc(this._docs[this.props.file.id]); - this._docs[prevProps.file.id] = oldDoc; - this._cm.focus(); + return () => { + provideController(null); + // if (cm.current) { + // cm.current.off('keyup', this.handleKeyUp); + // } + }; + }, []); - if (!prevProps.unsavedChanges) { - setTimeout(() => this.props.setUnsavedChanges(false), 400); - } - } - if (this.props.fontSize !== prevProps.fontSize) { - this._cm.getWrapperElement().style[ - 'font-size' - ] = `${this.props.fontSize}px`; - } - if (this.props.linewrap !== prevProps.linewrap) { - this._cm.setOption('lineWrapping', this.props.linewrap); - } - if (this.props.theme !== prevProps.theme) { - this._cm.setOption('theme', `p5-${this.props.theme}`); + useEffect(() => { + initializeDocuments(); + }, [files]); + + useEffect(() => { + const fileMode = getFileMode(file.name); + if (fileMode === 'javascript') { + // Define the new Emmet configuration based on the file mode + const emmetConfig = { + preview: ['html'], + markTagPairs: false, + autoRenameTags: true + }; + cm.current.setOption('emmet', emmetConfig); } - if (this.props.lineNumbers !== prevProps.lineNumbers) { - this._cm.setOption('lineNumbers', this.props.lineNumbers); + const oldDoc = cm.current.swapDoc(docs.current[file.id]); + if (previous?.file) { + docs.current[previous.file.id] = oldDoc; } - if ( - this.props.autocloseBracketsQuotes !== prevProps.autocloseBracketsQuotes - ) { - this._cm.setOption( - 'autoCloseBrackets', - this.props.autocloseBracketsQuotes - ); + cm.current.focus(); + + if (!previous?.unsavedChanges) { + setTimeout(() => setUnsavedChanges(false), 400); } - if (this.props.autocompleteHinter !== prevProps.autocompleteHinter) { - if (!this.props.autocompleteHinter) { - // close the hinter window once the preference is turned off - hideHinter(this._cm); - } + + for (let i = 0; i < cm.current.lineCount(); i += 1) { + cm.current.removeLineClass(i, 'background', 'line-runtime-error'); } - if (this.props.runtimeErrorWarningVisible) { - if (this.props.consoleEvents.length !== prevProps.consoleEvents.length) { - this.props.consoleEvents.forEach((consoleEvent) => { + // I think we only need to re-provide this if the content changes? idk + // TODO(connie) - Revisit the logic here + provideController({ + tidyCode: () => tidyCode(cm.current), + showFind, + showReplace, + getContent + }); + }, [file]); + + /** Move this to CM file */ + useEffect(() => { + cm.current.getWrapperElement().style['font-size'] = `${fontSize}px`; + }, [fontSize]); + useEffect(() => { + cm.current.setOption('lineWrapping', linewrap); + }, [linewrap]); + useEffect(() => { + cm.current.setOption('theme', `p5-${theme}`); + }, [theme]); + useEffect(() => { + cm.current.setOption('lineNumbers', lineNumbers); + }, [lineNumbers]); + useEffect(() => { + cm.current.setOption('autoCloseBrackets', autocloseBracketsQuotes); + }, [autocloseBracketsQuotes]); + /** end move to CM file */ + + useEffect(() => { + // close the hinter window once the preference is turned off + if (!autocompleteHinter) hideHinter(cm.current); + }, [autocompleteHinter]); + + // TODO: Should this be watching more deps? + useEffect(() => { + if (runtimeErrorWarningVisible) { + if (previous && consoleEvents.length !== previous.consoleEvents.length) { + consoleEvents.forEach((consoleEvent) => { if (consoleEvent.method === 'error') { // It doesn't work if you create a new Error, but this works // LOL const errorObj = { stack: consoleEvent.data[0].toString() }; StackTrace.fromError(errorObj).then((stackLines) => { - this.props.expandConsole(); + expandConsole(); const line = stackLines.find( (l) => l.fileName && l.fileName.startsWith('/') ); @@ -164,11 +247,11 @@ class Editor extends React.Component { const fileNameArray = line.fileName.split('/'); const fileName = fileNameArray.slice(-1)[0]; const filePath = fileNameArray.slice(0, -1).join('/'); - const fileWithError = this.props.files.find( + const fileWithError = files.find( (f) => f.name === fileName && f.filePath === filePath ); - this.props.setSelectedFile(fileWithError.id); - this._cm.addLineClass( + setSelectedFile(fileWithError.id); + cm.current.addLineClass( line.lineNumber - 1, 'background', 'line-runtime-error' @@ -177,156 +260,89 @@ class Editor extends React.Component { } }); } else { - for (let i = 0; i < this._cm.lineCount(); i += 1) { - this._cm.removeLineClass(i, 'background', 'line-runtime-error'); + for (let i = 0; i < cm.current.lineCount(); i += 1) { + cm.current.removeLineClass(i, 'background', 'line-runtime-error'); } } } + }, [consoleEvents, runtimeErrorWarningVisible]); - if (this.props.file.id !== prevProps.file.id) { - for (let i = 0; i < this._cm.lineCount(); i += 1) { - this._cm.removeLineClass(i, 'background', 'line-runtime-error'); - } - } - - this.props.provideController({ - tidyCode: () => tidyCode(this._cm), - showFind: this.showFind, - showReplace: this.showReplace, - getContent: this.getContent - }); - } + const editorSectionClass = classNames({ + editor: true, + 'sidebar--contracted': !isExpanded + }); - componentWillUnmount() { - if (this._cm) { - this._cm.off('keyup', this.handleKeyUp); - } - this.props.provideController(null); - } - - getContent() { - const content = this._cm.getValue(); - const updatedFile = Object.assign({}, this.props.file, { content }); - return updatedFile; - } - - showFind() { - this._cm.execCommand('findPersistent'); - } - - showReplace() { - this._cm.execCommand('replace'); - } - - initializeDocuments(files) { - this._docs = {}; - files.forEach((file) => { - if (file.name !== 'root') { - this._docs[file.id] = CodeMirror.Doc( - file.content, - getFileMode(file.name) - ); // eslint-disable-line - } - }); - } - - render() { - const editorSectionClass = classNames({ - editor: true, - 'sidebar--contracted': !this.props.isExpanded - }); + const editorHolderClass = classNames({ + 'editor-holder': true, + 'editor-holder--hidden': file.fileType === 'folder' || file.url + }); - const editorHolderClass = classNames({ - 'editor-holder': true, - 'editor-holder--hidden': - this.props.file.fileType === 'folder' || this.props.file.url - }); - - const { currentLine } = this.state; - - return ( - <MediaQuery minWidth={770}> - {(matches) => - matches ? ( - <section className={editorSectionClass}> - <div className="editor__header"> - <button - aria-label={this.props.t('Editor.OpenSketchARIA')} - className="sidebar__contract" - onClick={() => { - this.props.collapseSidebar(); - this.props.closeProjectOptions(); - }} - > - <LeftArrowIcon focusable="false" aria-hidden="true" /> - </button> - <button - aria-label={this.props.t('Editor.CloseSketchARIA')} - className="sidebar__expand" - onClick={this.props.expandSidebar} - > - <RightArrowIcon focusable="false" aria-hidden="true" /> - </button> - <div className="editor__file-name"> - <span> - {this.props.file.name} - <UnsavedChangesIndicator /> - </span> - <Timer /> - </div> + return ( + <MediaQuery minWidth={770}> + {(matches) => + matches ? ( + <section className={editorSectionClass}> + <div className="editor__header"> + <button + aria-label={t('Editor.OpenSketchARIA')} + className="sidebar__contract" + onClick={() => { + collapseSidebar(); + closeProjectOptions(); + }} + > + <LeftArrowIcon focusable="false" aria-hidden="true" /> + </button> + <button + aria-label={t('Editor.CloseSketchARIA')} + className="sidebar__expand" + onClick={expandSidebar} + > + <RightArrowIcon focusable="false" aria-hidden="true" /> + </button> + <div className="editor__file-name"> + <span> + {file.name} + <UnsavedChangesIndicator /> + </span> + <Timer /> </div> - <article + </div> + <article ref={onContainerMounted} className={editorHolderClass} /> + {file.url ? <AssetPreview url={file.url} name={file.name} /> : null} + <EditorAccessibility + lintMessages={lintMessages} + currentLine={currentLine} + /> + </section> + ) : ( + <EditorContainer expanded={isExpanded}> + <div> + <IconButton onClick={expandSidebar} icon={FolderIcon} /> + <span> + {file.name} + <UnsavedChangesIndicator /> + </span> + </div> + <section> + <EditorHolder ref={(element) => { this.codemirrorContainer = element; }} - className={editorHolderClass} /> - {this.props.file.url ? ( - <AssetPreview - url={this.props.file.url} - name={this.props.file.name} - /> + {file.url ? ( + <AssetPreview url={file.url} name={file.name} /> ) : null} <EditorAccessibility - lintMessages={this.props.lintMessages} + lintMessages={lintMessages} currentLine={currentLine} /> </section> - ) : ( - <EditorContainer expanded={this.props.isExpanded}> - <div> - <IconButton - onClick={this.props.expandSidebar} - icon={FolderIcon} - /> - <span> - {this.props.file.name} - <UnsavedChangesIndicator /> - </span> - </div> - <section> - <EditorHolder - ref={(element) => { - this.codemirrorContainer = element; - }} - /> - {this.props.file.url ? ( - <AssetPreview - url={this.props.file.url} - name={this.props.file.name} - /> - ) : null} - <EditorAccessibility - lintMessages={this.props.lintMessages} - currentLine={currentLine} - /> - </section> - </EditorContainer> - ) - } - </MediaQuery> - ); - } + </EditorContainer> + ) + } + </MediaQuery> + ); } Editor.propTypes = { @@ -352,7 +368,7 @@ Editor.propTypes = { ).isRequired, updateLintMessage: PropTypes.func.isRequired, clearLintMessage: PropTypes.func.isRequired, - // updateFileContent: PropTypes.func.isRequired, + updateFileContent: PropTypes.func.isRequired, fontSize: PropTypes.number.isRequired, file: PropTypes.shape({ name: PropTypes.string.isRequired, @@ -362,9 +378,9 @@ Editor.propTypes = { url: PropTypes.string }).isRequired, setUnsavedChanges: PropTypes.func.isRequired, - // startSketch: PropTypes.func.isRequired, - // autorefresh: PropTypes.bool.isRequired, - // isPlaying: PropTypes.bool.isRequired, + startSketch: PropTypes.func.isRequired, + autorefresh: PropTypes.bool.isRequired, + isPlaying: PropTypes.bool.isRequired, theme: PropTypes.string.isRequired, unsavedChanges: PropTypes.bool.isRequired, files: PropTypes.arrayOf( @@ -378,8 +394,8 @@ Editor.propTypes = { collapseSidebar: PropTypes.func.isRequired, closeProjectOptions: PropTypes.func.isRequired, expandSidebar: PropTypes.func.isRequired, - // clearConsole: PropTypes.func.isRequired, - // hideRuntimeErrorWarning: PropTypes.func.isRequired, + clearConsole: PropTypes.func.isRequired, + hideRuntimeErrorWarning: PropTypes.func.isRequired, runtimeErrorWarningVisible: PropTypes.bool.isRequired, provideController: PropTypes.func.isRequired, t: PropTypes.func.isRequired, diff --git a/client/utils/usePrevious.js b/client/utils/usePrevious.js new file mode 100644 index 0000000000..3f67c0729f --- /dev/null +++ b/client/utils/usePrevious.js @@ -0,0 +1,15 @@ +import { useEffect, useRef } from 'react'; + +/** + * Lets you do something similar to previousProps in class-based React. + * https://stackoverflow.com/questions/53446020/how-to-compare-oldvalues-and-newvalues-on-react-hooks-useeffect + */ +export default function usePrevious(value) { + const ref = useRef(); + + useEffect(() => { + // TODO: mb should just be {...}? + ref.current = JSON.parse(JSON.stringify(value)); + }, [value]); + return ref.current; +} From 55bb4a563603d541d1b4d956ecb489e4bec3d379 Mon Sep 17 00:00:00 2001 From: Connie Ye <constanceye1316@gmail.com> Date: Thu, 6 Mar 2025 23:25:45 -0800 Subject: [PATCH 05/14] change CM fifle to a useCodeMirror custom hook --- .../IDE/components/Editor/codemirror.js | 311 +++++++++--------- .../modules/IDE/components/Editor/index.jsx | 123 +++---- 2 files changed, 206 insertions(+), 228 deletions(-) diff --git a/client/modules/IDE/components/Editor/codemirror.js b/client/modules/IDE/components/Editor/codemirror.js index 6ef2056391..69842f6915 100644 --- a/client/modules/IDE/components/Editor/codemirror.js +++ b/client/modules/IDE/components/Editor/codemirror.js @@ -1,3 +1,4 @@ +import { useRef, useEffect } from 'react'; import CodeMirror from 'codemirror'; import 'codemirror/mode/css/css'; import 'codemirror/mode/clike/clike'; @@ -34,171 +35,181 @@ const INDENTATION_AMOUNT = 2; emmet(CodeMirror); -function setupCodeMirrorHooks( - cmInstance, - { - setUnsavedChanges, - hideRuntimeErrorWarning, - updateFileContent, - file, - autorefresh, - isPlaying, - clearConsole, - startSketch, - autocompleteHinter, - fontSize - }, - updateLineNumber -) { - cmInstance.on( - 'change', - debounce(() => { - setUnsavedChanges(true); - hideRuntimeErrorWarning(); - updateFileContent(file.id, cmInstance.getValue()); - if (autorefresh && isPlaying) { - clearConsole(); - startSketch(); - } - }, 1000) - ); +export default function useCodeMirror({ + theme, + lineNumbers, + linewrap, + autocloseBracketsQuotes, + setUnsavedChanges, + setCurrentLine, + hideRuntimeErrorWarning, + updateFileContent, + file, + autorefresh, + isPlaying, + clearConsole, + startSketch, + autocompleteHinter, + fontSize, + onUpdateLinting +}) { + const cmInstance = useRef(); - cmInstance.on('keyup', () => { - const lineNumber = parseInt(cmInstance.getCursor().line + 1, 10); - updateLineNumber(lineNumber); - }); + function onKeyUp() { + const lineNumber = parseInt(cmInstance.current.getCursor().line + 1, 10); + setCurrentLine(lineNumber); + } - cmInstance.on('keydown', (_cm, e) => { + function onKeyDown(_cm, e) { // Show hint - const mode = cmInstance.getOption('mode'); + const mode = cmInstance.current.getOption('mode'); if (/^[a-z]$/i.test(e.key) && (mode === 'css' || mode === 'javascript')) { showHint(_cm, autocompleteHinter, fontSize); } if (e.key === 'Escape') { e.preventDefault(); - const selections = cmInstance.listSelections(); + const selections = cmInstance.current.listSelections(); if (selections.length > 1) { const firstPos = selections[0].head || selections[0].anchor; - cmInstance.setSelection(firstPos); - cmInstance.scrollIntoView(firstPos); + cmInstance.current.setSelection(firstPos); + cmInstance.current.scrollIntoView(firstPos); } else { - cmInstance.getInputField().blur(); + cmInstance.current.getInputField().blur(); } } - }); + } - cmInstance.getWrapperElement().style['font-size'] = `${fontSize}px`; -} - -export default function setupCodeMirror( - container, - { - theme, - lineNumbers, - linewrap, - autocloseBracketsQuotes, - setUnsavedChanges, - hideRuntimeErrorWarning, - updateFileContent, - file, - autorefresh, - isPlaying, - clearConsole, - startSketch, - autocompleteHinter, - fontSize - }, - onUpdateLinting, - docs, - updateLineNumber -) { - const cm = CodeMirror(container, { - theme: `p5-${theme}`, - lineNumbers, - styleActiveLine: true, - inputStyle: 'contenteditable', - lineWrapping: linewrap, - fixedGutter: false, - foldGutter: true, - foldOptions: { widget: '\u2026' }, - gutters: ['CodeMirror-foldgutter', 'CodeMirror-lint-markers'], - keyMap: 'sublime', - highlightSelectionMatches: true, // highlight current search match - matchBrackets: true, - emmet: { - preview: ['html'], - markTagPairs: true, - autoRenameTags: true - }, - autoCloseBrackets: autocloseBracketsQuotes, - styleSelectedText: true, - lint: { - onUpdateLinting, - options: { - asi: true, - eqeqeq: false, - '-W041': false, - esversion: 11 + function onChange() { + debounce(() => { + setUnsavedChanges(true); + hideRuntimeErrorWarning(); + updateFileContent(file.id, cmInstance.current.getValue()); + if (autorefresh && isPlaying) { + clearConsole(); + startSketch(); } - }, - colorpicker: { - type: 'sketch', - mode: 'edit' - } - }); - - delete cm.options.lint.options.errors; - - const replaceCommand = - metaKey === 'Ctrl' ? `${metaKey}-H` : `${metaKey}-Option-F`; - cm.setOption('extraKeys', { - Tab: (tabCm) => { - if (!tabCm.execCommand('emmetExpandAbbreviation')) return; - // might need to specify and indent more? - const selection = tabCm.doc.getSelection(); - if (selection.length > 0) { - tabCm.execCommand('indentMore'); - } else { - tabCm.replaceSelection(' '.repeat(INDENTATION_AMOUNT)); + }, 1000); + } + + function setupCodeMirrorOnContainerMounted(container) { + cmInstance.current = CodeMirror(container, { + theme: `p5-${theme}`, + lineNumbers, + styleActiveLine: true, + inputStyle: 'contenteditable', + lineWrapping: linewrap, + fixedGutter: false, + foldGutter: true, + foldOptions: { widget: '\u2026' }, + gutters: ['CodeMirror-foldgutter', 'CodeMirror-lint-markers'], + keyMap: 'sublime', + highlightSelectionMatches: true, // highlight current search match + matchBrackets: true, + emmet: { + preview: ['html'], + markTagPairs: true, + autoRenameTags: true + }, + autoCloseBrackets: autocloseBracketsQuotes, + styleSelectedText: true, + lint: { + onUpdateLinting, + options: { + asi: true, + eqeqeq: false, + '-W041': false, + esversion: 11 + } + }, + colorpicker: { + type: 'sketch', + mode: 'edit' } - }, - Enter: 'emmetInsertLineBreak', - Esc: 'emmetResetAbbreviation', - [`Shift-Tab`]: false, - [`${metaKey}-Enter`]: () => null, - [`Shift-${metaKey}-Enter`]: () => null, - [`${metaKey}-F`]: 'findPersistent', - [`Shift-${metaKey}-F`]: () => tidyCode(cm), - [`${metaKey}-G`]: 'findPersistentNext', - [`Shift-${metaKey}-G`]: 'findPersistentPrev', - [replaceCommand]: 'replace', - // Cassie Tarakajian: If you don't set a default color, then when you - // choose a color, it deletes characters inline. This is a - // hack to prevent that. - [`${metaKey}-K`]: (metaCm, event) => - metaCm.state.colorpicker.popup_color_picker({ length: 0 }), - [`${metaKey}-.`]: 'toggleComment' // Note: most adblockers use the shortcut ctrl+. - }); - - setupCodeMirrorHooks( - cm, - { - setUnsavedChanges, - hideRuntimeErrorWarning, - updateFileContent, - file, - autorefresh, - isPlaying, - clearConsole, - startSketch, - autocompleteHinter, - fontSize - }, - updateLineNumber - ); - - cm.swapDoc(docs[file.id]); - - return cm; + }); + + delete cmInstance.current.options.lint.options.errors; + + const replaceCommand = + metaKey === 'Ctrl' ? `${metaKey}-H` : `${metaKey}-Option-F`; + cmInstance.current.setOption('extraKeys', { + Tab: (tabCm) => { + if (!tabCm.execCommand('emmetExpandAbbreviation')) return; + // might need to specify and indent more? + const selection = tabCm.doc.getSelection(); + if (selection.length > 0) { + tabCm.execCommand('indentMore'); + } else { + tabCm.replaceSelection(' '.repeat(INDENTATION_AMOUNT)); + } + }, + Enter: 'emmetInsertLineBreak', + Esc: 'emmetResetAbbreviation', + [`Shift-Tab`]: false, + [`${metaKey}-Enter`]: () => null, + [`Shift-${metaKey}-Enter`]: () => null, + [`${metaKey}-F`]: 'findPersistent', + [`Shift-${metaKey}-F`]: () => tidyCode(cmInstance.current), + [`${metaKey}-G`]: 'findPersistentNext', + [`Shift-${metaKey}-G`]: 'findPersistentPrev', + [replaceCommand]: 'replace', + // Cassie Tarakajian: If you don't set a default color, then when you + // choose a color, it deletes characters inline. This is a + // hack to prevent that. + [`${metaKey}-K`]: (metaCm, event) => + metaCm.state.colorpicker.popup_color_picker({ length: 0 }), + [`${metaKey}-.`]: 'toggleComment' // Note: most adblockers use the shortcut ctrl+. + }); + + cmInstance.current.on('change', onChange); + cmInstance.current.on('keyup', onKeyUp); + cmInstance.current.on('keydown', onKeyDown); + + cmInstance.current.getWrapperElement().style['font-size'] = `${fontSize}px`; + } + + useEffect(() => { + cmInstance.current.getWrapperElement().style['font-size'] = `${fontSize}px`; + }, [fontSize]); + useEffect(() => { + cmInstance.current.setOption('lineWrapping', linewrap); + }, [linewrap]); + useEffect(() => { + cmInstance.current.setOption('theme', `p5-${theme}`); + }, [theme]); + useEffect(() => { + cmInstance.current.setOption('lineNumbers', lineNumbers); + }, [lineNumbers]); + useEffect(() => { + cmInstance.current.setOption('autoCloseBrackets', autocloseBracketsQuotes); + }, [autocloseBracketsQuotes]); + + function teardownCodeMirror() { + cmInstance.current.off('keyup', onKeyUp); + cmInstance.current.off('change', onChange); + cmInstance.current.off('keydown', onKeyDown); + } + + const getContent = () => { + const content = cmInstance.current.getValue(); + const updatedFile = Object.assign({}, file, { content }); + return updatedFile; + }; + + const showFind = () => { + cmInstance.current.execCommand('findPersistent'); + }; + + const showReplace = () => { + cmInstance.current.execCommand('replace'); + }; + + return { + setupCodeMirrorOnContainerMounted, + teardownCodeMirror, + cmInstance, + getContent, + showFind, + showReplace + }; } diff --git a/client/modules/IDE/components/Editor/index.jsx b/client/modules/IDE/components/Editor/index.jsx index 7e00bc345e..3f6291bdb2 100644 --- a/client/modules/IDE/components/Editor/index.jsx +++ b/client/modules/IDE/components/Editor/index.jsx @@ -1,5 +1,3 @@ -// TODO: convert to functional component - import PropTypes from 'prop-types'; import React, { useCallback, useEffect, useRef, useState } from 'react'; import CodeMirror from 'codemirror'; @@ -40,7 +38,7 @@ import IconButton from '../../../../common/IconButton'; import { hideHinter } from './hinter'; import getFileMode from './utils'; import tidyCode from './tidier'; -import setupCodeMirror from './codemirror'; +import useCodeMirror from './codemirror'; import usePrevious from '../../../../utils/usePrevious'; function Editor({ @@ -76,7 +74,6 @@ function Editor({ expandSidebar }) { const [currentLine, setCurrentLine] = useState(1); - const cm = useRef(); const beep = useRef(); const docs = useRef(); const previous = usePrevious({ file, unsavedChanges, consoleEvents }); @@ -93,19 +90,31 @@ function Editor({ } }, 2000); - const getContent = () => { - const content = cm.current.getValue(); - const updatedFile = Object.assign({}, file, { content }); - return updatedFile; - }; - - const showFind = () => { - cm.current.execCommand('findPersistent'); - }; - - const showReplace = () => { - cm.current.execCommand('replace'); - }; + const { + setupCodeMirrorOnContainerMounted, + teardownCodeMirror, + cmInstance, + getContent, + showFind, + showReplace + } = useCodeMirror({ + theme, + lineNumbers, + linewrap, + autocloseBracketsQuotes, + setUnsavedChanges, + hideRuntimeErrorWarning, + updateFileContent, + file, + autorefresh, + isPlaying, + clearConsole, + startSketch, + autocompleteHinter, + fontSize, + updateLintingMessageAccessibility, + setCurrentLine + }); const initializeDocuments = () => { docs.current = {}; @@ -120,40 +129,14 @@ function Editor({ }; // Component did mount - const onContainerMounted = useCallback((containerNode) => { - beep.current = new Audio(beepUrl); - - // We need to do this first so docs exists. - initializeDocuments(); - - cm.current = setupCodeMirror( - containerNode, - { - theme, - lineNumbers, - linewrap, - autocloseBracketsQuotes, - setUnsavedChanges, - hideRuntimeErrorWarning, - updateFileContent, - file, - autorefresh, - isPlaying, - clearConsole, - startSketch, - autocompleteHinter, - fontSize - }, - updateLintingMessageAccessibility, - docs.current, - setCurrentLine - ); - }, []); + const onContainerMounted = useCallback(setupCodeMirrorOnContainerMounted, []); // Component did mount useEffect(() => { + beep.current = new Audio(beepUrl); + provideController({ - tidyCode: () => tidyCode(cm.current), + tidyCode: () => tidyCode(cmInstance.current), showFind, showReplace, getContent @@ -161,9 +144,7 @@ function Editor({ return () => { provideController(null); - // if (cm.current) { - // cm.current.off('keyup', this.handleKeyUp); - // } + teardownCodeMirror(); }; }, []); @@ -180,53 +161,35 @@ function Editor({ markTagPairs: false, autoRenameTags: true }; - cm.current.setOption('emmet', emmetConfig); + cmInstance.current.setOption('emmet', emmetConfig); } - const oldDoc = cm.current.swapDoc(docs.current[file.id]); + const oldDoc = cmInstance.current.swapDoc(docs.current[file.id]); if (previous?.file) { docs.current[previous.file.id] = oldDoc; } - cm.current.focus(); + cmInstance.current.focus(); if (!previous?.unsavedChanges) { setTimeout(() => setUnsavedChanges(false), 400); } - for (let i = 0; i < cm.current.lineCount(); i += 1) { - cm.current.removeLineClass(i, 'background', 'line-runtime-error'); + for (let i = 0; i < cmInstance.current.lineCount(); i += 1) { + cmInstance.current.removeLineClass(i, 'background', 'line-runtime-error'); } // I think we only need to re-provide this if the content changes? idk // TODO(connie) - Revisit the logic here provideController({ - tidyCode: () => tidyCode(cm.current), + tidyCode: () => tidyCode(cmInstance.current), showFind, showReplace, getContent }); }, [file]); - /** Move this to CM file */ - useEffect(() => { - cm.current.getWrapperElement().style['font-size'] = `${fontSize}px`; - }, [fontSize]); - useEffect(() => { - cm.current.setOption('lineWrapping', linewrap); - }, [linewrap]); - useEffect(() => { - cm.current.setOption('theme', `p5-${theme}`); - }, [theme]); - useEffect(() => { - cm.current.setOption('lineNumbers', lineNumbers); - }, [lineNumbers]); - useEffect(() => { - cm.current.setOption('autoCloseBrackets', autocloseBracketsQuotes); - }, [autocloseBracketsQuotes]); - /** end move to CM file */ - useEffect(() => { // close the hinter window once the preference is turned off - if (!autocompleteHinter) hideHinter(cm.current); + if (!autocompleteHinter) hideHinter(cmInstance.current); }, [autocompleteHinter]); // TODO: Should this be watching more deps? @@ -251,7 +214,7 @@ function Editor({ (f) => f.name === fileName && f.filePath === filePath ); setSelectedFile(fileWithError.id); - cm.current.addLineClass( + cmInstance.current.addLineClass( line.lineNumber - 1, 'background', 'line-runtime-error' @@ -260,8 +223,12 @@ function Editor({ } }); } else { - for (let i = 0; i < cm.current.lineCount(); i += 1) { - cm.current.removeLineClass(i, 'background', 'line-runtime-error'); + for (let i = 0; i < cmInstance.current.lineCount(); i += 1) { + cmInstance.current.removeLineClass( + i, + 'background', + 'line-runtime-error' + ); } } } From 7c822b11214443cb74d1728d5226520e02dd07a9 Mon Sep 17 00:00:00 2001 From: Connie Ye <constanceye1316@gmail.com> Date: Sun, 23 Mar 2025 19:25:52 +0000 Subject: [PATCH 06/14] fix codemirror file debounce bug --- .../IDE/components/Editor/codemirror.js | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/client/modules/IDE/components/Editor/codemirror.js b/client/modules/IDE/components/Editor/codemirror.js index 69842f6915..7c8e9491e4 100644 --- a/client/modules/IDE/components/Editor/codemirror.js +++ b/client/modules/IDE/components/Editor/codemirror.js @@ -56,11 +56,13 @@ export default function useCodeMirror({ const cmInstance = useRef(); function onKeyUp() { + console.log('keyup'); const lineNumber = parseInt(cmInstance.current.getCursor().line + 1, 10); setCurrentLine(lineNumber); } function onKeyDown(_cm, e) { + console.log('keydown'); // Show hint const mode = cmInstance.current.getOption('mode'); if (/^[a-z]$/i.test(e.key) && (mode === 'css' || mode === 'javascript')) { @@ -81,16 +83,16 @@ export default function useCodeMirror({ } function onChange() { - debounce(() => { - setUnsavedChanges(true); - hideRuntimeErrorWarning(); - updateFileContent(file.id, cmInstance.current.getValue()); - if (autorefresh && isPlaying) { - clearConsole(); - startSketch(); - } - }, 1000); + console.log('change'); + setUnsavedChanges(true); + hideRuntimeErrorWarning(); + updateFileContent(file.id, cmInstance.current.getValue()); + if (autorefresh && isPlaying) { + clearConsole(); + startSketch(); + } } + const debouncedOnChange = debounce(onChange, 1000); function setupCodeMirrorOnContainerMounted(container) { cmInstance.current = CodeMirror(container, { @@ -161,7 +163,8 @@ export default function useCodeMirror({ [`${metaKey}-.`]: 'toggleComment' // Note: most adblockers use the shortcut ctrl+. }); - cmInstance.current.on('change', onChange); + console.log('setting up change handlers??', cmInstance.current); + cmInstance.current.on('change', debouncedOnChange); cmInstance.current.on('keyup', onKeyUp); cmInstance.current.on('keydown', onKeyDown); From 1e575f90c9ee1dca4c5ef1a66d2a1060bd4c3fd3 Mon Sep 17 00:00:00 2001 From: Connie Ye <constanceye1316@gmail.com> Date: Sun, 23 Mar 2025 20:05:48 +0000 Subject: [PATCH 07/14] fix codemirrorr bug with debouncer again --- client/modules/IDE/components/Editor/codemirror.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/client/modules/IDE/components/Editor/codemirror.js b/client/modules/IDE/components/Editor/codemirror.js index 7c8e9491e4..c623b719f7 100644 --- a/client/modules/IDE/components/Editor/codemirror.js +++ b/client/modules/IDE/components/Editor/codemirror.js @@ -56,13 +56,11 @@ export default function useCodeMirror({ const cmInstance = useRef(); function onKeyUp() { - console.log('keyup'); const lineNumber = parseInt(cmInstance.current.getCursor().line + 1, 10); setCurrentLine(lineNumber); } function onKeyDown(_cm, e) { - console.log('keydown'); // Show hint const mode = cmInstance.current.getOption('mode'); if (/^[a-z]$/i.test(e.key) && (mode === 'css' || mode === 'javascript')) { @@ -82,11 +80,15 @@ export default function useCodeMirror({ } } + // We have to create a ref for the file ID, or else the debouncer + // will old onto an old version of the fileId and just overrwrite the initial file. + const fileId = useRef(); + fileId.current = file.id; + function onChange() { - console.log('change'); setUnsavedChanges(true); hideRuntimeErrorWarning(); - updateFileContent(file.id, cmInstance.current.getValue()); + updateFileContent(fileId.current, cmInstance.current.getValue()); if (autorefresh && isPlaying) { clearConsole(); startSketch(); @@ -189,7 +191,7 @@ export default function useCodeMirror({ function teardownCodeMirror() { cmInstance.current.off('keyup', onKeyUp); - cmInstance.current.off('change', onChange); + cmInstance.current.off('change', debouncedOnChange); cmInstance.current.off('keydown', onKeyDown); } From 107e9bf21a737d7488821ca1e29708e6aaf77739 Mon Sep 17 00:00:00 2001 From: Connie Ye <constanceye1316@gmail.com> Date: Sun, 23 Mar 2025 20:11:38 +0000 Subject: [PATCH 08/14] only useeffect on file id change --- client/modules/IDE/components/Editor/index.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/modules/IDE/components/Editor/index.jsx b/client/modules/IDE/components/Editor/index.jsx index 3f6291bdb2..d27b0edf48 100644 --- a/client/modules/IDE/components/Editor/index.jsx +++ b/client/modules/IDE/components/Editor/index.jsx @@ -185,7 +185,7 @@ function Editor({ showReplace, getContent }); - }, [file]); + }, [file.id]); useEffect(() => { // close the hinter window once the preference is turned off From e45d2654f7f9d18c6c1fca2f5433665d2656e4cc Mon Sep 17 00:00:00 2001 From: Connie Ye <constanceye1316@gmail.com> Date: Sun, 23 Mar 2025 20:29:41 +0000 Subject: [PATCH 09/14] switches from using usePrevious to an existing hook --- .../modules/IDE/components/Editor/index.jsx | 151 ++++++++++-------- client/utils/usePrevious.js | 15 -- 2 files changed, 80 insertions(+), 86 deletions(-) delete mode 100644 client/utils/usePrevious.js diff --git a/client/modules/IDE/components/Editor/index.jsx b/client/modules/IDE/components/Editor/index.jsx index d27b0edf48..2c29abbf88 100644 --- a/client/modules/IDE/components/Editor/index.jsx +++ b/client/modules/IDE/components/Editor/index.jsx @@ -39,7 +39,7 @@ import { hideHinter } from './hinter'; import getFileMode from './utils'; import tidyCode from './tidier'; import useCodeMirror from './codemirror'; -import usePrevious from '../../../../utils/usePrevious'; +import { useEffectWithComparison } from '../../hooks/custom-hooks'; function Editor({ provideController, @@ -76,7 +76,6 @@ function Editor({ const [currentLine, setCurrentLine] = useState(1); const beep = useRef(); const docs = useRef(); - const previous = usePrevious({ file, unsavedChanges, consoleEvents }); const updateLintingMessageAccessibility = debounce((annotations) => { clearLintMessage(); @@ -116,6 +115,15 @@ function Editor({ setCurrentLine }); + // Lets the parent component access file content-specific functionality... + // TODO(connie) - Revisit the logic here, can we wrap this in useCallback or something? + provideController({ + tidyCode: () => tidyCode(cmInstance.current), + showFind, + showReplace, + getContent + }); + const initializeDocuments = () => { docs.current = {}; files.forEach((currentFile) => { @@ -152,40 +160,38 @@ function Editor({ initializeDocuments(); }, [files]); - useEffect(() => { - const fileMode = getFileMode(file.name); - if (fileMode === 'javascript') { - // Define the new Emmet configuration based on the file mode - const emmetConfig = { - preview: ['html'], - markTagPairs: false, - autoRenameTags: true - }; - cmInstance.current.setOption('emmet', emmetConfig); - } - const oldDoc = cmInstance.current.swapDoc(docs.current[file.id]); - if (previous?.file) { - docs.current[previous.file.id] = oldDoc; - } - cmInstance.current.focus(); - - if (!previous?.unsavedChanges) { - setTimeout(() => setUnsavedChanges(false), 400); - } + useEffectWithComparison( + (_, prevProps) => { + const fileMode = getFileMode(file.name); + if (fileMode === 'javascript') { + // Define the new Emmet configuration based on the file mode + const emmetConfig = { + preview: ['html'], + markTagPairs: false, + autoRenameTags: true + }; + cmInstance.current.setOption('emmet', emmetConfig); + } + const oldDoc = cmInstance.current.swapDoc(docs.current[file.id]); + if (prevProps?.file) { + docs.current[prevProps.file.id] = oldDoc; + } + cmInstance.current.focus(); - for (let i = 0; i < cmInstance.current.lineCount(); i += 1) { - cmInstance.current.removeLineClass(i, 'background', 'line-runtime-error'); - } + if (!prevProps?.unsavedChanges) { + setTimeout(() => setUnsavedChanges(false), 400); + } - // I think we only need to re-provide this if the content changes? idk - // TODO(connie) - Revisit the logic here - provideController({ - tidyCode: () => tidyCode(cmInstance.current), - showFind, - showReplace, - getContent - }); - }, [file.id]); + for (let i = 0; i < cmInstance.current.lineCount(); i += 1) { + cmInstance.current.removeLineClass( + i, + 'background', + 'line-runtime-error' + ); + } + }, + [file.id] + ); useEffect(() => { // close the hinter window once the preference is turned off @@ -193,46 +199,49 @@ function Editor({ }, [autocompleteHinter]); // TODO: Should this be watching more deps? - useEffect(() => { - if (runtimeErrorWarningVisible) { - if (previous && consoleEvents.length !== previous.consoleEvents.length) { - consoleEvents.forEach((consoleEvent) => { - if (consoleEvent.method === 'error') { - // It doesn't work if you create a new Error, but this works - // LOL - const errorObj = { stack: consoleEvent.data[0].toString() }; - StackTrace.fromError(errorObj).then((stackLines) => { - expandConsole(); - const line = stackLines.find( - (l) => l.fileName && l.fileName.startsWith('/') - ); - if (!line) return; - const fileNameArray = line.fileName.split('/'); - const fileName = fileNameArray.slice(-1)[0]; - const filePath = fileNameArray.slice(0, -1).join('/'); - const fileWithError = files.find( - (f) => f.name === fileName && f.filePath === filePath - ); - setSelectedFile(fileWithError.id); - cmInstance.current.addLineClass( - line.lineNumber - 1, - 'background', - 'line-runtime-error' - ); - }); + useEffectWithComparison( + (_, prevProps) => { + if (runtimeErrorWarningVisible) { + if (consoleEvents.length !== prevProps.consoleEvents.length) { + consoleEvents.forEach((consoleEvent) => { + if (consoleEvent.method === 'error') { + // It doesn't work if you create a new Error, but this works + // LOL + const errorObj = { stack: consoleEvent.data[0].toString() }; + StackTrace.fromError(errorObj).then((stackLines) => { + expandConsole(); + const line = stackLines.find( + (l) => l.fileName && l.fileName.startsWith('/') + ); + if (!line) return; + const fileNameArray = line.fileName.split('/'); + const fileName = fileNameArray.slice(-1)[0]; + const filePath = fileNameArray.slice(0, -1).join('/'); + const fileWithError = files.find( + (f) => f.name === fileName && f.filePath === filePath + ); + setSelectedFile(fileWithError.id); + cmInstance.current.addLineClass( + line.lineNumber - 1, + 'background', + 'line-runtime-error' + ); + }); + } + }); + } else { + for (let i = 0; i < cmInstance.current.lineCount(); i += 1) { + cmInstance.current.removeLineClass( + i, + 'background', + 'line-runtime-error' + ); } - }); - } else { - for (let i = 0; i < cmInstance.current.lineCount(); i += 1) { - cmInstance.current.removeLineClass( - i, - 'background', - 'line-runtime-error' - ); } } - } - }, [consoleEvents, runtimeErrorWarningVisible]); + }, + [consoleEvents, runtimeErrorWarningVisible] + ); const editorSectionClass = classNames({ editor: true, diff --git a/client/utils/usePrevious.js b/client/utils/usePrevious.js deleted file mode 100644 index 3f67c0729f..0000000000 --- a/client/utils/usePrevious.js +++ /dev/null @@ -1,15 +0,0 @@ -import { useEffect, useRef } from 'react'; - -/** - * Lets you do something similar to previousProps in class-based React. - * https://stackoverflow.com/questions/53446020/how-to-compare-oldvalues-and-newvalues-on-react-hooks-useeffect - */ -export default function usePrevious(value) { - const ref = useRef(); - - useEffect(() => { - // TODO: mb should just be {...}? - ref.current = JSON.parse(JSON.stringify(value)); - }, [value]); - return ref.current; -} From 0ac323432bb7c1a28f71ea08395311cfd134f9b9 Mon Sep 17 00:00:00 2001 From: Connie Ye <constanceye1316@gmail.com> Date: Sun, 23 Mar 2025 20:39:47 +0000 Subject: [PATCH 10/14] moves document management to the cm file --- .../IDE/components/Editor/codemirror.js | 53 ++++++++++++++++ .../modules/IDE/components/Editor/index.jsx | 62 ++----------------- 2 files changed, 59 insertions(+), 56 deletions(-) diff --git a/client/modules/IDE/components/Editor/codemirror.js b/client/modules/IDE/components/Editor/codemirror.js index c623b719f7..03dc463b65 100644 --- a/client/modules/IDE/components/Editor/codemirror.js +++ b/client/modules/IDE/components/Editor/codemirror.js @@ -27,9 +27,11 @@ import 'codemirror-colorpicker'; import { debounce } from 'lodash'; import emmet from '@emmetio/codemirror-plugin'; +import { useEffectWithComparison } from '../../hooks/custom-hooks'; import { metaKey } from '../../../../utils/metaKey'; import { showHint } from './hinter'; import tidyCode from './tidier'; +import getFileMode from './utils'; const INDENTATION_AMOUNT = 2; @@ -45,6 +47,7 @@ export default function useCodeMirror({ hideRuntimeErrorWarning, updateFileContent, file, + files, autorefresh, isPlaying, clearConsole, @@ -54,6 +57,7 @@ export default function useCodeMirror({ onUpdateLinting }) { const cmInstance = useRef(); + const docs = useRef(); function onKeyUp() { const lineNumber = parseInt(cmInstance.current.getCursor().line + 1, 10); @@ -189,6 +193,55 @@ export default function useCodeMirror({ cmInstance.current.setOption('autoCloseBrackets', autocloseBracketsQuotes); }, [autocloseBracketsQuotes]); + const initializeDocuments = () => { + docs.current = {}; + files.forEach((currentFile) => { + if (currentFile.name !== 'root') { + docs.current[currentFile.id] = CodeMirror.Doc( + currentFile.content, + getFileMode(currentFile.name) + ); + } + }); + }; + + useEffect(() => { + initializeDocuments(); + }, [files]); + + useEffectWithComparison( + (_, prevProps) => { + const fileMode = getFileMode(file.name); + if (fileMode === 'javascript') { + // Define the new Emmet configuration based on the file mode + const emmetConfig = { + preview: ['html'], + markTagPairs: false, + autoRenameTags: true + }; + cmInstance.current.setOption('emmet', emmetConfig); + } + const oldDoc = cmInstance.current.swapDoc(docs.current[file.id]); + if (prevProps?.file) { + docs.current[prevProps.file.id] = oldDoc; + } + cmInstance.current.focus(); + + if (!prevProps?.unsavedChanges) { + setTimeout(() => setUnsavedChanges(false), 400); + } + + for (let i = 0; i < cmInstance.current.lineCount(); i += 1) { + cmInstance.current.removeLineClass( + i, + 'background', + 'line-runtime-error' + ); + } + }, + [file.id] + ); + function teardownCodeMirror() { cmInstance.current.off('keyup', onKeyUp); cmInstance.current.off('change', debouncedOnChange); diff --git a/client/modules/IDE/components/Editor/index.jsx b/client/modules/IDE/components/Editor/index.jsx index 2c29abbf88..198dac3cea 100644 --- a/client/modules/IDE/components/Editor/index.jsx +++ b/client/modules/IDE/components/Editor/index.jsx @@ -1,6 +1,5 @@ import PropTypes from 'prop-types'; import React, { useCallback, useEffect, useRef, useState } from 'react'; -import CodeMirror from 'codemirror'; import { withTranslation } from 'react-i18next'; import StackTrace from 'stacktrace-js'; @@ -36,7 +35,6 @@ import { FolderIcon } from '../../../../common/icons'; import IconButton from '../../../../common/IconButton'; import { hideHinter } from './hinter'; -import getFileMode from './utils'; import tidyCode from './tidier'; import useCodeMirror from './codemirror'; import { useEffectWithComparison } from '../../hooks/custom-hooks'; @@ -50,7 +48,6 @@ function Editor({ lineNumbers, closeProjectOptions, setSelectedFile, - unsavedChanges, setUnsavedChanges, lintMessages, lintWarning, @@ -75,7 +72,6 @@ function Editor({ }) { const [currentLine, setCurrentLine] = useState(1); const beep = useRef(); - const docs = useRef(); const updateLintingMessageAccessibility = debounce((annotations) => { clearLintMessage(); @@ -89,6 +85,8 @@ function Editor({ } }, 2000); + // The useCodeMirror hook manages CodeMirror state and returns + // a reference to the actual CM instance. const { setupCodeMirrorOnContainerMounted, teardownCodeMirror, @@ -105,6 +103,7 @@ function Editor({ hideRuntimeErrorWarning, updateFileContent, file, + files, autorefresh, isPlaying, clearConsole, @@ -124,22 +123,11 @@ function Editor({ getContent }); - const initializeDocuments = () => { - docs.current = {}; - files.forEach((currentFile) => { - if (currentFile.name !== 'root') { - docs.current[currentFile.id] = CodeMirror.Doc( - currentFile.content, - getFileMode(currentFile.name) - ); - } - }); - }; - - // Component did mount + // When the CM container div mounts, we set up CodeMirror. const onContainerMounted = useCallback(setupCodeMirrorOnContainerMounted, []); - // Component did mount + // This is acting as a "componentDidMount" call where it runs once + // at the start and never again. It also provides a cleanup function. useEffect(() => { beep.current = new Audio(beepUrl); @@ -156,43 +144,6 @@ function Editor({ }; }, []); - useEffect(() => { - initializeDocuments(); - }, [files]); - - useEffectWithComparison( - (_, prevProps) => { - const fileMode = getFileMode(file.name); - if (fileMode === 'javascript') { - // Define the new Emmet configuration based on the file mode - const emmetConfig = { - preview: ['html'], - markTagPairs: false, - autoRenameTags: true - }; - cmInstance.current.setOption('emmet', emmetConfig); - } - const oldDoc = cmInstance.current.swapDoc(docs.current[file.id]); - if (prevProps?.file) { - docs.current[prevProps.file.id] = oldDoc; - } - cmInstance.current.focus(); - - if (!prevProps?.unsavedChanges) { - setTimeout(() => setUnsavedChanges(false), 400); - } - - for (let i = 0; i < cmInstance.current.lineCount(); i += 1) { - cmInstance.current.removeLineClass( - i, - 'background', - 'line-runtime-error' - ); - } - }, - [file.id] - ); - useEffect(() => { // close the hinter window once the preference is turned off if (!autocompleteHinter) hideHinter(cmInstance.current); @@ -358,7 +309,6 @@ Editor.propTypes = { autorefresh: PropTypes.bool.isRequired, isPlaying: PropTypes.bool.isRequired, theme: PropTypes.string.isRequired, - unsavedChanges: PropTypes.bool.isRequired, files: PropTypes.arrayOf( PropTypes.shape({ id: PropTypes.string.isRequired, From f3f7ab44196299da6c9276047d11a0e1a76379d8 Mon Sep 17 00:00:00 2001 From: Connie Ye <constanceye1316@gmail.com> Date: Mon, 24 Mar 2025 00:58:49 -0700 Subject: [PATCH 11/14] fix test error --- .../IDE/components/Editor/codemirror.js | 1 - .../modules/IDE/components/Editor/index.jsx | 22 +++++++++++-------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/client/modules/IDE/components/Editor/codemirror.js b/client/modules/IDE/components/Editor/codemirror.js index 03dc463b65..921521c5f3 100644 --- a/client/modules/IDE/components/Editor/codemirror.js +++ b/client/modules/IDE/components/Editor/codemirror.js @@ -169,7 +169,6 @@ export default function useCodeMirror({ [`${metaKey}-.`]: 'toggleComment' // Note: most adblockers use the shortcut ctrl+. }); - console.log('setting up change handlers??', cmInstance.current); cmInstance.current.on('change', debouncedOnChange); cmInstance.current.on('keyup', onKeyUp); cmInstance.current.on('keydown', onKeyDown); diff --git a/client/modules/IDE/components/Editor/index.jsx b/client/modules/IDE/components/Editor/index.jsx index 198dac3cea..0564c839af 100644 --- a/client/modules/IDE/components/Editor/index.jsx +++ b/client/modules/IDE/components/Editor/index.jsx @@ -115,13 +115,14 @@ function Editor({ }); // Lets the parent component access file content-specific functionality... - // TODO(connie) - Revisit the logic here, can we wrap this in useCallback or something? - provideController({ - tidyCode: () => tidyCode(cmInstance.current), - showFind, - showReplace, - getContent - }); + useEffect(() => { + provideController({ + tidyCode: () => tidyCode(cmInstance.current), + showFind, + showReplace, + getContent + }); + }, [showFind, showReplace, getContent]); // When the CM container div mounts, we set up CodeMirror. const onContainerMounted = useCallback(setupCodeMirrorOnContainerMounted, []); @@ -149,11 +150,14 @@ function Editor({ if (!autocompleteHinter) hideHinter(cmInstance.current); }, [autocompleteHinter]); - // TODO: Should this be watching more deps? + // TODO: test this useEffectWithComparison( (_, prevProps) => { if (runtimeErrorWarningVisible) { - if (consoleEvents.length !== prevProps.consoleEvents.length) { + if ( + prevProps.consoleEvents && + consoleEvents.length !== prevProps.consoleEvents.length + ) { consoleEvents.forEach((consoleEvent) => { if (consoleEvent.method === 'error') { // It doesn't work if you create a new Error, but this works From 6d7ef6d44adbeb4bc0124c222b1dabe6b796f47a Mon Sep 17 00:00:00 2001 From: Connie Ye <constanceye1316@gmail.com> Date: Thu, 27 Mar 2025 19:58:30 -0700 Subject: [PATCH 12/14] fix mobile bug --- client/modules/IDE/components/Editor/index.jsx | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/client/modules/IDE/components/Editor/index.jsx b/client/modules/IDE/components/Editor/index.jsx index 0564c839af..3411f893bd 100644 --- a/client/modules/IDE/components/Editor/index.jsx +++ b/client/modules/IDE/components/Editor/index.jsx @@ -256,11 +256,7 @@ function Editor({ </span> </div> <section> - <EditorHolder - ref={(element) => { - this.codemirrorContainer = element; - }} - /> + <EditorHolder ref={onContainerMounted} /> {file.url ? ( <AssetPreview url={file.url} name={file.name} /> ) : null} From ea815f7ef122945854bfb4184ea28e05ae926f82 Mon Sep 17 00:00:00 2001 From: Connie Ye <constanceye1316@gmail.com> Date: Thu, 27 Mar 2025 20:28:46 -0700 Subject: [PATCH 13/14] adds comments, removes a logic block that wasn't doing anything I'm fairly certain that we can remove ``` if (!prevProps?.unsavedChanges) { setTimeout(() => setUnsavedChanges(false), 400); } ``` I looked at the git blame and it looks like the intention was to stop setting unsavedchanges to false when the files switched, but i think the solve that was implemented 9 years ago did something like "if c == false, c = false" and we should be able to safely remove it instead. reference commit: https://github.com/processing/p5.js-web-editor/commit/77e2f5bfff4774d52adc0ec2c8dd06fd22bd7507 --- .../IDE/components/Editor/codemirror.js | 25 ++++++++++++------- .../modules/IDE/components/Editor/index.jsx | 4 +-- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/client/modules/IDE/components/Editor/codemirror.js b/client/modules/IDE/components/Editor/codemirror.js index 921521c5f3..299d7cf995 100644 --- a/client/modules/IDE/components/Editor/codemirror.js +++ b/client/modules/IDE/components/Editor/codemirror.js @@ -37,6 +37,7 @@ const INDENTATION_AMOUNT = 2; emmet(CodeMirror); +/** This is a custom React hook that manages CodeMirror state. */ export default function useCodeMirror({ theme, lineNumbers, @@ -56,7 +57,9 @@ export default function useCodeMirror({ fontSize, onUpdateLinting }) { + // The codemirror instance. const cmInstance = useRef(); + // The current codemirror files. const docs = useRef(); function onKeyUp() { @@ -89,6 +92,7 @@ export default function useCodeMirror({ const fileId = useRef(); fileId.current = file.id; + // When the file changes, update the file content and save status. function onChange() { setUnsavedChanges(true); hideRuntimeErrorWarning(); @@ -100,6 +104,8 @@ export default function useCodeMirror({ } const debouncedOnChange = debounce(onChange, 1000); + // When the container component enters the DOM, we want this function + // to be called so we can setup the CodeMirror instance with the container. function setupCodeMirrorOnContainerMounted(container) { cmInstance.current = CodeMirror(container, { theme: `p5-${theme}`, @@ -169,6 +175,7 @@ export default function useCodeMirror({ [`${metaKey}-.`]: 'toggleComment' // Note: most adblockers use the shortcut ctrl+. }); + // Setup the event listeners on the CodeMirror instance. cmInstance.current.on('change', debouncedOnChange); cmInstance.current.on('keyup', onKeyUp); cmInstance.current.on('keydown', onKeyDown); @@ -176,6 +183,7 @@ export default function useCodeMirror({ cmInstance.current.getWrapperElement().style['font-size'] = `${fontSize}px`; } + // When settings change, we pass those changes into CodeMirror. useEffect(() => { cmInstance.current.getWrapperElement().style['font-size'] = `${fontSize}px`; }, [fontSize]); @@ -192,7 +200,8 @@ export default function useCodeMirror({ cmInstance.current.setOption('autoCloseBrackets', autocloseBracketsQuotes); }, [autocloseBracketsQuotes]); - const initializeDocuments = () => { + // Initializes the files as CodeMirror documents. + function initializeDocuments() { docs.current = {}; files.forEach((currentFile) => { if (currentFile.name !== 'root') { @@ -202,12 +211,13 @@ export default function useCodeMirror({ ); } }); - }; + } - useEffect(() => { - initializeDocuments(); - }, [files]); + // When the files change, reinitialize the documents. + useEffect(initializeDocuments, [files]); + // When the file changes, we change the file mode and + // make the CodeMirror call to swap out the document. useEffectWithComparison( (_, prevProps) => { const fileMode = getFileMode(file.name); @@ -226,10 +236,6 @@ export default function useCodeMirror({ } cmInstance.current.focus(); - if (!prevProps?.unsavedChanges) { - setTimeout(() => setUnsavedChanges(false), 400); - } - for (let i = 0; i < cmInstance.current.lineCount(); i += 1) { cmInstance.current.removeLineClass( i, @@ -241,6 +247,7 @@ export default function useCodeMirror({ [file.id] ); + // Remove the CM listeners on component teardown. function teardownCodeMirror() { cmInstance.current.off('keyup', onKeyUp); cmInstance.current.off('change', debouncedOnChange); diff --git a/client/modules/IDE/components/Editor/index.jsx b/client/modules/IDE/components/Editor/index.jsx index 3411f893bd..bf1eca9882 100644 --- a/client/modules/IDE/components/Editor/index.jsx +++ b/client/modules/IDE/components/Editor/index.jsx @@ -146,11 +146,11 @@ function Editor({ }, []); useEffect(() => { - // close the hinter window once the preference is turned off + // Close the hinter window once the preference is turned off if (!autocompleteHinter) hideHinter(cmInstance.current); }, [autocompleteHinter]); - // TODO: test this + // Updates the error console. useEffectWithComparison( (_, prevProps) => { if (runtimeErrorWarningVisible) { From 36c412922ba928bb37929850bb48e072b16f5250 Mon Sep 17 00:00:00 2001 From: Connie Ye <constanceye1316@gmail.com> Date: Fri, 28 Mar 2025 09:36:49 -0700 Subject: [PATCH 14/14] add TODO --- client/modules/IDE/components/Editor/codemirror.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/client/modules/IDE/components/Editor/codemirror.js b/client/modules/IDE/components/Editor/codemirror.js index 299d7cf995..08e787724e 100644 --- a/client/modules/IDE/components/Editor/codemirror.js +++ b/client/modules/IDE/components/Editor/codemirror.js @@ -37,7 +37,10 @@ const INDENTATION_AMOUNT = 2; emmet(CodeMirror); -/** This is a custom React hook that manages CodeMirror state. */ +/** + * This is a custom React hook that manages CodeMirror state. + * TODO(Connie Ye): Revisit the linting on file switch. + */ export default function useCodeMirror({ theme, lineNumbers,