Feature syntax highlight check #290
Conversation
| readOnly: true, | ||
| mode: 'javascript' | ||
| }, | ||
| code: ' function iamHappy(happy) {\n\tif(happy){\n\t console.log("I am Happy!")\n\t}else{\n\t console.log("I am not Happy!")\n\t}\n};' |
There was a problem hiding this comment.
[question] Does code need to be kept by state?
There was a problem hiding this comment.
Please place white space before and after else, if.
|
|
||
|
|
||
|
|
||
| let newOptions = { |
There was a problem hiding this comment.
Maybe you can use const.
Basically, please use const.
|
|
||
| this.setState({options: newOptions}) | ||
|
|
||
| this.setState({ config }) |
There was a problem hiding this comment.
You can write like below.
this.setState({
options: newOptions, config,
})
| this.state = { | ||
| config: props.config | ||
| config: props.config, | ||
| options: { |
There was a problem hiding this comment.
options is fuzzy word. How about codeMirrorOptions?
| lineNumbers: true, | ||
| theme: props.config.editor.theme, | ||
| readOnly: true, | ||
| mode: 'javascript' |
There was a problem hiding this comment.
I think lineNumbers, readOnly, mode won't be changed.
You don't need to keep them in state.
| let { config, options } = this.state | ||
| let checkHighLight = document.getElementById('checkHighLight') | ||
|
|
||
| if (checkHighLight == null) { |
There was a problem hiding this comment.
Please use ===. == or != is too fuzzy.
| config: props.config, | ||
| options: { | ||
| lineNumbers: true, | ||
| theme: props.config.editor.theme, |
There was a problem hiding this comment.
theme attribute looks not necessary for CodeMirror component.
https://github.com/JedWatson/react-codemirror/blob/master/src/Codemirror.js#L13-L23
| this.state = { | ||
| config: props.config | ||
| config: props.config, | ||
| codemirrorTheme: props.config.editor.theme |
|
|
||
| handleUIChange (e) { | ||
| let { config } = this.state | ||
| let { codemirrorTheme } = this.state |
| const newCodemirrorTheme = this.refs.editorTheme.value | ||
|
|
||
| if (newCodemirrorTheme !== codemirrorTheme) { | ||
| checkHighLight.setAttribute('href', '../node_modules/codemirror/theme/' + newCodemirrorTheme + '.css') |
There was a problem hiding this comment.
Using template literal is more readable.
checkHighLight.setAttribute('href', `../node_modules/codemirror/theme/${newCodemirrorTheme}.css`)
| } | ||
| </select> | ||
| <div styleName='code-mirror'> | ||
| <CodeMirror value={codemirrorSampleCode} options={{ lineNumbers: true, readOnly: true, mode: 'javascript', theme: codemirrorTheme }} /> |
There was a problem hiding this comment.
Good component! Readable 🍰
sota1235
left a comment
sota1235
left a comment
There was a problem hiding this comment.
Please fix some points.
sota1235
left a comment
sota1235
left a comment
There was a problem hiding this comment.
Great work! Thanks for many fixing 👍
Change Editor Theme Setting Screen
Before, when we selected editor theme in UI setting screen, we cannot check real syntax highlight.
After, when we selected editor theme in UI setting screen, we can check real syntax highlight.
Sorry
sometimes , javascript mode is not working. If you do not mind, please tell me the solution.