Page reordering from carousel

[miina]

Summary

Fixes #3835

• Adds basic DropZone.
• Adds DropZoneProvider which keeps track of the DropZones and detects which is hovered on.
• Adds functionality for reordering Pages via Thumbnails.
• Assumes that focusing and keyboard interaction will be part of Keyboard handling #3826 (PR: Implement keyboard and focus management #3897

Checklist

• My pull request is addressing an open issue (please create one otherwise).
• My code is tested and passes existing tests.
• My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

[miina]

Follow-up issue for keyboard reordering only: https://github.com/ampproject/amp-wp/issues/4112

[spacedmonkey]

What is the expected behaviour when a page is reordered and the presses undo / redo?

[miina]

What is the expected behaviour when a page is reordered and the presses undo / redo?

Good question. It should undo it :)

Note: When testing, it looks like the history is generally not working properly, e.g. when adding a Page and clicking Undo, it removes the Page, however, clicking again adds the Page back again.

Same with adding an element -- the first Undo removes it, the second adds it back again, instead of undoing a previous action. Try adding a few elements and then clicking Undo:

It looks like the Undo/Redo should be looked at separately, I was (incorrectly) assuming that it already worked as expected with the actions in the reducer but looks like not.

I think it would be good to create a separate issue for fixing History first. Thoughts? cc @barklund

[barklund]

I think it would be good to create a separate issue for fixing History first. Thoughts? cc @barklund

Yes, I think it actually broke all the way back in #3910 at some stage in the revisions of the PR. I have an idea about what the problem is (essentially that restore doesn't actually set the same objects, but clone at least one of them, which makes the history think that state hasn't be restored to an old state, it's just a new state).

I think it's a fairly easy fix once looked at separately.

[swissspidy]

There are now some merge conflicts after the page carousel revamp.

[miina]

Just pushed the change, the conflicts should be resolved now.

[swissspidy]

Seems like the editor is crashing when adding new pages. Getting Uncaught Invariant Violation: Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. within DropZone.

[miina]

Thanks, @swissspidy! Should be fixed now.

[dvoytenko]

I'm LGTM otherwise, but please take a look at the display:inherit issue. It's a bit dangerous IMHO.

[dvoytenko]

I know I'm a bit late with this question, but just to confirm: does reordering of pages ever change the currently shown page?

[miina]

Yes, currently it sets the page that was reordered as current. Perhaps it shouldn't be like this? That was an assumption but maybe the current page should not change? It made more sense without the page preview that we have now.

[swissspidy]

Opened an issue to confirm this: GoogleForCreators/web-stories-wp#141

[dvoytenko]

Yeah. I tried both ways and either seem pretty logical to me. I think this is low priority and let's assign to Sam.