Zip downloading: added modal for successfully downloading zips in game #10255
Conversation
| {lf("Your projects were zipped and downloaded successfully!")} | ||
| <br /> | ||
| <br /> | ||
| {lf("Look for the file {0} on your computer. It might have a number before the .zip if you've downloaded before.", fileName)} |
There was a problem hiding this comment.
At least when I've done this, I get a popup where I can choose the location and name for the file (both in the browser and when running dev Minecraft build on Windows pointing to your uploaded target). I'm wondering if we really need this additional string, or if we can just say it was downloaded successfully and leave it at that?
There was a problem hiding this comment.
Yeah, that's what happens for me, too. I added the extra string because it just felt a bit bare to me with just one line, but I can remove the extra stuff.
There was a problem hiding this comment.
I prefer concise language here. This needs to be translated to lot of languages and number system might be different. Just say "Your projects successfully downloaded as a zip"
| if (pxt.BrowserUtils.isInGame()) { | ||
| const downloadJsx = this.renderDownloadDialog(zipName); | ||
|
|
||
| setTimeout(async () => { |
There was a problem hiding this comment.
Is the timeout waiting for the download to finish? 2 seconds may not be enough time for someone with a whole bunch of projects and very slow internet (though not sure how likely that is).
There was a problem hiding this comment.
Yes, it is. That's a good point. I don't really want to increase the time for it off the bat, though, because if it's too long and the notification shows up way after downloading then it feels even more disjoint.
This is not my favorite solution, but I included the timeout because I couldn't find an event that could tell us that the user stopped interacting with the filesystem/download unfortunately. No timeout was even worse because the modal would show up even before the filesystem modal would show up.
| @@ -424,11 +435,25 @@ export class ScriptManagerDialog extends data.Component<ScriptManagerDialogProps | |||
|
|
|||
| const zipName = `makecode-${targetNickname}-project-download.zip` | |||
|
|
|||
| pxt.BrowserUtils.browserDownloadDataUri(datauri, zipName); | |||
|
|
|||
| this.setState({ | |||
There was a problem hiding this comment.
Was the reordering of download: null state and browserDownloadDataUri intentional? My assumption would be that we should stay in a downloading state until all the steps have finished, but I think I may not be understanding what browserDownloadDataUri does...
There was a problem hiding this comment.
You're right, I should move this back, thanks for pointing that out. I think I was reordering stuff to better understand how everything interacted and moving it didn't change anything.
The browserDownloadDataUri function is doing the actual steps to initiate the download of the content to the user's computer. The way we do that differs per method and browser, but regardless, that function holds the download action. For the in-game, chromium case, we are basically putting the contents in an <a> tag and then clicking the link to initiate the download. The logic in handleDownloadAsync is basically only handling the zipping part.
|
Discussed that because the timeout and the lack of ability to know what happens when the user interacts with the file system, the modal wasn't useful enough to the user. |
When downloading zips in-game in Minecraft, there is no indicator that tells you that a download was successful like you get in-browser with browser built-ins. This PR adds a modal that pops up only if you are in game after downloading a zip. I also added a check for showing the download zip button because you shouldn't be able to download a zip of the projects if you can't interact with the filesystem, regardless of the target.
edit: Fixes https://github.com/microsoft/pxt-minecraft/issues/2437
Upload target: https://minecraft.makecode.com/app/b439897404713a96f9e8313e79029cbb195288ed-91a4b94271?inGame=1