Move Javalab Editor header and dialogs into their own components#47116
Conversation
molly-moen
left a comment
There was a problem hiding this comment.
LGTM! I agree with your assessment on moving the file errors to redux but keeping the functions passed around.
| /> | ||
| {backpackEnabled && ( | ||
| <PaneSection style={styles.backpackSection}> | ||
| <Backpack |
There was a problem hiding this comment.
Not sure if this is worth noting somewhere, but there are a couple of dialogs that can be shown within the Backpack component
There was a problem hiding this comment.
Ah I see - I think it would make sense to leave those where they are since they're contained and controlled within the Backpack component?
There was a problem hiding this comment.
Yeah totally, not suggesting we should move them (at least for now), but was wondering if we should note there are more possible dialogs than what appear in JavalabEditorDialogManager.
| onDeleteFile={this.onDeleteFile} | ||
| filenameToDelete={fileMetadata[fileToDelete]} | ||
| onRenameFile={this.onRenameFile} | ||
| filenameToRename={fileMetadata[editTabKey]} |
There was a problem hiding this comment.
Not sure if this would make it more confusing, but I think this could come from Redux in JavalabEditorDialogManager
There was a problem hiding this comment.
Ah good call, will update to use Redux here.
There was a problem hiding this comment.
Actually looking at this again, looks like fileToDelete is part of JavalabEditor's state so we'd still need to pass it in somehow. I could move the filenameToRename to redux since editTabKey comes from redux, but I think I may keep it as is for now for consistency, if that sounds ok? I think once we merge all these we'll be able to still move more stuff into redux/separate components so this would end up simplifying anyway.
There was a problem hiding this comment.
Yep I was looking specifically at filenameToRename, and yeah agree consistency sounds good for now 👍
…factor/file-errors-redux Move file error message into redux
625d79d
into
sanchit/javalab-editor-refactor/commitdialog
Now we can start to break up Javalab Editor by moving groups of related pieces into their own components. This moves the header (buttons & text) and dialogs into their own components. As with the other editor refactor PRs, this is branched off of the previous PR (#47082).
Links
JIRA: https://codedotorg.atlassian.net/browse/JAVA-603
Testing story
Tested locally.
Deployment strategy
Follow-up work
Some notes/next steps
JavalabEditorDialogManager.rename/newFileErrorseem like good candidates to move into redux. However I think the functions (onCreateFile,onDeleteFile, etc) will need to stay in JavalabEditor, or at least in some higher level component. I thought about how we could move these to their own component and I think the issue here is that these functions end up modifying the global state of the editor, namely the files and tabs. They also produce some side effects (such as callingprojectChanged) so I don't think it makes sense to put that logic in redux either. My thought for now is to keep those inJavalabEditorand treat it as the high-level container component, while moving anything else out to its own component.JavalabEditorwith just the logic to update file metadata on create/edit/delete.Privacy
Security
Caching
PR Checklist: