-
Notifications
You must be signed in to change notification settings - Fork 28.4k
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
feature: replace electron File.path with electron webUtils #213031
feature: replace electron File.path with electron webUtils #213031
Conversation
Thanks, I wasn't actually sure how to approach this when I saw it, so I will learn from your changes 🙏 |
Thanks! Initially, I only added electron webUtils to the preload script and exposed it as a global variable |
But I think a service that cannot be implemented for web is also not ideal. I think the right thing would be to depend on |
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.
Thanks for the feedback. Do you have an example of similar code where that is applied? I'm not sure how I would proceed here: // browser/fileActions.ts
export const pasteFileHandler = async (accessor: ServicesAccessor, fileList?: FileList) => {
if (toPaste.type === 'paths' && item instanceof URI) {
return item.path
}
// how to call something some electron-sandbox here without importing the file from electron-sandbox?
} Thanks again! |
Yeah e.g. here:
For this, the action needs to be moved to a |
Since this action seems to be used in web as well, it probably should be subclassed on desktop to provide the extra functionality and then contributed per-web and per-desktop. |
…/terminal.contribution.ts
…/terminalInstanceService.ts
…minalInstance.test.ts
I like reusing the |
@SimonSiefke I pushed c0298e5 to cleanup, I found many warnings (missing semicolons, etc), did you see them in the editor before committing? please check if things still work. |
I forgot I had ESLint disabled, thanks for the fixes! Pasting files in the explorer and terminal is also still working for me! |
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.
Thanks!
From the electron docs:
This PR replaces the deprecated
File.path
property with the new electronwebUtils
api.For testing:
Native Drag and Drop in the File Explorer:
Normal Drag and Drop in the File Explorer:
Drag and Drop in the Terminal: