-
Notifications
You must be signed in to change notification settings - Fork 15.8k
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 Request]: "blocking" version of setSaveDialogOptions #41640
Comments
For those who stumbled across this issue, I built a library that resolves a lot of issues that https://github.com/theogravity/electron-dl-manager I use the alternative technique I described in the ticket. |
@codebytere Can we ban the above poster? It's the second time I've seen this person comment in my issue tickets with junk. It seems to be generated from AI. |
This comment was marked as off-topic.
This comment was marked as off-topic.
@theogravity Banned - thanks for the call out, and sorry for the trouble 🙇♀️ |
Update: My workaround didn't actually work, and part of it was due to my lack of knowledge at the time around the lifecycle of a download in Electron. The missing part in the story is: If user doesn't set the save path via the API, Electron will use the original routine to determine the save path; this usually prompts a save dialog. What this means is if you did not manually do a So what happens if you use the workaround I describe above where you use Because Summary: In a scenario where you are using
So we must use The solution I have posted a new version of my library that no longer uses the above suggestion, but a different one instead: // item from will-download handler
// Pause the download because if we don't pause and the download finishes before the user
// makes their selection, events won't get fired in a save as dialog situation
item.pause();
if (saveDialogOptions) {
// we'll eventually show a save as dialog
item.setSaveDialogOptions(window, {
defaultPath: filePath,
...saveDialogOptions,
});
} else {
// do not use a save as dialog, just save immediately after download to the specified path
item.setSavePath("");
}
if (saveDialogOptions) {
// Because the download happens concurrently as the user is choosing a save location
// we need to wait for the save location to be chosen before we can start to fire out events
// there's no good way to listen for this, so we need to poll
const interval = setInterval(async () => {
// Check if the user has specified a save location
if (item.getSavePath()) {
// Save location specified, now register events and resume the download
clearInterval(interval);
item.on("updated", updatedHandler);
item.once("done", doneHandler);
item.resume();
} else if (item.getState() === "cancelled") {
clearInterval(interval);
// Your cancel dialog logic
}
}, 500);
} else {
// Not showing dialog, execute immediately
item.on("updated", updatedHandler);
item.once("done", doneHandler);
item.resume();
} |
That being said, it's clear that the workflow for handling downloads when you want to use a save as dialog is really clunky. It definitely needs some kind of overhaul. |
Preflight Checklist
Problem Description
I'm writing a file downloader which makes use of
DownloadItem.setSaveDialogOptions()
, and the behavior around it is unintuitive.item.getFilename()
doesn't update with the filename the user has selected from the dialogitem.getSavePath()
will howeverProposed Solution
This might be difficult due to how
setSaveDialogOptions()
is coupled to theDownloadItem
, but I'd like to see the following:setSaveDialogOptions()
-asyncSetSaveDialogOptions()
?DownloadItem
is to just immediately download when created), and "resumed" if the user decides to save the file;DownloadItem.cancel()
is called if the user cancels the dialog. The value returned fromasyncSetSaveDialogOptions()
would be a boolean to indicate if the user has confirmed or denied save.Alternatives Considered
Edit: The below does not work. See comments on why and an actual fix
Discovered a workaround: don't use
setSaveDialogOptions()
at all, butdialog.showSaveDialog()
instead:Additional Information
No response
The text was updated successfully, but these errors were encountered: