-
Notifications
You must be signed in to change notification settings - Fork 3
add: PreloaderModal and integrated to the Toolbar #81
Conversation
✅ Deploy Preview for 3dstreet-editor-builds ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
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 left one comment asking about the 2.5 second timeout -- is there no method to remove the save dialog only when file downloaded?
- Also can we change the name from "PreloaderModal" to "SavingModal"? Reason being, we will later have a loading modal for the entire application which is different than this saving indicator.
src/components/scenegraph/Toolbar.js
Outdated
new Promise(resolve => { | ||
const sceneElem = AFRAME.scenes[0]; | ||
sceneElem.components.screenshot.capture('perspective'); | ||
setTimeout(() => resolve(), 2500); |
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 wait 2500 ms? is there not a mechanism to wait to remove the saving modal until the file is downloaded?
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.
Hello @kfarr!
-
I've tried. But for some reason, it triggers much earlier (approx 2.5 sec). I think it's because the capture() function ends with triggering the download and not with the download ends. If it's a critical issue, I could investigate how to make this in the other way;
-
Sure, I'll rename the component
Hello, @kfarr, I've reduced timeout to 1s, and renamed PreloaderModal to SavingModal |
Thanks @omedvediev the last request is it possible to actually keep the loader up until the snapshot is actually completed and downloaded, for example with some sort of callback? A timeout is not a great solution IMHO but if that's the only thing possible right now it is at least better than nothing. We could also make a separate ticket for adding callback in the future. |
Hello, @kfarr! Ok, I'm investigating possible solutions. |
Hello @kfarr , it looks like 'setTimeout' is the best solution for now. As the Browser's file downloading is beyond the scope of the Web App, I haven't found any solution for download finish detection for React. |
@omedvediev thanks for the research. Understood it's easiest to stick with setTimeout, at least for a first version. To finish this out:
For future reference: (KF TODO make a new ticket for this)
|
Hello @kfarr , timeout is set to 2sec, conflicts are resolved. |
No description provided.