-
Notifications
You must be signed in to change notification settings - Fork 313
Fix file not getting save after a change (not sending api request) #351
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
Conversation
|
|
||
| setEditorFileStatus(EnumEditorFileStatus.Saving) | ||
|
|
||
| if (activeFile?.isLocal || activeFile.content === value) return setIsSaved(true) |
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.
What's the return value of setIsSaved()?
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.
undefined
| <small>File Status: {fileStatus}</small> | ||
| <small>{getLanguageByExtension(activeFile?.extension)}</small> | ||
| <div className='flex align-center mr-4'> | ||
| <Indicator text="Valid" ok={isSaved} /> |
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.
Why is Valid tied to whether the file is saved or not? If it's a placeholder, can you just set it to true?
| )} | ||
|
|
||
| {!activeFile?.isLocal && ( | ||
| {activeFile?.isLocal === false && ( |
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.
why do you need to check isLocal? is it because activeFile? could be null?
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.
activeFile in editor can be either from file system (where we need to read content of that file) or created by clicking on plus button to add empty in-memory sql file to send queries, so isLocal just indicates that
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.
sorry my question is why not just check isLocal which is boolean, why do you === false
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.
actually here activeFile should not be 'null'.
always missing that ! so === more explicit
Fix bug and added indication when file saved.
