Skip to content

When user uploads project file, project will auto-save before loading#4134

Merged
benjiwheeler merged 3 commits intoscratchfoundation:developfrom
benjiwheeler:save-before-upload
Dec 18, 2018
Merged

When user uploads project file, project will auto-save before loading#4134
benjiwheeler merged 3 commits intoscratchfoundation:developfrom
benjiwheeler:save-before-upload

Conversation

@benjiwheeler
Copy link
Contributor

@benjiwheeler benjiwheeler commented Dec 18, 2018

Resolves

Proposed Changes

  • adds several states to project-state reducer to handle file upload
  • introduces ability to indicate that a project load failed, and then to return to normal state with currently showing project; we should discuss this
  • refactors sb-file-uploader so that it syncs with project-state state machine

Note that the titles still aren't set right; we could add that here or do it as a follow on. But, at least now the old project's title isn't overwritten.

Basic functionality: uploading a project saves current project first
main

The original project is indeed saved correctly:
orig-is-saved

Uploading a second project file, with no page reload (so keeping the same FileReader object), works correctly:
second

trying to upload an invalid file fails without taking you away from your current project:
invalid

Reason for Changes

current project wasn't being saved before uploading project and saving as new project

Test Coverage

Adjusted and added project state reducer tests

Browser Coverage

Check the OS/browser combinations tested (At least 2)

Mac

  • Chrome
  • Firefox
  • Safari

Windows

  • Chrome
  • Firefox
  • Edge

Chromebook

  • Chrome

iPad

  • Safari

Android Tablet

  • Chrome

Copy link
Contributor

@paulkaplan paulkaplan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Travis tests are failing

@benjiwheeler benjiwheeler assigned paulkaplan and unassigned rschamp Dec 18, 2018
@benjiwheeler benjiwheeler requested review from paulkaplan and removed request for rschamp December 18, 2018 20:15
@paulkaplan paulkaplan removed their assignment Dec 18, 2018
@paulkaplan
Copy link
Contributor

paulkaplan commented Dec 18, 2018

@benjiwheeler I noticed that this causes a regression where the project title does not get updated in the standalone editor. If possible, I think we should get this in without introducing a regression. This will, for instance, break the Scratch Desktop functionality

@benjiwheeler benjiwheeler changed the title added project states for sb file upload; refactored uploader When user uploads project file, project will auto-save before loading Dec 18, 2018
@benjiwheeler
Copy link
Contributor Author

I was trying to keep this change focused to fixing this one issue and not the title-switching one; I can't keep the current functionality. But let me try fixing that too, here -- it's probably not so hard.

@paulkaplan
Copy link
Contributor

@benjiwheeler yeah, it's just it is hard to revert PRs that break one thing in favor of another. We have to fix both before either goes in I think, so let's just include both here

Copy link
Contributor

@paulkaplan paulkaplan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay! Looks good now. Let's get this to staging for some 👀👀👀👀 /cc @BryceLTaylor

@benjiwheeler
Copy link
Contributor Author

Just waiting for tests to pass, then will merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants