Skip to content

Gutenframe: Re-enable links to manage reusable blocks#12412

Merged
kraftbj merged 2 commits intomasterfrom
update/re-enable-reusable-blocks-links
May 20, 2019
Merged

Gutenframe: Re-enable links to manage reusable blocks#12412
kraftbj merged 2 commits intomasterfrom
update/re-enable-reusable-blocks-links

Conversation

@codebykat
Copy link
Copy Markdown
Contributor

@codebykat codebykat commented May 18, 2019 •
edited
Loading

In D26034-code we unhid the Manage Reusable Blocks links on WPCOM Simple Sites by removing some styles from Calypsoify that were hiding them. These changes now need to be synced to Jetpack.

Fixes Automattic/wp-calypso#32746

Changes proposed in this Pull Request:

  • Re-enable links to manage reusable blocks

Is this a new feature or does it add/remove features to an existing part of Jetpack?

  • Re-enables a feature in the block editor

Testing instructions:

  • Load a site with this patch applied within the Gutenberg iframe and open the block editor.
  • Add a block to reusable blocks if you don't have one already
  • Verify that links to manage reusable blocks are present in the sidebar and in the add block menu:

  • Verify that both links go to the Calypso block manager at /types/wp_block/:siteSlug
  • In wp-admin's block editor, these links should still be present, and go to wp-admin/edit.php?post_type=wp_block

Proposed changelog entry for your changes:

  • Block editor: Allow managing reusable blocks when iframed in Calypso

@codebykat codebykat added [Status] Needs Review This PR is ready for review. [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Feature] Calypsoify labels May 18, 2019
@codebykat codebykat requested a review from a team May 18, 2019 00:52
@codebykat codebykat self-assigned this May 18, 2019
@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented May 18, 2019 •
edited
Loading

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: June 4, 2019.
Scheduled code freeze: May 28, 2019

Generated by 🚫 dangerJS against 82af96c

@codebykat
Copy link
Copy Markdown
Contributor Author

codebykat commented May 18, 2019

Two potential issues:

  • The first time I visited the reusable block section in Calypso, I had a WSOD and some kind of an error about ownProps not existing. Unfortunately I didn't have persist error logs in the console and I haven't been able to reproduce it.
  • On hover, the link in the add a block dropdown shows the URL to wp-admin, even though clicking it takes you to Calypso. This glitch is also present for WPCOM Simple Sites right now so it can probably be addressed in a follow-up PR if desired.

@gwwar
Copy link
Copy Markdown
Contributor

gwwar commented May 18, 2019 •
edited
Loading

As a general question, @Automattic/lannister when would we use modules/calypsoify/mods-gutenberg.js over the common folder in https://github.com/Automattic/wp-calypso/tree/master/apps/wpcom-block-editor ?

Are there a specific cases we need to be aware of, or do we need more follow up janitorials?

@mmtr
Copy link
Copy Markdown
Member

mmtr commented May 18, 2019 •
edited
Loading

As a general question, @Automattic/lannister when would we use modules/calypsoify/mods-gutenberg.js over the common folder in https://github.com/Automattic/wp-calypso/tree/master/apps/wpcom-block-editor ?

Are there a specific cases we need to be aware of, or do we need more follow up janitorials?

We just need to open more janitorial PRs. I didn't notice this logic when I moved the code from gutenberg-wpcom.js to @automattic/wpcom-block-editor.

In general, I think we should avoid having logic in calypsoify/mods-gutenberg.js to decrease confusion and maintenance cost. We can include logic in @automattic/wpcom-block-editor specific to Gutenframe only (in the calypso folder) or to the editor in general (both Gutenframe and WP Admin, in the common folder) and one single patch will update both Simple and Jetpack sites without requiring Fusion/manual sync.

Anyway, not a blocker for this PR. We can move the "Manage reusable blocks" custom code to @automattic/wpcom-block-editor once the logic is sync in both WP.com/Jetpack. See comment below.

@codebykat codebykat requested a review from a team May 18, 2019 02:00
mmtr
mmtr reviewed May 18, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@mmtr mmtr left a comment

Choose a reason for hiding this comment

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

On a second look, I remembered we already moved this logic to the @automattic/wpcom-block-editor package, so I think we only need to keep the changes on class.jetpack-calypsoify.php and style-gutenberg.scss.

Changes in mods-gutenberg.js are likely not needed so the file can be reverted.

@codebykat
Copy link
Copy Markdown
Contributor Author

codebykat commented May 18, 2019

Aha I reproduced the ownProps error, but I don't know if it was caused by this PR.

TypeError: ownProps.query is null posts-custom~posts-pages.js line 396 > eval:327:16
    default app:///./client/my-sites/post-type-list/index.jsx:327
    React 29
    hydrate app:///./client/controller/index.web.js:142
    middleware app:///./node_modules/page/page.js:1163
    nextEnter app:///./node_modules/page/page.js:712
    makeLayoutMiddleware app:///./client/controller/shared.js:43
    middleware app:///./node_modules/page/page.js:1163
    nextEnter app:///./node_modules/page/page.js:712
    list app:///./client/my-sites/types/controller.jsx:34
    middleware app:///./node_modules/page/page.js:1163
    nextEnter app:///./node_modules/page/page.js:712
    navigation app:///./client/my-sites/controller.js:413
    middleware app:///./node_modules/page/page.js:1163
    nextEnter app:///./node_modules/page/page.js:712
    siteSelection app:///./client/my-sites/controller.js:381
    run app:///./node_modules/core-js/library/modules/es6.promise.js:75
    notify app:///./node_modules/core-js/library/modules/es6.promise.js:92
    flush app:///./node_modules/core-js/library/modules/_microtask.js:18
