Skip to content

Save now#3021

Merged
rschamp merged 5 commits intoscratchfoundation:developfrom
rschamp:save-now
Aug 30, 2018
Merged

Save now#3021
rschamp merged 5 commits intoscratchfoundation:developfrom
rschamp:save-now

Conversation

@rschamp
Copy link
Contributor

@rschamp rschamp commented Aug 28, 2018

Dependent PRs

This requires https://github.com/LLK/scratch-projects/pull/76

Resolves

What Github issue does this resolve (please include link)?

Proposed Changes

Describe what this Pull Request does

  • Add post/put stores for projects and assets
  • Augment the project saver component to allow saving projects to a server in addition to locally
  • Add a reducer for the project id, so that the project saver component has access to it
  • Hook up the "Save now" menu item to save the project to the store, conditionally if there is an active session. So this will continue to be disabled in the GUI playground, but will be enabled if you're logged in on the preview project page
  • Also, while I was there, close the file menu after saving the project to your computer. See
    7e3b53a for why I couldn't do the same for loading a project

Reason for Changes

Explain why these changes should be made
We want to start testing how the GUI will be used on the website, and part of that is saving projects.

Test Coverage

Please show how you have added tests to cover your changes
It breaks some tests. I'm fixing that. Hard to test this type of thing since it happens over the network though.

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

Ray Schamp added 3 commits August 28, 2018 18:37
Unfortunately I couldn't do the same with loading projects, since the presence of the component (necessitating the menu to be open) allows the actual upload from file to work, since it needs the FileInput component to be present, so it can emit a change event.  We should revisit
chrisgarrity
chrisgarrity previously approved these changes Aug 29, 2018
Copy link
Contributor

@chrisgarrity chrisgarrity left a comment

Choose a reason for hiding this comment

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

It will be nice to be able to actually update projects, but there's still a lot of todo items. Perhaps there should be open issues/@todos with links to them?

@@ -0,0 +1,27 @@
const SET_PROJECT_ID = 'scratch-gui/project-id/SET_PROJECT_ID';

This comment was marked as abuse.

animation: hue-rotate 3s linear infinite;
}

@keyframes hue-rotate {

This comment was marked as abuse.

{saveNowMessage}
</MenuItem>
) : (
<MenuItemTooltip

This comment was marked as abuse.

This comment was marked as abuse.

};

const mapStateToProps = state => ({
canUpdateProject: typeof (state.session && state.session.session && state.session.session.user) !== 'undefined',

This comment was marked as abuse.

This comment was marked as abuse.

this.setState({projectSaveInProgress: true},
() => {
updateFun().then(() => {
this.setState({projectSaveInProgress: false});

This comment was marked as abuse.

This comment was marked as abuse.

handleCloseFileMenuAndThen (fn) {
return () => {
this.props.onRequestCloseFile();
fn();

This comment was marked as abuse.

This comment was marked as abuse.

@chrisgarrity
Copy link
Contributor

@rschamp none of the things I flagged are blockers, but it probably does need to have a passing build.

@rschamp
Copy link
Contributor Author

rschamp commented Aug 29, 2018

Thanks @chrisgarrity! Captured these as follow-up items ^

Remove the default prop test because the default projectId prop comes from redux now, so it doesn't make sense to test the default

Lint
Fastly is not configured yet
@rschamp rschamp merged commit 45031c6 into scratchfoundation:develop Aug 30, 2018
@rschamp rschamp deleted the save-now branch August 30, 2018 01:28
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.

Allow saving projects and assets

2 participants