Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gutenberg: start editor shell in Calypso #26438

Merged
merged 13 commits into from
Aug 10, 2018
Merged

Gutenberg: start editor shell in Calypso #26438

merged 13 commits into from
Aug 10, 2018

Conversation

vindl
Copy link
Member

@vindl vindl commented Aug 1, 2018

Part of #26180

Sets up a Calypso test branch for @wordpress/editor package and starts a Gutenberg editor shell in the new /gutenberg section.

Folders in this PR that, for the most part, don't need to be reviewed (copied code from Gutenberg editor):

  • edit-post - In order to replicate the functionality in initializeEditor, I copied over most of the components from Gutenberg's edit-post folder (won't be exposed as a package in the future).
  • core-blocks - Since core-blocks package is not published yet, some of the core-blocks from Gutenberg have been ported here in order to support registerCoreBlocks functionality. This is a temporary addition that will be removed once the package is published.

Remaining tasks

@wordpress/editor sub-dependencies:

  • @wordpress/a11y
  • @wordpress/api-fetch
  • @wordpress/blob
  • @wordpress/blocks
  • @wordpress/components
  • @wordpress/compose
  • @wordpress/core-data
  • @wordpress/data
  • @wordpress/date
  • @wordpress/deprecated
  • @wordpress/dom
  • @wordpress/element
  • @wordpress/hooks
  • @wordpress/html-entities
  • @wordpress/i18n
  • @wordpress/is-shallow-equal
  • @wordpress/keycodes
  • @wordpress/nux
  • @wordpress/url
  • @wordpress/viewport
  • @wordpress/wordcount

Bundle sizes delta

