Add blockEditor settings on the widget screen #15788
Conversation
jorgefilipecosta
added
[Priority] High
jorgefilipecosta
requested review from
aduth,
gziolo and
youknowriad
as code owners
lib/widgets-page.php
Outdated
| @@ -29,9 +29,14 @@ function gutenberg_widgets_init( $hook ) { | |||
| return; | |||
| } | |||
|
|
|||
| $block_editor_settings = apply_filters( 'block_editor_settings', $editor_settings, $post ); | |||
There was a problem hiding this comment.
Usually apply_filters needs a doc block which documents the filter. Not sure why the linter hasn't complained here.
There was a problem hiding this comment.
Do we really need the settings and the filter to be the same between the two pages? I think adding the settings raises a lot of questions (what settings do we want to share and what settings are specific to the post editor). I think this answer should be addressed iteratively and settings added step by step as needed.
| useEffect( () => { | ||
| setupWidgetAreas(); | ||
| }, [] ); | ||
| return <Layout />; | ||
| const blockEditorSettings = useMemo( |
| */ | ||
| const EMPTY_OBJECT = {}; | ||
| const WidgetBlockEditorSettings = createContext( EMPTY_OBJECT ); | ||
| export default WidgetBlockEditorSettings; |
There was a problem hiding this comment.
In other parts of Gutenberg (e.g. DropZoneProvider) we export default the Provider and expose Consumer as a named export. I think it makes sense to be consistent.
So, that is, I suggest we rename this file to widget-block-editor-settings-provider/index.js and go:
const { Provider, Consumer } = createContext( EMPTY_OBJECT );
export default Provider;
export { Consumer };| /** | ||
| * Reference to an empty object | ||
| */ | ||
| const EMPTY_OBJECT = {}; |
lib/widgets-page.php
Outdated
| @@ -29,9 +29,14 @@ function gutenberg_widgets_init( $hook ) { | |||
| return; | |||
| } | |||
|
|
|||
| $block_editor_settings = apply_filters( 'block_editor_settings', $editor_settings, $post ); | |||
There was a problem hiding this comment.
$editor_settings and $post are undefined.
Notice: Undefined variable: editor_settings in /var/www/html/wp-content/plugins/gutenberg/lib/widgets-page.php on line 32
Notice: Undefined variable: post in /var/www/html/wp-content/plugins/gutenberg/lib/widgets-page.php on line 32
| > | ||
| <BlockList /> | ||
| </BlockEditorProvider> | ||
| <WidgetBlockEditorSettings.Consumer> |
There was a problem hiding this comment.
This could be useContext
| /** | ||
| * WordPress dependencies | ||
| */ | ||
| import { createContext } from '@wordpress/element'; |
There was a problem hiding this comment.
Using a context for the widgets settings does make sense if we forsee us having a complex nested component tree for the widgets page. Is this the case or will be just passing the settings down to the block editors?
Also, I want to remind that we did start with "Context" in the intial versions of the editor settings as well and we ultimately changed to use the data module because we needed to make updates to the settings from the component and we didn't want to reinvent the wheel as this just meant we need global state that can be updated (data store).
As always, I like to start with the simplest possible solution. Isn't a "settings" prop a solution?
|
This PR was updated we now pass the settings down in the components instead of using components, and we only pass a small subset of settings (that we will need for sure) from the server until we decide what should be passed or not. |
jorgefilipecosta
force-pushed
the
add/settings-to-the-block-editor-on-the-widget-screen
branch
2 times, most recently
from
5a863f7
to
4ae9c2a
Compare
…ver. (+1 squashed commit) Squashed commits: [46e57de] Add blockEditor settings on the widget screen; Required to make legacy widgets work.
jorgefilipecosta
force-pushed
the
add/settings-to-the-block-editor-on-the-widget-screen
branch
from
4ae9c2a
to
bb20459
Compare
Hi @noisysocks, I'm not being able to reproduce the problem. When I add the legacy widgets block I see some widgets available, (they can still not be used because of other bugs). Do you have any widget installed? (core widgets are hidden because we have equivalent blocks) I'm doing some tests with https://wordpress.org/plugins/meks-smart-author-widget/. |
lib/widgets-page.php
Outdated
| } | ||
|
|
||
| $color_palette = current( (array) get_theme_support( 'editor-color-palette' ) ); | ||
| $font_sizes = current( (array) get_theme_support( 'editor-font-sizes' ) ); |
There was a problem hiding this comment.
What is the current() here for? It's a fairly low-level array function so I'm surprised to see it. We probably just want to get the first item in the array.
list( $color_palette, ) = (array) get_theme_support( 'editor-color-palette' )
jorgefilipecosta
deleted the
add/settings-to-the-block-editor-on-the-widget-screen
branch
The block editor settings are required to ensure legacy widgets work as expected on the widgets block editor screen.
This PR is related to #15521 given that in both PR's we add block editor settings support to the widget screen.
@youknowriad referred in #15521 that saving the block editor settings in the widgets store too. This PR uses context as an alternative approach.
Description
I added a new legacy widget block to the widget block editor page.
I verified that the legacy widget loads the placeholder normally, instead of a message saying the user has no permissions to manage widgets.