Framework: Extract "edit-post" to its own module

[youknowriad]

Sorry for the huge PR, sometimes there's no alternative.

This PR extracts the edit-post module to its own module. This surfaced some "issues"

• The need to spit the store into two stores. One store for the "editor" module (library) which is more a "local state" to the EditorProvider component. This store is saved in the React context using a different key "editorStore" to avoid ambiguity when calling connect

• Some components in edit-post where still needed access to the editor module's store, in general this is fixed by adding reusable components to the editor module.

• The "Show block inspector" button proved to be a challenge, because the idea is that clicking this button shows the inspector, but since the BlockInspector is a reusable component that can be shown anyware in the application using the "editor" module, we needed a showBlockInspector to allow the surrouding application (edit-post) to handle this button properly.

I'd appreciate quick feedback and opinions, this won't be enought to keep without conflicts and continuous rebasing.

Testing instructions

• This may introduce some unnoticed regressions, heavily testing this PR is necessary.

[codecov]

Codecov Report

Merging #3649 into master will decrease coverage by <.01%.
The diff coverage is 16.1%.

@@            Coverage Diff             @@
##           master    #3649      +/-   ##
==========================================
- Coverage   37.32%   37.31%   -0.01%     
==========================================
  Files         277      284       +7     
  Lines        6685     6692       +7     
  Branches     1213     1206       -7     
==========================================
+ Hits         2495     2497       +2     
- Misses       3532     3544      +12     
+ Partials      658      651       -7

Impacted Files	Coverage Δ
...dit-post/components/sidebar/post-schedule/index.js	50% <ø> (ø)
editor/components/word-count/index.js	0% <ø> (ø)	⬆️
...mponents/editor-global-keyboard-shortcuts/index.js	0% <ø> (ø)	⬆️
editor/store-defaults.js	100% <ø> (ø)	⬆️
editor/components/post-publish-button/label.js	83.33% <ø> (ø)	⬆️
editor/actions.js	51.02% <ø> (+3.85%)	⬆️
...t-post/components/sidebar/page-attributes/index.js	0% <ø> (ø)
editor/components/document-outline/check.js	0% <ø> (ø)	⬆️
editor/components/post-pending-status/index.js	12.5% <ø> (ø)	⬆️
editor/components/post-format/index.js	11.11% <ø> (ø)	⬆️
... and 128 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9864a60...d8a28c2. Read the comment docs.

[gziolo]

I took a quick glance at this code. Looks pretty good.

The "Show block inspector" button proved to be a challenge, because the idea is that clicking this button shows the inspector, but since the BlockInspector is a reusable component that can be shown anyware in the application using the "editor" module, we needed a showBlockInspector to allow the surrouding application (edit-post) to handle this button properly.

I'm not so happy about passing it down to all those components. I will think about some alternatives.

[gziolo]

It looks like some SCSS variables should remain located in editor/ folder.

[youknowriad]

Yeah, we have this issue for all components, these variables and mixins are not modularized properly. They are used in all the modules "components, "block", "editor", "edit-post" and a lot of them are specific to a module.

[gziolo]

We should tackle this separately in that case.

[gziolo]

We discussed it extensively on Slack, but let me summarize my feedback here.

So as I understand, the idea here is to have isolated Redux state for the editor and a separate one for edit-post or any other type that we introduce in the future. It's not ideal because all contributors will have to pass the store name explicitly in the connect method. We might want to create local connect method and/or document it properly. We could add lint option to make sure editor library uses connect method properly.

I’m not happy about onShowInspector is connected at the UI level with Settings button:

It's passed down from edit-post's <Layout /> through several child components. All other toolbar buttons are handled within the editor, this one is not. I guess one of the reasons is because we can’t expose its current state to the edit-post library. Another one is as @youknowriad and @mtias suggested is that some apps might want to display advanced settings in a modal. I think the latter is a good reason to pass down this callback to make it easy to customize. I still hope we can find a solution that will simplify exposing this method to the nested components.

I was exploring if we could reuse pattern from <BlockCount /> to tackle isSidebarOpened flag differently. We could rename it to sth like areAdvancedSettingActive and still keep in the editor's state. We could expose it as <AdvancedSettings /> component and use this way inside edit-post library:

<AdvancedSettings>
        { ( areAdvancedSettingActive, closeAdvancedSettings ) => areAdvancedSettingActive && <Sidebar onClose={ closeAdvancedSettings } /> }
</AdvancedSettings>

This would solve the nesting issue, but the obvious issue here is that it won't play well with the modal case.

It would be nice to gather feedback from @mkaz and @georgeh how this proposal fits with their work on P2 integration. It would be a very valuable to hear from them before we proceed.

[gziolo]

One more thing to consider. Previously only editor library was using Redux libraries. This PR changes that, which will cause both editor and edit-post to bundle the same files twice. We should either make those libs external or put them inside shared wp.store library similat to what wp.element does.

[youknowriad]

I'm closing this right now, I'll do another attempt once the "data" module finalized, which will help with state access accross modules.