chunk                                                                                                           stat_size              parsed_size              gzip_size
async-load-design                                                                                                +32977 B    (+10.3%)     +15302 B     (+7.1%)    +3915 B    (+10.6%)
async-load-design-blocks                                                                                         +34182 B     (+2.5%)     +18788 B     (+2.6%)    +3695 B     (+2.4%)
async-load-design-blocks~checkout~domains~purchases~woocommerce                                                  -34182 B   (deleted)     -18854 B   (deleted)    -4609 B   (deleted)
async-load-design-playground                                                                                     +32977 B    (+12.0%)     +15302 B     (+8.3%)    +3832 B    (+11.5%)
async-load-design-typography                                                                                         +0 B                     +0 B                   -1 B     (-0.1%)
async-load-design~async-load-design-playground~google-my-business                                                -32977 B   (deleted)     -15364 B   (deleted)    -4390 B   (deleted)
async-load-docs-selectors                                                                                            +0 B                     +0 B                   -2 B     (-0.1%)
async-load-extensions-woocommerce-app-store-stats-referrers                                                          +0 B                     +0 B                   +1 B     (+0.0%)
async-load-lib-rubberband-scroll-disable                                                                             +0 B                     +0 B                   +1 B     (+0.3%)
async-load-post-editor-editor-discussion                                                                             +0 B                     +0 B                   -1 B     (-0.1%)
async-load-post-editor-editor-location                                                                               +0 B                     +0 B                   +1 B     (+0.0%)
async-load-post-editor-editor-seo-accordion                                                                          +0 B                     -1 B     (-0.0%)       -1 B     (-0.0%)
async-load-post-editor-media-modal                                                                              +105199 B   (+333.2%)     +48136 B   (+205.6%)   +11647 B   (+150.5%)
async-load-post-editor-media-modal~post-editor                                                                  -105199 B   (deleted)     -48197 B   (deleted)   -12572 B   (deleted)
async-load-reader-conversations-stream                                                                               +0 B                     +0 B                   -3 B     (-0.2%)
async-load-reader-feed-stream                                                                                        +0 B                     +0 B                   -1 B     (-0.1%)
async-load-reader-search-stream                                                                                      +0 B                     +0 B                   +1 B     (+0.0%)
async-load-reader-team-main                                                                                          +0 B                     +0 B                   -1 B     (-0.4%)
build                                                                                                                +0 B                    +43 B     (+0.0%)       +8 B     (+0.0%)
checklist                                                                                                            +0 B                     +0 B                   -1 B     (-0.0%)
devdocs                                                                                                              +0 B                     +9 B     (+0.0%)       -2 B     (-0.0%)
domains                                                                                                              +0 B                     +0 B                   -1 B     (-0.0%)
google-my-business                                                                                               +32977 B    (+25.6%)     +15302 B    (+23.7%)    +3552 B    (+24.2%)
gutenberg-editor                                                                                                +108628 B  (+4779.1%)     +42580 B  (+3228.2%)   +10220 B  (+1765.1%)
happychat                                                                                                            +0 B                     +0 B                   -1 B     (-0.1%)
mailing-lists                                                                                                        +0 B                     +0 B                   -1 B     (-0.1%)
manifest                                                                                                             +0 B                    +79 B     (+0.5%)      +29 B     (+0.5%)
me                                                                                                                   +0 B                     +0 B                   -1 B     (-0.2%)
post-editor                                                                                                     +105199 B    (+10.5%)     +48127 B    (+10.5%)   +11908 B    (+10.4%)
posts-custom                                                                                                         +0 B                     +0 B                   +1 B     (+0.1%)
posts-custom~posts-pages                                                                                             +0 B                     +0 B                   -1 B     (-0.0%)
preview                                                                                                              +0 B                     +0 B                   -1 B     (-0.1%)
reader                                                                                                               +0 B                     +0 B                   -1 B     (-0.0%)
settings                                                                                                             +0 B                    -16 B     (-0.0%)       -7 B     (-0.0%)
settings-writing                                                                                                     +0 B                    -16 B     (-0.0%)       -5 B     (-0.0%)
vendors~account~checkout~comments~domains~signup~stats                                                               +0 B                     +0 B                   +1 B     (+0.0%)
vendors~async-load-components-web-preview-component                                                                  +0 B                     +0 B                   -1 B     (-0.0%)
vendors~async-load-design-blocks                                                                                     +0 B                     +0 B                   -1 B     (-0.0%)
vendors~async-load-design~async-load-design-blocks~async-load-design-gutenberg-components~async-load~3d27eab9  -1228924 B    (-47.9%)    -528848 B    (-50.9%)  -138971 B    (-50.2%)
vendors~async-load-lib-happychat-connection                                                                          +0 B                     +0 B                   -1 B     (-0.0%)
vendors~async-load-post-editor-media-modal~media~post-editor                                                         +0 B                     +0 B                   +1 B     (+0.0%)
vendors~build                                                                                                        +0 B                   +156 B     (+0.0%)     +523 B     (+0.2%)
vendors~devdocs                                                                                                      +0 B                     +0 B                   +1 B     (+0.0%)
vendors~post-editor                                                                                                  +0 B                     +0 B                   -1 B     (-0.0%)
vendors~post-editor~woocommerce                                                                                 -911202 B    (-72.2%)    -355865 B    (-70.5%)  -118419 B    (-72.0%)
woocommerce                                                                                                          +0 B                     +0 B                   -1 B     (-0.0%)
wp-job-manager                                                                                                       +0 B                     +0 B                   +2 B     (+0.0%)
checkout~domains~purchases~woocommerce                                                                           +34182 B       (new)     +18854 B       (new)    +4609 B       (new)
vendors~async-load-design~async-load-design-blocks~async-load-design-gutenberg-components~async-load~bcad13b4  +1232073 B       (new)    +538309 B       (new)  +142923 B       (new)
vendors~gutenberg-editor                                                                                       +1733965 B       (new)    +691067 B       (new)  +170598 B       (new)
vendors~gutenberg-editor~post-editor~woocommerce                                                                +911202 B       (new)    +355927 B       (new)  +119005 B       (new)

Testing instructions

@vindl vindl added [Status] In Progress [Goal] Gutenberg Working towards full integration with Gutenberg labels Aug 1, 2018
@vindl vindl self-assigned this Aug 1, 2018
@vindl vindl requested a review from a team August 1, 2018 11:29
@matticbot
Copy link
Contributor

matticbot commented Aug 1, 2018

