-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Stabilize Block Editor APIs used by Isolated Block Editor #57672
Comments
This is a quick message as I can't reply entirely right now. But my opinion is that we should deprecate the isolated block editor. Using raw block-editor should be easy enough that the isolated block editor shouldn't be needed. That's what I tried to convey with my several playground stories. That said, I'll circle back here and try to reply to each of the APIs proposed here. |
That's what the isolated block editor essentially is. It imports various pieces from <InterfaceSkeleton
secondarySidebar={
isList ? <ListView /> : <Library />
}
content={
<KeyboardShortcuts>
<BlockTools>
<BlockCanvas shouldIframe={ false } contentRef={ useTypewriter() }>
<LayoutStyle />
<LayoutStyle />
<RecursionProvider>
<BlockList />
</RecursionProvider>
</BlockCanvas>
</BlockTools>
</KeyboardShortcuts>
}
/> The problem is that half of these pieces are private/experimental APIs. With the recent GaaF work taken into account, is this editor code going to look fundamentally different? Or at least greatly simplified?
What does "playground stories" mean? 🙂 |
@jsnajdr I believe @youknowriad is referring to https://github.com/WordPress/gutenberg/blob/trunk/storybook/stories/playground/with-undo-redo/index.js which shows the ongoing efforts of #53874 I think the important distinction between the Isolated Block Editor and "Block Canvas" work done in #53874 is the additional UI provided by the Isolated Block Editor and own internal stores for managing its own sidebars, etc. I have personally moved away from the Isolated Block Editor with the progress made in |
Thanks for the pointer! Did you have to import any private/experimental APIs from the I don't mind deprecating the isolated block editor package and building the editor from the "raw" block editor packages. Namely for the P2 editor. But so far it seems to me that it's going to be essentially the same code, just in different repo. Even if it gets to be much simpler, but that simplification can be done in the original isolated block editor package, too. |
@jsnajdr Yes, there are still a few components that could be stabilized. I ended up with: import {
BlockCanvas,
BlockEditorProvider,
BlockList,
store as blockEditorStore,
__experimentalListView as ListView,
__experimentalLibrary as InserterView,
} from '@wordpress/block-editor'; Edit: To to be fair, that is to build the main block editor area. Other things like building a sidebar with new |
I took a look at
Regarding its APIs:
A few considerations:
I'm not sure if Ariakit offers via its public APIs an easy drop-in replacement for the logic of the editor (cc @diegohaz ). |
There's a import { forwardRef } from "react";
import { useDialog } from "@ariakit/react-core/dialog/dialog";
import { Role } from "@ariakit/react";
const Dialog = forwardRef(function Dialog(props, ref) {
const roleProps = useDialog({ ref, ...props });
return <Role.div {...roleProps} />;
}); Besides not following semver, I don't believe it can work as a drop-in substitute because the hook might yield custom props that |
Hi @youknowriad 👋 Pinging you here as a reminder if you still want to share your feedback. Especially after the Drupal workshop which was also about integrating the editor in an "unusual" way. |
Hi @jsnajdr Sorry for the late response.
I intentionally kept these props private. They were needed because of our weird implementations and backward compatibility needs in WordPress but from a third-party developer perspective, I would love for the iframe to be default (no need to provide a choice), and for the other props to be not needed. I didn't want to commit to them as API because they felt very unclear from an implementer's perspective.
This for me also shouldn't be something the implementor thinks about. So maybe this one could be a prop of
This feels like an API that blocks should be using rather than the editor. I know that we have one exception to this (the root level post content) but I guess that's fine too. I agree that it feels like a stable enough API at this point. cc @mcsf
I'm torn here. I don't think it should be integrated to BlockCanvas because it would mean that the block editor gains knowledge of devices/breakpoints... which has a lot of complexities to it. Right now I see it as an API to be used in the
I think passing a "layout" to
Yes, this one is an Inserter without "toggle". I think it's fine to stabilize. Some of the __ex props need to be checked and potentially stabilized too.
Yes, same but it does feel like it has way too much props. Probably some of these are very specific and don't make sense in a good reusable component. I hope this helps, I do think we should shift and consider the IsolatedBlockEditor as deprecated though. If there's too much boilerplate to use BlockEditor directly, we should fix these instead and provide examples in our storybook directly. (And update the platform docs as well) |
Currently the isolated block editor is used by the P2 theme, to support inline posting and commenting. If we deprecate it, the equivalent P2 code will have exactly the same problems, they will not disappear. Another similar integration project is "Verbum", a WordPress.com feature to provide inline Gutenberg commenting on site frontends. It's interesting that they are able to almost completely avoid unstable API, for a very similar functionality. There are a few prefixed fields in the editor |
I'm just saying that P2 should be using block-editor directly in this case. There's no need for middle solution. It doesn't negate the fact that we need to stabilize stuff... |
Are you suggesting that it live in |
Probably not, it's completely normal that 3rd party blocks import APIs for block authors from |
As someone who is working on upgrading Gutenberg on P2 (for the first time), and also having to fiddle with isolated-block-editor to update it fully (because we have weird issues on the frontend editor, where the isolated block editor is used in P2). I want to ask: Are you sure about this, @youknowriad? I'm asking because, despite the progress on #53874, I'm not sure how easy it would be to make P2 use the block editor directly or if maybe we should still have the isolated block editor somewhere working as a "middleware" to integrate with Gutenberg (but maybe in a more sustainable way, using the components more directly while not relying so much on the private APIs). Also, I do not know much about Gutenberg internals to confirm that using it directly in P2 is, in fact, feasible, to start with. Finally, the fact that we have even more teams adopting Isolated Block Editor in Gutenberg makes me wonder how easy this, in fact, is. :) |
Yes, did you look at the examples in the storybook. I'm not saying that everything is perfect but IMO, isolated block editor shouldn't have to exist, so if there are things that you still think are unclear or too hard to do, please share. |
Nope, I didn't. And to be honest the more I work on this Gutenberg upgrade in P2, the more I agree with this:
Isolated Block Editor has quite some code copied directly from Gutenberg, which makes the upgrade process way tougher than it should be. IMHO, it should be possible to use Gutenberg directly, and that's why I'm kinda interested in learning more about what's possible to do with Gutenberg directly. However, I'm a bit worried about bringing the idea and suddenly finding that it is not feasible right now, and then we end up with Isolated Block Editor 2.0. :) |
@fjorgemota You could start by simply copying the Isolated Block Editor sources into the P2 repository, and then you can start updating them there. There should be many opportunities to simplify the integration code. The "Gutenberg as a Framework" project has shipped many improvements in the integration API, and the Storybook examples, and also the integration in the Verbum project, they all look much less complex than the current IBE. |
Hey @jsnajdr
I'm not sure I'm following what you mean here. What's your idea? Do this to upgrade Gutenberg? Or to replace it with Gutenberg directly? Given the current status of P2, I do not think it is a good idea to copy the IBE files into the P2 repository. Also, right now I was mostly researching how feasible is to use Gutenberg directly instead of IBE. We do not have any plans right now to do that migration. Our current plan is to just upgrade P2 and IBE to run on latest Gutenberg as of now. |
My idea is to do exactly what you describe here: to use Gutenberg directly instead of IBE. If you want to do this, a good way to proceed is to first copy the existing IBE files into P2, and then modify the code there. The modifications would be hopefully mostly simplifying and streamlining the code. |
The isolated block editor uses several Block Editor private APIs and that has become an ever bigger liability since Core started intentionally breaking private API usages on every major release. In this issue I'm writing down what exactly is being used and what we need to do to stabilize all these APIs and make them public.
<BlockCanvas>
: has two versions. One is exported by@wordpress/block-editor
as public API and supports only a limited set of props. The full component is exported only as anExperimentalBlockCanvas
in the private API. The props that would need to be added to the public version are:shouldIframe
is by defaulttrue
, but the isolated editor wants to set this tofalse
. Alternatively this could be solved by making the isolated editor always work in iframed mode, which it currently doesn't support.iframeProps
are props for the iframe element, and one thing we need to pass here are the mobile styles fromuseResizeCanvas
.contentRef
is widely used across the editor to access the scrolling element of the editor. See theuseTypewriter()
andusePopoverScroll()
description below.useTypewriter()
: watches the content scrollable element in order to scroll the caret into view when user starts typing (@ellatrix did I guess the function of the hook correctly?). ThisuseTypewriter
hook should be make public (it's now the old-fashioned private API with an__unstable
prefix) and the various__unstableContentRef
props should be also made public.The scrollable content element (usually passed as
__unstableContentRef
) is also used by theusePopoverScroll()
hook which ensures that we can scroll smoothly even when the mouse is over a popover element. IMO this hook currently doesn't work, as it wasn't updated for the iframed editor.<RecursionProvider>
is used to prevent infinite rendering loops, like when apost-content
template would contain an innerpost-content
block that renders the same post. This API (RecursionProvider
anduseHasRecursion
) could easily be stabilized by removing the__experimental
prefix. Or maybe it could be integrated intoBlockList
. And instead of using the provider and the hook explicitly, make it triggered by someblock.json
attribute, likepreventRecursion: { id: 'ref' }
(pointing to the name of attribute that serves as unique ID).useResizeCanvas()
: provides extra styles for mobile views. Maybe could be integrated intoBlockCanvas
.<LayoutStyle>
,useLayoutClasses()
anduseLayoutStyles()
: used to render theme layout styles for the top-level post content. In isolated block editor this code is identical to whatEditorCanvas
does. So maybe usingEditorCanvas
directly in the isolated editor could also be possible, eliminating these dependencies, and also others likeuseResizeCanvas()
oruseTypewriter()
.<Library>
is a wrapper aroundInserterMenu
, exported as__experimentalLibrary
, is used by everyone who wants to implement an inserter sidebar menu. Let's review its API and stabilize it.<ListView>
is a similar component used to implement block list view in a sidebar. Let's review API and stabilize, too.useDialog()
is a hook in@wordpress/compose
, exported with__experimental
prefix, used to manage focus on modal elements and to close modals on Esc key press. We should either stabilize it, or replace it with something in Ariakit or in the components package. It feels like an a11y thing that these packages work with all the time (cc @ciampo).<Text>
is exported from@wordpress/components
, and used by isolated editor to render list view outline. It's basically adiv
styled with styles fetched from the component system context. Maybe we can replace it with regular<div>
everywhere. I'm not sure if all the extra features are really used.reusableBlocksStore
: the isolated block editor reimplements this store with its own version, and most of this store's actions have the__experimental
prefix. And looking at the codebase, I have some doubts whether this store is still used at all (?)And that's all 🙂 FYI @youknowriad as there is a very big overlap between this and the GaaF project in #53874.
The text was updated successfully, but these errors were encountered: