Load Button Loading/Error State #1621
Conversation
…pdate load button to use new revamped loadProject function from VM. Loading a project via load button should clear the url hash (without triggering a reload or the project-loader-hoc).
There was a problem hiding this comment.
LGTM apart from one nit, though I would recommend also getting a review from someone more familiar with React.
A few "have you thought about"s, since I can't quite tell the behavior just by reading this:
- If a load fails and then a subsequent load succeeds, does it clear out the error message display?
- Is it possible to start a second load while one is already in progress?
| @@ -26,6 +26,7 @@ const LoadButtonComponent = ({ | |||
| </ButtonComponent> | |||
| <input | |||
| disabled | |||
| accept=".sb,.sb2,.sb3,.zip" | |||
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
| } | ||
| handleChange (e) { | ||
| this.props.openLoadingState(); | ||
| // Remove the hash if any (without triggering a hash change event or a reload) | ||
| history.replaceState({}, document.title, '.'); |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
…ct it's actual contents." scratch-audio depends on sound property being called md5 even though it refers to md5+file ext, so this property cannot be renamed just yet. This reverts commit dee7195.
…Remove .zip from the same. Save project as .sb3 (and untitled-project).
|
@cwillisf, to answer your questions:
The way this code is set up, it should not be possible to start another load after a load fails. If a load fails, the 'Oops' page shows up, and users have to reload the gui via the reload button on the 'Oops' page.. It should also not be possible to start a second load (via the load button) while one is already in progress. Starting a load triggers the loading state animation, and it covers the whole gui, including the menu bar where the save/load buttons are located. It is possible to start a second load while a load is already in progress via the hash in the url. With some minimal testing, this seems to work fine. The second load is the project that loads and there aren't any unexpected errors in the console. |
|
@kchadha Is this ready for re-review? |
…the current project id and current project data should happen in a reducer since they are part of global state. That way, the load button can affect the global project data as well.
…nges to the current project id and current project data should happen in a reducer since they are part of global state. That way, the load button can affect the global project data as well." This reverts commit e4fe143.
(a.k.a. Save/Load pt. 2)
Proposed Changes
This PR is a continuation of the save/load work, and accommodates the changes made in scratchfoundation/scratch-vm#979 and scratchfoundation/scratch-parser#21. This PR depends on these two sets of changes, and should not be merged until these others have been merged.
Changes in this PR include:
loadProjectfunction to handle both JSON strings (returned by storage) as well as input buffers (from the load-button file input), instead of usingloadProjectfor one andloadProjectLocal(introduced by Save and Load sb3 projects #1564) for the other.Reason for Changes
Continuing Save/Load work.
Test Coverage
Existing tests pass.
(Travis build will fail until scratchfoundation/scratch-parser#21 and scratchfoundation/scratch-vm#979 are merged)