-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Fixing bugs of FE #2386 #2427
Fixing bugs of FE #2386 #2427
Conversation
src/app/files/file-explorer.js
Outdated
if (error) return modalDialogCustom.alert('Failed to create file ' + newName + ' ' + error) | ||
const currentPath = !parentFolder ? self._deps.fileManager.currentPath() : parentFolder | ||
newName = currentPath ? currentPath + '/' + newName : self.files.type + '/' + newName | ||
if (input === '') input = 'New file' |
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.
I would rather do if (input)
like that you check for null & undefined also.
src/app/files/file-explorer.js
Outdated
@@ -601,14 +605,24 @@ fileExplorer.prototype.createNewFile = function (parentFolder) { | |||
|
|||
fileExplorer.prototype.createNewFolder = function (parentFolder) { | |||
let self = this | |||
modalDialogCustom.prompt('Create new folder', '', '', (input) => { | |||
modalDialogCustom.prompt('Create new folder', '', 'New folder', (input) => { | |||
if (input === '') { |
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.
Same as above
src/app/files/file-explorer.js
Outdated
modalDialogCustom.prompt('Create new folder', '', 'New folder', (input) => { | ||
if (input === '') { | ||
tooltip('Failed to create folder. The name can not be empty') | ||
return 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.
If you're using return
to stop the process of the scope I would rath do return tooltip(...)
. Returning false
feels like it's used somewhere. If this is the case then forget this comment.
} else { | ||
window.remixFileSystem.unlinkSync(unprefixedpath, console.log) | ||
path = this.removePrefix(path) | ||
if (window.remixFileSystem.existsSync(path)) { |
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 ??? We inject the FileSystem as a global value in the file system o.O"
src/app/files/fileProvider.js
Outdated
this.event.trigger('fileRemoved', [this._normalizePath(path)]) | ||
return true | ||
} else { | ||
let items = window.remixFileSystem.readdirSync(path) |
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.
You're not reassigning it. So you can use const
src/app/files/fileProvider.js
Outdated
let items = window.remixFileSystem.readdirSync(path) | ||
if (items.length !== 0) { | ||
items.forEach((item, index) => { | ||
let curPath = path + '/' + item |
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.
Maybe :
const currPath = `${path}/${item}`
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 it is better?
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.
You don't reassign it so you can you const
.
Personally I find the ${a}divder${b}
writing more readable than using concatenation (just suggesting here).
No description provided.