Restore deleted sprite.

[kchadha]

Resolves

Towards implementing 'undelete' functionality from 2.0.

Proposed Changes

• Remove 'Redo' item in the 'Edit' menu
• Replace 'Undo' item in the 'Edit' menu with 'Restore' which restores the last deleted sprite. Restore is disabled in the menu by default, and becomes enabled after a sprite gets deleted. When the option becomes enabled, it reads 'Restore sprite'. Clicking on the option should add the last deleted sprite to the project and then disable the 'Restore' option again, changing the text of the option back to 'Restore'

Not Addressed By This PR

• Restoring costumes, sounds, or blocks
• Removal of the confirmation dialog when deleting a sprite (this confirmation is linked to deleting all things: sprites, costumes, sounds, so it has not yet been removed)

Related PRs

Depends on scratchfoundation/scratch-vm#1438, and tests will fail until that PR is merged in.

Test Coverage

Manual testing. Changes can be tested at https://llk.github.io/scratch-gui/restore-sprite

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

[rschamp]

This looks great overall!

Addressed feedback.

[kchadha]

@rschamp, I addressed the comments you made.

One thing I have a question about is that I am getting warnings in the console, and I'm not exactly sure where they're coming from (this was happening prior to the last commit as well). Any ideas?:

[rschamp]

That warning happens when you use mapStateToProps without mapDispatchToProps. Without mapDispatchToProps, connect provides a dispatch prop. Then if you do <Component {...props} /> the dispatch prop gets passed along until it's added to the final <div> (or whatever output component), and React complains about not knowing what to do with a dispatch attribute on a <div>. To remedy, use () => {} as the mapDispatchToProps argument to keep dispatch from being passed.

[rschamp]

I didn't notice the second error referencing dispatchUpdateRestore. That is from you beginning to pass the dispatchUpdateRestore prop to the component for use in the handler methods, but you haven't prevented passing it down to the child component in render. You should omit it from the props being passed to the child.

[rschamp]

Thanks for making those changes, still keeping as needs work for the prop type errors.

[kchadha]

@rschamp thanks for explaining those warnings! I fixed the second one, but found that the first one is actually already on develop because there aren't any places in this PR where I'm using mapStateToProps without mapDispatchToProps.

For that second warning, react was only complaining about the specific instance of dispatchUpdateRestore being passed through to the TargetPaneComponent, but I added the analogous change to the deletion-restorer as well. I'm not sure why it wasn't complaining about that instance of the issue...?

[rschamp]

Thanks for fixing up the dispatchupdateRestore prop. Like we talked about offline, it's good that you cleaned it up even in the instance where it has no effect for future-proofing. It wasn't causing an issue because you omitted it in the implementation: https://github.com/LLK/scratch-gui/pull/2824/files#diff-8d887729f7ca8c4b671c7a4ecd6f0c2bR290 — if you had used props rather than {restorable, deletedItem} and then used ...props in a child component, you would have seen it pop up.