-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add an empty list block v2 #39459
Add an empty list block v2 #39459
Conversation
Size Change: +486 B (0%) Total Size: 1.16 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me. Thank you :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice idea to lay out the initial scaffolding for the work @youknowriad! I think the approach looks good to me, but I'll defer to @glendaviesnz if he has a chance to take a look since he's probably got a better idea of the pain points in the gallery block work.
@@ -41,3 +42,7 @@ export const settings = { | |||
save, | |||
deprecated, | |||
}; | |||
|
|||
export const settings = window.__experimentalEnableListBlockV2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, this is an interesting way to do it! In the gallery block, the checks / swapping out logic happens within the edit
and save
functions. E.g. in edit-wrapper.js and within saveWithInnerBlocks, so it happens a little later in the process rather than when the list's index.js
file is evaluated.
Since you're doing it earlier here, is that why the __experimentalEnableListBlockV2
flag is set on window
instead of window.wp
?
I think it probably makes it easier for your exploration to swap out which settings object you're using as you've done here (also means less changes to the existing list block) so I quite like the approach personally, though it might be better to nest the setting within window.wp
for consistency sake?
I'll just CC: @glendaviesnz since he dealt with more of the pain points in the gallery v2 settings. There's also the option of exposing the setting in Gutenberg settings as with __unstableGallleryWithImagesBlocks, but that approach would likely preclude swapping out settings objects and necessitate the switch happening within the edit and save functions, so I think I'd lean toward the approach in this PR, which feels a bit simpler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeh, this seems like a simpler approach than the one we took with the Gallery block refactor.
The other main pain point in the Gallery refactor was difficulties with the mobile apps using/relying on flags to dictate which version to use - flipping the flags was easy in the web client, ie. we could go from defaulting from v1 to v2 with a simple toggle of the flag, but in mobile, the flag checks ended up compiled into app versions so you need to account for the expectations of the older versions of the app in the wild. It was critical to have the flags in the mobile app though as the old version of the editor compiled in lost data when they were confronted with a new version of the block with innerBlocks - but it took a lot of coordination with the mobile team to work out how to transition mobile to the new version.
This is most likely stuff you are already aware of as has also been discussed on Quote block PR. It looks like @dmsnell is making some progress on the data loss from unknown blocks issue.
Related #6394
What?
This is a PR that adds a v2 version to the list block that you can trigger using an experimental flag on the "experiments" page of the Gutenberg Plugin. For now, that list block does nothing, it's just a static block rendering a div. The idea is to use this as a placeholder to iterate on the list block described in #6394, the main point being that it should use inner blocks for list items.
Why?
Building a list block with inner blocks is going to be a complex task that will take several PRs and iterations until success (or not). So we need a safe place to iterate on the block, get some feedback, make breaking changes... without breaking production websites.
How?
I think a similar approach with a unique block that changes behavior depending on an experimental flag has been used before for the Gallery block refactor. Would love some eyes and thoughts from folks who worked on that to assess the pros/cons here. cc @andrewserong @aaronrobertshaw @glendaviesnz
Notes
I hesitated about whether to push the PR further and start adding some real logic (inner blocks) but decided that there's a merit to discuss and land the "placeholder empty block" on its own first.
Testing Instructions
1- Open the experiments page under "Gutenberg" menu item
2- Enable the list block v2 flag
3- Go back to the editor
4- Check that inserting a list block gives just a static div for now.