Converted ConsoleInput to functional components #2409
Conversation
There was a problem hiding this comment.
This code doesn't actually work, does it? If I type let x = 1 + 2 into the editor console and hit enter, it just creates extra codemirror instances! 😭
So definitely more work is needed here. I suspect it's the way that we are passing down the dispatchConsoleEvent prop, in which case it can be fixed by connecting this component to Redux directly. But I'm still working on figuring it out.
| 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 |
There was a problem hiding this comment.
I would probably leave this in? Even though the code is not as similar anymore.
| 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.
Love that you are using a prevState callback!
| return { commandCursor: newCursor }; | ||
| setCommandCursor((prevCursor) => { | ||
| const newCursor = Math.min(prevCursor + 1, commandHistory.length - 1); | ||
| cm.getDoc().setValue(commandHistory[newCursor] || ''); |
There was a problem hiding this comment.
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.
| codemirrorRef.current.off('keydown', handleKeyDown); | ||
| codemirrorRef.current = null; | ||
| }; | ||
| }, [theme, dispatchConsoleEvent, fontSize]); |
There was a problem hiding this comment.
We have to be careful here because we don't want to re-create the entire codemirror instance when theme or fontSize change.
I think (need to try it out to be sure) that you can remove the theme: p5-${theme}, and the setting of the fontSize from this main useEffect as they are already covered by your next useEffect.
| codemirrorRef.current.getWrapperElement().style[ | ||
| 'font-size' | ||
| ] = `${fontSize}px`; |
There was a problem hiding this comment.
This part of the code has always felt a bit wonky to me. I think that you can delete these lines from all useEffects and instead put style={{ fontSize }} on the <div ref={codemirrorContainerRef}> element
| // 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.
Let's put a // eslint-disable-next-line no-unused-vars above this line to suppress the warning. It's definitely not the best code to have a state which isn't used but that's not your fault, that's how it is currently.
This is a very complicated component! I confirmed that I can type and submit to the console if the only dependency of the You might be able to get it working if you use one |
|
I tried to use the above method but still am not getting anywhere can you please tell what's wrong in this I have even tried visiting other websites which use a similar type of console box and looked at their code but still can't find out what's wrong with mine. |
Ref #2358
Changes: I have changed the ConsoleInput.jsx file from legacy to functional component.
I have verified that this pull request:
npm run lint)npm run test)developbranch.Fixes #123