createing default workspace #1111
Conversation
| @@ -563,8 +563,6 @@ class FileManager extends Plugin { | |||
| if (file.startsWith('browser')) { | |||
| return this._deps.filesProviders.browser | |||
| } | |||
| const provider = this._deps.filesProviders.workspace | |||
There was a problem hiding this comment.
do you need to remove this?
There was a problem hiding this comment.
yes we don't need to show that error anymore
| @@ -109,6 +117,18 @@ export const Workspace = (props: WorkspaceProps) => { | |||
| } | |||
| }, []) | |||
|
|
|||
| const createNewWorkspace = async (workspaceName) => { | |||
There was a problem hiding this comment.
maybe it would make sense to refactor this with onFinishCreateWorkspace because they do the same thing.
| } catch (error) { | ||
| console.error(error) | ||
| else { | ||
| this._deps.fileProviders.workspace.setWorkspace(workspaceName) |
There was a problem hiding this comment.
I don't think you need the else here
There was a problem hiding this comment.
it is changing nothing just increasing readability of the code
| @@ -8,6 +9,7 @@ class WorkspaceFileProvider extends FileProvider { | |||
| super('') | |||
| this.workspacesPath = '.workspaces' | |||
| this.workspace = null | |||
| this.event = new EventManager() | |||
There was a problem hiding this comment.
instead of using require('../../lib/events') you could also use const EventEmitter = require('events') which is the oob solution. The previous one is no longer used for new features.
There was a problem hiding this comment.
it is not working with a change.
There was a problem hiding this comment.
you might need to update the function, it should work
There was a problem hiding this comment.
just check where it is used
There was a problem hiding this comment.
you can find example where it's being used there https://github.com/ethereum/remix-project/search?q=EventEmitter
|
|
||
| createWorkspace (name) { | ||
| if (!name) name = 'default_workspace' | ||
| this.event.trigger('create_workspace', [name]) |
There was a problem hiding this comment.
name of the events are usually camel case I think createWorkspace
| } catch (error) { | ||
| console.error(error) | ||
| else { | ||
| this._deps.fileProviders.workspace.setWorkspace(workspaceName) |
There was a problem hiding this comment.
when no workspace is selected and a plugin try to write to remix, did you make sure the app switch to the new created workspace? cause I am not sure doing fileProviders.workspace.setWorkspace(workspaceName) is enough
There was a problem hiding this comment.
is selected or available?
There was a problem hiding this comment.
I meant "selected".
I am not sure doing this._deps.fileProviders.workspace.setWorkspace(workspaceName) is the best way to handle that, but that seem to be ok for the moment
fix #1028