The above error occurred in the <Connect(Localized(PostTypeList))> component:
    in Connect(Localized(PostTypeList)) (created by Types)
    in main (created by Main)
    in Main (created by Types)
    in Types (created by Connect(Types))
    in Connect(Types)
    in div (created by Layout)
    in div (created by Layout)
    in div (created by Layout)
    in Layout (created by Connect(Layout))
    in Connect(Layout) (created by ReduxWrappedLayout)
    in MomentProvider (created by Connect(MomentProvider))
    in Connect(MomentProvider) (created by ReduxWrappedLayout)
    in Provider (created by ReduxWrappedLayout)
    in ReduxWrappedLayout
Unhandled promise rejection TypeError: "ownProps.query is null"
    default app:///./client/my-sites/post-type-list/index.jsx:327
    React 25
    hydrate app:///./client/controller/index.web.js:142
    middleware app:///./node_modules/page/page.js:1163
    nextEnter app:///./node_modules/page/page.js:712
    makeLayoutMiddleware app:///./client/controller/shared.js:43
    middleware app:///./node_modules/page/page.js:1163
    nextEnter app:///./node_modules/page/page.js:712
    list app:///./client/my-sites/types/controller.jsx:34
    middleware app:///./node_modules/page/page.js:1163
    nextEnter app:///./node_modules/page/page.js:712
    navigation app:///./client/my-sites/controller.js:413
    middleware app:///./node_modules/page/page.js:1163
    nextEnter app:///./node_modules/page/page.js:712
    siteSelection app:///./client/my-sites/controller.js:381
    run app:///./node_modules/core-js/library/modules/es6.promise.js:75
    notify app:///./node_modules/core-js/library/modules/es6.promise.js:92
    flush app:///./node_modules/core-js/library/modules/_microtask.js:18
index.jsx:94:7

@codebykat
Copy link
Copy Markdown
Contributor Author

codebykat commented May 18, 2019

Another glitchy behavior here is you can't open the link to manage blocks in a background tab. If I CMD+click it loads in the same tab, and if I right-click and "Open Link in New Tab" it goes to the wp-admin link.

@codebykat
Copy link
Copy Markdown
Contributor Author

codebykat commented May 18, 2019

This error occurs on Calypso master visiting a Simple Site's reusable block page, so I think it's not related to this PR. Up for review again...

@codebykat codebykat requested a review from a team May 18, 2019 19:03
@jeherve jeherve added this to the 7.4 milestone May 20, 2019
jeherve
jeherve approved these changes May 20, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

This works well in my tests. 👍

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels May 20, 2019
@jeherve
Copy link
Copy Markdown
Member

jeherve commented May 20, 2019

The first time I visited the reusable block section in Calypso, I had a WSOD and some kind of an error about ownProps not existing. Unfortunately I didn't have persist error logs in the console and I haven't been able to reproduce it.

Noting that I experienced the same thing with a brand new site. After a few minutes the error disappeared. This happens in other places in Calypso when you try to access Calypso pages right after connecting your site to your WordPress.com.

@kraftbj
Copy link
Copy Markdown
Contributor

kraftbj commented May 20, 2019

Nice, works for me as well. Merging.

@kraftbj kraftbj merged commit 478935e into master May 20, 2019
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels May 20, 2019
@kraftbj kraftbj deleted the update/re-enable-reusable-blocks-links branch May 20, 2019 17:24
jeherve added a commit that referenced this pull request May 23, 2019
@jeherve
Changelog: add #12412
fa5ba90
jeherve added a commit that referenced this pull request May 27, 2019
@kraftbj @jeherve
7.4 Changelog (#12304)
0fb4436 
* Kick off the changelog

* Add 7.3.1

* Update date and post link

* changelog: add #12219

* changelog: add #12170

* changelog: add #12184

* Changelog: add #12268

* Changelog: add #12081

* Changelog: add #12323

* Changelog: add #12204

* Changelog: add #12269

* Changelog: add #12332

* changelog: add #12339

* changelog: add #12209

* Changelog: add #12319

* Changelog: add #12357

* Changelog: add #12124

* Changelog: add #12373

* Changelog: add #12252

* Changelog: add #12383

* Changelog: add #12372

* changelog: add #12337

* Changelog: add #12290

* Changelog: add #12301

* Changelog: add #12061

* Testing list: add instructions for #12061

* Changelog: add #12393

* Update minimum supported version

See #12287

* Changelog: add #12406

* Testing list: add #12406

* Changelog: add #12277

* Changelog: add #12412

* Changelog: add #11318

* Changelog: add #12328

* Changelog: add #12425

* Changelog: add #12380

* Changelog: add #12428

* Changelog: add #12414

* Changelog: add #12395

* Changelog & Testing list: add #12416, #12417, #12418, and #12348

* changelog: add #12379

* Changelog: add #12341

* changelog: add #12444

* Changelog: add #12434

* Changelog: add #12454

* Changelog: add #12460

* Changelog: add #12463

* Changelog: add #12457

* Changelog / testing list: add #10333

* Changelog: add #12467


Co-authored-by: Jeremy Herve <jeremy@jeremy.hu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mmtr mmtr mmtr left review comments

@jeherve jeherve jeherve approved these changes

Assignees

@codebykat codebykat

Labels

[Feature] Calypsoify [Feature] WordPress.com Block Editor [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack

Projects

None yet

Milestone

7.4

Development

Successfully merging this pull request may close these issues.

Gutenframe: Manage Reusable Blocks links not visible on Jetpack and Atomic sites [5]

7 participants

@codebykat @jetpackbot @gwwar @mmtr @jeherve @kraftbj @matticbot