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

Blocks: load different bundle in the block-based widget editor #20362

Closed
wants to merge 5 commits into from

Conversation

jeherve
Copy link
Member

@jeherve jeherve commented Jul 14, 2021

Related: #20357

Changes proposed in this Pull Request:

As described in #20357, we cannot use blocks that rely on post information and the @wordpress/editor package in the block-based widget editor that will ship in WordPress 5.8.

We happen to have an existing alternative bundle, that was built for P2, and that had a similar goal: see #15467 and D41999-code for more info. One potential solution to #20357 would be to use that same bundle in the widget editor.
This PR leverages that existing bundle, and updates it to remove the blocks that are problematic in the widget editor.

This has an advantage: we do not have to come up with a new solution.
And some disadvantages:

  • Although named no-post-editor, I believe that bundle mostly meant "No blocks that only appear in the sidebar". I believe we may be able, and want to be using blocks such as our Video block or the Tiled Gallery block on P2. cc @lezama.
  • Additionally, it also means that from now on, we would have to manually maintain the 2 bundles and remove where each one belongs. That same problem was highlighted in Blocks: Add No Post Editor entry point #15467 (comment)
  • It's also a problem for folks using the jetpack_blocks_variation filter to overwrite the block variation used anywhere on a site. Maybe we should force the use of the no-post-editor bundle after that filter has been applied.

Jetpack product discussion

  • p1626278473009100-slack-CBTN58FTJ

Does this pull request change what data or activity we track or use?

  • No

Testing instructions:

  • Start from a site using WP 5.8 RC
  • Go to Appearance > Widgets
  • You should see some Jetpack blocks available there.
  • The Tiled gallery block should not be available.
  • You should see no errors in your logs.

@jeherve jeherve added this to the jetpack/9.9.1 milestone Jul 14, 2021
@jeherve jeherve added [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Pri] High [Status] In Progress [Status] Proposal [Type] Bug When a feature is broken and / or not performing as intended labels Jul 14, 2021
@jeherve jeherve self-assigned this Jul 14, 2021
Copy link

@test-case-reminder test-case-reminder bot left a comment

Choose a reason for hiding this comment

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

Here are some suggested test cases for this PR.

Gutenberg extensions

  • Use Core's block editor
  • Use latest stable Gutenberg plugin

Blocks

  • Tiled Gallery
  • Business Hours
  • Calendly
  • Form
  • Contact Info
  • Eventbrite
  • Google calendar
  • Mailchimp
  • Map
  • OpenTable
  • Pinterest
  • Podcast player
  • Star rating
  • Recurring Payments
  • Repeat Visitor
  • Revue
  • Simple Payments
  • Slideshow

Extensions

  • Publicize
  • Likes

If you think that suggestions should be improved please edit the configuration file here. You can also modify/add test-suites to be used in the configuration file.

@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello jeherve! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D64085-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

@github-actions github-actions bot added the [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ label Jul 14, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Jul 14, 2021

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ All commits were linted before commit.
  • ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖


The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.


Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.


Jetpack plugin:

  • Next scheduled release: February 1, 2022.
  • Scheduled code freeze: January 24, 2022.

@sdixon194
Copy link
Contributor

Thinking out loud, while the error notices are annoying and less than ideal, is it worth only fixing the notice at the expense of the user not being able to add whatever block they want at all? Unless we have more reasons to restrict the kinds of blocks that can be used in the widget editor (although some might not make sense to add to a widget area), I think I'd prefer the ability to add most of them. The blocks themselves still work and don't break anything, just some don't make sense, right?

@sdixon194
Copy link
Contributor

Did we land on a way forward for this? Do we want to revisit this before 10.1, or take it out of the milestone?

@jeherve
Copy link
Member Author

jeherve commented Aug 26, 2021

This would impact P2s, so I'd love to have @lezama's green light before we go ahead with this.

@jeherve jeherve requested a review from lezama August 26, 2021 14:27
@lezama
Copy link
Contributor

lezama commented Aug 26, 2021

Although named no-post-editor, I believe that bundle mostly meant "No blocks that only appear in the sidebar". I believe we may be able, and want to be using blocks such as our Video block or the Tiled Gallery block on P2. cc @lezama.

The p2-editor used to not have the document inspector part, I think that's what no-post-editor was referring to.

I think this should be safe to merge from P2 side.

@jeherve
Copy link
Member Author

jeherve commented Aug 26, 2021

@lezama This would stop us from uploading videos to VideoPress via the P2 editor, is that okay?

@lezama
Copy link
Contributor

lezama commented Aug 26, 2021

@lezama This would stop us from uploading videos to VideoPress via the P2 editor, is that okay?

Not okay 😅 , I see the problem now, I will check what we need to do P2 side and get back to you this week.

@samiff samiff modified the milestones: jetpack/10.1.1, jetpack/10.2 Sep 22, 2021
@samiff
Copy link
Contributor

samiff commented Sep 22, 2021

@lezama This would stop us from uploading videos to VideoPress via the P2 editor, is that okay?

Not okay 😅 , I see the problem now, I will check what we need to do P2 side and get back to you this week.

@lezama Did you have any other feedback on this?

@lezama
Copy link
Contributor

lezama commented Feb 7, 2022

p2editor has sidebar these days, I think this shouldn't be a problem p2 side

@jeherve
Copy link
Member Author

jeherve commented Aug 30, 2022

I'll close this now since we haven't progressed on this imperfect solution lately. I believe we should be able to solve this properly by fixing the problems outlined in #17099.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Pri] High [Status] Proposal Touches WP.com Files [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Editor: solve conflict between Jetpack blocks' @wordpress/editor dependencies and Block-based Widgets Editor
5 participants