Edit Post: Remove SlotFillProvider as rendered by block editor

[aduth]

Extracted from #14715
Unblocks #15949

This pull request cherry-picks the ade6069 commit from #14715. This is necessary both for the reusable blocks embedded editor refactor, and for the custom widgets integration of block editor. Currently, because the BlockEditorProvider component renders the context provider for Slot/Fill, it is not possible to render slots outside the editor. This prevents block inspector rendering as in the #14715 refactoring and Popover rendering as in #15949.

It may be the case that we want to expose a component with "easy" bundled behavior, but it should still be possible to opt out of behaviors in advanced use cases like described above.

#14715 proposes additional enhancements to Slot/Fill behavior to allow whitelisted "proxying" to allow layered Fill capturing. It's not relevant for this pull request, but noteworthy for context.

Testing Instructions:

Verify there are no regressions in Slot/Fill (block inspector, block toolbar, popovers, etc).

[jorgefilipecosta]

Works correctly in my tests, thank you for this change 👍

[youknowriad]

Makes me wonder about this as well :) It feels like it's the same questions could be asked for this provider.

[aduth]

Makes me wonder about this as well :) It feels like it's the same questions could be asked for this provider.

Probably. Let's sort it out separately?

[youknowriad]

Should we add it to the playground as well.

[aduth]

In rebasing this, I sought to add a CHANGELOG mention. In doing so, it occurs to me:

• This is technically a breaking change.
  • It probably won't have much external impact, but in bumping the major, we ought to consider including DropZoneProvider, likely in this same pull request or at least without much delay.
• In crafting the CHANGELOG message, I sought to mention some future addition of a "pre-packaged" editor component, and in so doing describe the purpose of BlockEditorProvider.
  • But what is the purpose of BlockEditorProvider?
  • We have some rough idea, but a README.md to document the component would force us to consider it explicitly
  • If we move out these providers, its purpose is mostly in helping manage value and onChange sync. This isn't really what "Provider" suffix would indicate that it does.
  • Instead, we could implement the sync behavior as a separate component, which BlockEditorProvider uses. If someone wanted more granular control of provider contexts (Block Library: Reimplement Reusable Block using EditorProvider for embedded post editor #14715, Fix: Popover positions on the widget screen #15949), they too could use this same sync component and add contexts if/where needed. In other words, they wouldn't use BlockEditorProvider, and BlockEditorProvider could continue to render all of the provider contexts typically necessary for "pre-packaged" use. It also avoids considering it a breaking change.

Thoughts on this last point?

[aduth]

Note to self: Consider how to reflect chosen approach in the native variant of the component:

gutenberg/packages/block-editor/src/components/provider/index.native.js

Line 130 in e9e3565

<SlotFillProvider>

[aduth]

In reflecting on this and discussing with @jorgefilipecosta , I'm thinking to suggest the following alternative, which is almost the opposite of what was originally proposed here:

• BlockEditorProvider effectively remains unchanged. The component's purpose is to establish the context expected to be necessary for an operable standalone editor.
• A new BlockEditor component is exposed from @wordpress/block-editor, which includes only the behaviors of syncing values via value and onChange / onInput.
• The reusable blocks refactor of Block Library: Reimplement Reusable Block using EditorProvider for embedded post editor #14715 and widgets screen requirements of Fix: Popover positions on the widget screen #15949 use BlockEditor, establishing their own DropZone and Slot/Fill context wherever and if-ever needed.
• BlockEditorProvider's implementation effectively becomes:
  • <DropZoneProvider><SlotFillProvider><BlockEditor { ...props } /></SlotFillProvider></DropZoneProvider>
  • Potentially other "default" providers could be added here in the future.

Proposed benefits:

• No breaking changes, only a "New Feature" in the introduction of the new component
• BlockEditorProvider maintains the semantics attached to the "Provider" suffix as to mean that it establishes necessary context
• BlockEditor becomes the simpler, stripped-down component for use as dictated by requirements of consuming implementer
  • Thus far, attempts to try to establish an abstraction have fallen short, indicating that the customization requirements supersede the viability of a common interface, or that we've not yet come to settle on what these commonalities are. I think it's fairer to start from the basics and observe / evaluate with later enhancements to BlockEditorProvider, if necessary.

Possible downsides:

• Possibly unclear distinction between BlockEditorProvider and BlockEditor (as explained above, the semantic distinction exists, but it's unknown whether this is obvious)
• The default BlockEditor component becomes more "difficult" to use out-of-the-box

[youknowriad]

@aduth yes, in my mind I'd have suggested to switch BlockEditorProvider and BlockEditor as proposed here 🤷‍♂ In my mind BlockEditor could be a component that renders and ready-2-use block editor (I can imagine even including UI) while BlockEditorProvider being the more flexible one.

Not that I disagree a lot but the reasoning to switch those as proposed is not clear.

[aduth]

Not that I disagree a lot but the reasoning to switch those as proposed is not clear.

I was hoping my last comment would serve to exhaustively demonstrate why 😄 The problem for me (aside from the fact that it's a breaking change) is that, looking at the source of BlockEditorProvider, if you take away the DropZoneProvider and the SlotFillProvider, what you're left with... is not a provider, at least in the understood sense of establishing context. All it's doing is managing value syncing. To me, this is the fundamental behavior, which we represent by the fundamental base element BlockEditor. In its name implying to establish some context, BlockEditorProvider remains to assign these context provider wrapper elements.

[youknowriad]

@aduth

That you're left with... is not a provider, at least in the understood sense of establishing context. All it's doing is managing value syncing. To me, this is the fundamental behavior

I think I disagree with this, it's still establishing a context (maybe it's a little bit more implicit) but it is creating a data store and even creating RegistryProvider on the default case.

[aduth]

Oh, I totally missed withRegistryProvider 🤦‍♂ Which, in retrospect, is the primary context we'd want to be established. Would you propose then that we return to the original plan, and remove both SlotFillProvider and DropZoneProvider from BlockEditorProvider ?

[youknowriad]

Would you propose then that we return to the original plan, and remove both SlotFillProvider and DropZoneProvider from BlockEditorProvider ?

yes, I'd prefer that if possible :)

[jorgefilipecosta]

So just to recap the plan is to have:
- BlockEditorProvider - that now is changed to not contain SlotFillProvider, and DropZoneProvider. To be done in this PR.
- BlockEditor component that contains the block editor provider and maybe other providers (SlotFillProvider, and DropZoneProvider) and UI parts. The component is supposed to provide a simple to use block editor. Can be done in other PR.

Is that the case?

[aduth]

@jorgefilipecosta Right. I'm not sure what (if any) form BlockEditor would take at this point, but it's not a problem we need to solve today.

I have an incomplete rebase toward the original propose, with some additional changes I will try to push up before my end of day.

[aduth]

Okay, pushed rebase, including:

• Resolved conflicts
• Removed DropZoneProvider as rendered by BlockEditorProvider
• Include CHANGELOG notes
• Restore behavior to Playground

[youknowriad]

This should be moved to ## Master I guess.