@vindl vindl mentioned this pull request Aug 1, 2018
27 tasks
@@ -247,7 +247,7 @@ if ( calypsoEnv === 'desktop' ) {
} else {
// jquery is only needed in the build for the desktop app
// see electron bug: https://github.com/atom/electron/issues/254
webpackConfig.externals.push( 'jquery' );
// webpackConfig.externals.push( 'jquery' );
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm running into:

ReferenceError: jquery is not defined
    at eval (external_"jquery":1)
    at Object.jquery (build.js:14586)
    at __webpack_require__ (manifest.js:788)
    at fn (manifest.js:151)
    at eval (nonce.js:4)
    at Module../node_modules/@wordpress/api-fetch/build-module/middlewares/nonce.js (vendors~gutenberg-editor.js:249)
    at __webpack_require__ (manifest.js:788)
    at fn (manifest.js:151)
    at eval (index.js:11)
    at Module../node_modules/@wordpress/api-fetch/build-module/index.js (vendors~gutenberg-editor.js:213)

Copy link
Member

Choose a reason for hiding this comment

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

hm. this could be related to the fact that it's an npm package depending on jQuery and not Calypso code.

@blowery @ockham haven't we run into this before? do we have to try and "transpile the non-transpired" dependency here?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's right, some of the Gutenberg packages (e.g. @wordpress/api-fetch) still depend on jQuery.

Copy link
Contributor

@gwwar gwwar Aug 1, 2018

Choose a reason for hiding this comment

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

See related work in WordPress/gutenberg#8311 to remove it

Copy link
Member

Choose a reason for hiding this comment

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

I should test, land and publish new version which is jquery free later today.

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed this after updating to latest package version.

@vindl vindl force-pushed the try/wordpress-editor branch 5 times, most recently from c2c5fd8 to b7e02b2 Compare August 1, 2018 16:09
@gwwar
Copy link
Contributor

gwwar commented Aug 1, 2018

Yay no console errors at least 🎉

Some high level notes:

  • I wonder if the editor component is expecting anything else from the app shell. I'm not seeing anything in the UI, unless registerCoreBlocks does a lot more, to trigger editor init.
  • Don't forget to pull in styles from the @wordpress/editor node_modules package. We normally put these in _vendor.scss, but we may want to put it in the gutenberg section for now. Once we see a UI, we'll want to verify if we run into any CSS namespace issues.
  • SinceregisterCoreBlocks from core-blocks isn't available yet, could we try adding custom block registration? Maybe we can temporarily register a p block for example:
registerBlockType( myTestBlock, settings );
setDefaultBlockName( myTestBlock.name );
setUnknownTypeHandlerName( myTestBlock.name );

Feel free to DM me if you need any more detail on these 👍

@gwwar gwwar requested review from youknowriad and gziolo August 1, 2018 22:59
@gziolo
Copy link
Member

gziolo commented Aug 2, 2018

@gwwar when not sure, it is always the best to look at the following scripts and styles setup logic:

https://github.com/WordPress/gutenberg/blob/master/lib/client-assets.php#L1091-L1313

In particular, this part is something that you are looking for:

$init_script = <<<JS
	( function() {
		var editorSettings = %s;
		window._wpLoadGutenbergEditor = new Promise( function( resolve ) {
			wp.api.init().then( function() {
				wp.domReady( function() {
					resolve( wp.editPost.initializeEditor( 'editor', "%s", %d, editorSettings, window._wpGutenbergDefaultPost ) );
				} );
			} );
		} );
} )();
JS;

I think, you nedd to replicate what wp.editPost.initializeEditor does:

https://github.com/WordPress/gutenberg/blob/master/edit-post/index.js#L58-L89

in fact the only thing which is mandaroty is render in there. I'm sure you don't need metaboxes, nux is also optional. You can't register core blocks because they aren't published to npm so using test blocks is enough.


const editorSettings = {};

const wpApiSettings = {
Copy link
Member

Choose a reason for hiding this comment

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

You no longer need to pass wpApiSettings variable.

@gwwar gwwar requested a review from jsnajdr August 2, 2018 18:13
@gwwar
Copy link
Contributor

gwwar commented Aug 2, 2018

@gwwar when not sure, it is always the best to look at the following scripts and styles setup logic:

@gziolo With limited time, I'm not afraid of asking for help 😄 Thanks, I think this is a good starting point for us.

Another quick question though, it sounds like we can skip the wp.api.init() promise? Looks like this is the client init for wp/v2 though @Copons feel free to correct me.

screen shot 2018-08-02 at 11 18 04 am

@vindl vindl force-pushed the try/wordpress-editor branch 3 times, most recently from 4f8f83b to 0ef7b71 Compare August 3, 2018 01:55
@gziolo
Copy link
Member

gziolo commented Aug 3, 2018

I don't know about your use case if you need wp.api.init or not. However, if you want to ensure that plugins can register customization for blocks and editor itself, you should be using wp.domReady or equivalent solution.

@Copons
Copy link
Contributor

Copons commented Aug 3, 2018

Another quick question though, it sounds like we can skip the wp.api.init() promise? Looks like this is the client init for wp/v2

@gwwar That's the whole REST API initialization done by core, it doesn't have anything to do with Gutenberg.

https://github.com/WordPress/WordPress/blob/947a12f2b2284271db9a43a9ba975419db921873/wp-includes/js/wp-api.js#L1493-L1536

@vindl vindl force-pushed the try/wordpress-editor branch from 0ef7b71 to 1f6dc42 Compare August 3, 2018 12:38
@gwwar
Copy link
Contributor

gwwar commented Aug 3, 2018

Thanks for the extra context @Copons!

@vindl vindl force-pushed the try/wordpress-editor branch 3 times, most recently from e0c79fb to 55f455e Compare August 7, 2018 21:44
@gwwar
Copy link
Contributor

gwwar commented Aug 8, 2018

@vindl I dug a bit further, and it looks like the following stores are registered, in the current sequence, so we are missing core/editor. Right now things are registered during import, which feels more like a side effect 🤷‍♀️ .

screen shot 2018-08-07 at 5 29 00 pm
You can see this output from adding a console.log here, or adding a debugger; statement if you'd like to step through
screen shot 2018-08-07 at 5 29 18 pm

Next steps would be to compare the store registration sequence in Gutenberg master, and try to mirror that here.

@vindl vindl force-pushed the try/wordpress-editor branch 4 times, most recently from 2fa6510 to de33f4c Compare August 9, 2018 03:24
@vindl
Copy link
Member Author

vindl commented Aug 9, 2018

After some latest fixes I managed to get past the blank screen and display the UI elements, although the styling seems broken. I updated the PR summary with additional details and remaining tasks.

@vindl vindl force-pushed the try/wordpress-editor branch 2 times, most recently from 6aa9c78 to 361da77 Compare August 10, 2018 00:05
Copy link
Contributor

@gwwar gwwar left a comment

Choose a reason for hiding this comment

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

@vindl nice work! This is a good start, and it's exciting to see brand new errors. Let's land this piece and we can create follow up issues to tackle issues like API authentication, styling, squashing misc bugs etc.

This is good to 🚢 after we verify that prod bundles aren't affected too much.

@vindl vindl force-pushed the try/wordpress-editor branch from 7fcdf62 to d02743a Compare August 10, 2018 02:28
@import 'gutenberg/editor/core-blocks/heading/editor';

// Edit-post components styles
@import 'gutenberg/editor/edit-post/assets/stylesheets/colors';
Copy link
Member

Choose a reason for hiding this comment

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

SCSS variables and helpers from the assets folder could probably be published to npm as its own package. What do you think?

Copy link
Member Author

@vindl vindl Aug 10, 2018

Choose a reason for hiding this comment

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

It would probably help us in the short term, but given that we are supposed to re-implement Calypso specific versions of edit post components, I'm not sure if we'll be reusing them in the long run.

Copy link
Member

Choose a reason for hiding this comment

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

not worth the effort then

Copy link
Contributor

Choose a reason for hiding this comment

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

Depends on if core wants to eventually publish edit-post we'll untangle this in a bit otherwise in follow up PRs.

*/
import MetaBoxesArea from './meta-boxes-area';

function MetaBoxes( { location, isActive } ) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if we want to support meta boxes in Calypso at all :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Most likely not, I'm leaving the detailed clean up of the edit-post folder for the follow-up. :)


const replaceMediaUpload = () => null;

addFilter(
Copy link
Member

Choose a reason for hiding this comment

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

This is going to be the way to hook in Calypso's custom Media Gallery implementation.

Copy link
Member

Choose a reason for hiding this comment

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

There is a similar hook for Featured Image, however I guess you won't need it since you will build your own sidebar

@vindl vindl force-pushed the try/wordpress-editor branch from d02743a to a11e6fd Compare August 10, 2018 08:41
@vindl vindl merged commit 897febf into master Aug 10, 2018
@vindl vindl deleted the try/wordpress-editor branch August 10, 2018 12:12
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Goal] Gutenberg Working towards full integration with Gutenberg
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants