-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Converted ConsoleInput to functional components #2409
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,39 +1,33 @@ | ||
import React, { useRef, useEffect, useState } from 'react'; | ||
import PropTypes from 'prop-types'; | ||
import React from 'react'; | ||
import CodeMirror from 'codemirror'; | ||
import { Encode } from 'console-feed'; | ||
|
||
import RightArrowIcon from '../../../images/right-arrow.svg'; | ||
import { dispatchMessage, MessageTypes } from '../../../utils/dispatcher'; | ||
|
||
// heavily inspired by | ||
// https://github.com/codesandbox/codesandbox-client/blob/92a1131f4ded6f7d9c16945dc7c18aa97c8ada27/packages/app/src/app/components/Preview/DevTools/Console/Input/index.tsx | ||
const ConsoleInput = ({ theme, dispatchConsoleEvent, fontSize }) => { | ||
const [commandHistory, setCommandHistory] = useState([]); | ||
const [commandCursor, setCommandCursor] = useState(-1); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's put a |
||
|
||
class ConsoleInput extends React.Component { | ||
constructor(props) { | ||
super(props); | ||
this.state = { | ||
commandHistory: [], | ||
commandCursor: -1 | ||
}; | ||
} | ||
const codemirrorContainerRef = useRef(null); | ||
const codemirrorRef = useRef(null); | ||
|
||
componentDidMount() { | ||
this._cm = CodeMirror(this.codemirrorContainer, { | ||
// eslint-disable-line | ||
theme: `p5-${this.props.theme}`, | ||
useEffect(() => { | ||
codemirrorRef.current = CodeMirror(codemirrorContainerRef.current, { | ||
theme: `p5-${theme}`, | ||
scrollbarStyle: null, | ||
keymap: 'sublime', | ||
mode: 'javascript', | ||
inputStyle: 'contenteditable' | ||
}); | ||
|
||
this._cm.on('keydown', (cm, e) => { | ||
const handleKeyDown = (cm, e) => { | ||
if (e.key === 'Enter' && !e.shiftKey) { | ||
e.preventDefault(); | ||
e.stopPropagation(); | ||
const value = cm.getValue(); | ||
if (value.trim(' ') === '') { | ||
if (value.trim() === '') { | ||
return false; | ||
} | ||
const messages = [ | ||
|
@@ -47,92 +41,84 @@ class ConsoleInput extends React.Component { | |
messages | ||
} | ||
}); | ||
this.props.dispatchConsoleEvent(consoleEvent); | ||
dispatchConsoleEvent(consoleEvent); | ||
cm.setValue(''); | ||
this.setState((state) => ({ | ||
commandCursor: -1, | ||
commandHistory: [value, ...state.commandHistory] | ||
})); | ||
setCommandCursor(-1); | ||
setCommandHistory((prevHistory) => [value, ...prevHistory]); | ||
} else if (e.key === 'ArrowUp') { | ||
const lineNumber = this._cm.getDoc().getCursor().line; | ||
const lineNumber = cm.getDoc().getCursor().line; | ||
if (lineNumber !== 0) { | ||
return false; | ||
} | ||
|
||
this.setState((state) => { | ||
const newCursor = Math.min( | ||
state.commandCursor + 1, | ||
state.commandHistory.length - 1 | ||
); | ||
this._cm.getDoc().setValue(state.commandHistory[newCursor] || ''); | ||
const cursorPos = this._cm.getDoc().getLine(0).length - 1; | ||
this._cm.getDoc().setCursor({ line: 0, ch: cursorPos }); | ||
return { commandCursor: newCursor }; | ||
setCommandCursor((prevCursor) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Love that you are using a prevState callback! |
||
const newCursor = Math.min(prevCursor + 1, commandHistory.length - 1); | ||
cm.getDoc().setValue(commandHistory[newCursor] || ''); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems not quite right to be performing these side effects inside of a set state. But that's how it was before so let's leave it for now. |
||
const cursorPos = cm.getDoc().getLine(0).length - 1; | ||
cm.getDoc().setCursor({ line: 0, ch: cursorPos }); | ||
return newCursor; | ||
}); | ||
} else if (e.key === 'ArrowDown') { | ||
const lineNumber = this._cm.getDoc().getCursor().line; | ||
const lineCount = this._cm.getValue().split('\n').length; | ||
const lineNumber = cm.getDoc().getCursor().line; | ||
const lineCount = cm.getValue().split('\n').length; | ||
if (lineNumber + 1 !== lineCount) { | ||
return false; | ||
} | ||
|
||
this.setState((state) => { | ||
const newCursor = Math.max(state.commandCursor - 1, -1); | ||
this._cm.getDoc().setValue(state.commandHistory[newCursor] || ''); | ||
const newLineCount = this._cm.getValue().split('\n').length; | ||
const newLine = this._cm.getDoc().getLine(newLineCount); | ||
setCommandCursor((prevCursor) => { | ||
const newCursor = Math.max(prevCursor - 1, -1); | ||
cm.getDoc().setValue(commandHistory[newCursor] || ''); | ||
const newLineCount = cm.getValue().split('\n').length; | ||
const newLine = cm.getDoc().getLine(newLineCount); | ||
const cursorPos = newLine ? newLine.length - 1 : 1; | ||
this._cm.getDoc().setCursor({ line: lineCount, ch: cursorPos }); | ||
return { commandCursor: newCursor }; | ||
cm.getDoc().setCursor({ line: lineCount, ch: cursorPos }); | ||
return newCursor; | ||
}); | ||
} | ||
return true; | ||
}); | ||
}; | ||
|
||
this._cm.getWrapperElement().style[ | ||
codemirrorRef.current.on('keydown', handleKeyDown); | ||
codemirrorRef.current.getWrapperElement().style[ | ||
'font-size' | ||
] = `${this.props.fontSize}px`; | ||
} | ||
] = `${fontSize}px`; | ||
|
||
componentDidUpdate(prevProps) { | ||
this._cm.setOption('theme', `p5-${this.props.theme}`); | ||
this._cm.getWrapperElement().style[ | ||
'font-size' | ||
] = `${this.props.fontSize}px`; | ||
this._cm.refresh(); | ||
} | ||
return () => { | ||
codemirrorRef.current.off('keydown', handleKeyDown); | ||
codemirrorRef.current = null; | ||
}; | ||
}, [theme, dispatchConsoleEvent, fontSize]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have to be careful here because we don't want to re-create the entire codemirror instance when I think (need to try it out to be sure) that you can remove the |
||
|
||
componentWillUnmount() { | ||
this._cm = null; | ||
} | ||
useEffect(() => { | ||
if (codemirrorRef.current) { | ||
codemirrorRef.current.setOption('theme', `p5-${theme}`); | ||
codemirrorRef.current.getWrapperElement().style[ | ||
'font-size' | ||
] = `${fontSize}px`; | ||
Comment on lines
+95
to
+97
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This part of the code has always felt a bit wonky to me. I think that you can delete these lines from all |
||
codemirrorRef.current.refresh(); | ||
} | ||
}, [theme, fontSize]); | ||
|
||
render() { | ||
return ( | ||
<div className="console__input"> | ||
<div | ||
className="console-active__arrow-container" | ||
style={{ height: `${this.props.fontSize * 1.3333}px` }} | ||
> | ||
<RightArrowIcon | ||
className="console-active__arrow" | ||
focusable="false" | ||
aria-hidden="true" | ||
style={{ | ||
width: `${this.props.fontSize}px`, | ||
height: `${this.props.fontSize * 0.57}px` | ||
}} | ||
/> | ||
</div> | ||
<div | ||
ref={(element) => { | ||
this.codemirrorContainer = element; | ||
return ( | ||
<div className="console__input"> | ||
<div | ||
className="console-active__arrow-container" | ||
style={{ height: `${fontSize * 1.3333}px` }} | ||
> | ||
<RightArrowIcon | ||
className="console-active__arrow" | ||
focusable="false" | ||
aria-hidden="true" | ||
style={{ | ||
width: `${fontSize}px`, | ||
height: `${fontSize * 0.57}px` | ||
}} | ||
className="console__editor" | ||
/> | ||
</div> | ||
); | ||
} | ||
} | ||
<div ref={codemirrorContainerRef} className="console__editor" /> | ||
</div> | ||
); | ||
}; | ||
|
||
ConsoleInput.propTypes = { | ||
theme: PropTypes.string.isRequired, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would probably leave this in? Even though the code is not as similar anymore.