React component conversion(class to functional) - 1 #3208
Conversation
|
🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already. |
Release EnvironmentsThis Environment is provided by Release, learn more! 🔧Environment Status : https://app.release.com/public/Processing%20Foundation/env-3a3542f66b |
nahbee10
changed the title
| @@ -146,168 +336,135 @@ class Editor extends React.Component { | |||
| } | |||
| }); | |||
|
|
|||
| this.hinter = new Fuse(hinter.p5Hinter, { | |||
There was a problem hiding this comment.
did we lose this in the conversion? do we not need it anymore?
There was a problem hiding this comment.
if we don't need it anymore, let's also remove it from the dependencies~
| } else { | ||
| cm.replaceSelection(' '.repeat(INDENTATION_AMOUNT)); | ||
| _cmInstance.replaceSelection(' '.repeat(INDENTATION_AMOUNT)); |
There was a problem hiding this comment.
not sure why eslint didn't catch this but i don't think this codebase uses underscores for private variables! looks like they are following the airbnb style guide: https://github.com/airbnb/javascript?tab=readme-ov-file#naming--leading-underscore
this comment applies to _cmInstance but also all others with underscores
I do see that the original file had underscores too so i guess it's up to us if we want to re-align the code with the style guide named in the contributor docs (my perference) or not change things up too much : P @raclim any thoughts?
There was a problem hiding this comment.
I think this rule is turned off in our eslint configuration 🤔 but it makes sense to me to aim to align with the style guide as much as possible!
| [`${metaKey}-K`]: (cm, event) => | ||
| cm.state.colorpicker.popup_color_picker({ length: 0 }), | ||
| [`${metaKey}-.`]: 'toggleComment' // Note: most adblockers use the shortcut ctrl+. | ||
| [`${metaKey}-K`]: (_cmInstMetaKey, event) => |
There was a problem hiding this comment.
can we keep the comments from cassie and other notes? will probably be nice for the future : )
|
Left a few comments but overall looks great! : D thanks nahee! |
|
When I test the staging link, something kind of crazy happens when i hit tidy code :O |
|
I think we may have lost the error highlighting as well -- left is this branch and right is what's live |
|
Just to add, I'm on the staging environment and am having trouble switching between different files in the left-hand navigation as well! test_editor_component.mov |
raclim
added
Type:Task
Area:Editor
nahbee10
force-pushed
the
react-fc-conversion
branch
from
2b0f82d
to
df4ef47
Compare
…5.js-web-editor into react-fc-conversion
Fixes #2709
Changes:
IDE/components/Editor/index.jsxto use React functional componentI have verified that this pull request:
npm run lint)npm run test)developbranch.Fixes #123