Experimental: try adding Widget + Title block pattern to simplify creation of previous Widget markup patterns #33878
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
getdave
added
[Feature] Widgets Screen
getdave
requested review from
adamziel,
draganescu,
kevin940726,
noisysocks,
talldan and
tellthemachines
as code owners
|
Size Change: +179 B (0%) Total Size: 1.03 MB
ℹ️ View Unchanged
|
11 tasks
|
Nice, this is an awesome first step towards introducing patterns! I'm wondering if for this experiment, there's a way to make it even more clear where the widget needs to be inserted — it's easy to use the bottom appender and accidentally insert a block outside of the Group, for example. One simple solution might be to include an empty Paragraph block underneath the Heading so it would display the default placeholder text there, "Type / to choose a block" |
Or we could create a custom appender and use that on the |
|
The biggest problem I have here is this:
This does not appear to solve all the problem immediately but only half of it:
|
Good question. Our current advice to users wishing to better emulate the legacy markup pattern for widgets is to use the Group Block and a heading and apply the appropriate classes. This PR seeks to streamline that process somewhat, although personally I feel it's still asking a lot of the user. I'd treat this PR as one possible option which may solve some of the requirements but not necessarily all. Indeed the major problem is that the Group block unavoidably outputs a lot more HTML that we'd like. As this does not mirror what older Themes expect from Widgets the styling is broken. See below - as you can see there are too many "containers" and nested elements:
We may deem the trade offs to be acceptable in order to avoid introducing another block (e.g. "Widget Box"), but I doubt it. I would probably prefer the approach of adding the new Widget Box block, as that provides greater control and solves more of the problems. |
getdave
force-pushed
the
try/widget-patterns-to-streamline-old-markup-patterns
branch
from
af3a592 to
f4f5d33
Compare
|
Following on from discussion over at #33881 (comment) I've had a go at trying to improve the markup of the Group block to avoid the nesting of divs to better match the markup pattern. I've discovered two things: In Editor Group inner container is conditionalCurrently the Group block only renders the inner container div if the Theme doesn't support "layout" themeSupportsLayout: getSettings()?.supportsLayout,However that doesn't effect the front end. Only the editor. We force the inner container on Group blocks for themes without Theme JSONWhat I've discovered is that for themes without a gutenberg/lib/block-supports/layout.php Lines 147 to 172 in fce770a This is part of Core (or if not registered then part of Gutenberg Plugin) so it's difficult to undo. This is likely related to the "layout" feature. As a temporary hack you can add the following which removes the filter and allows you to have no inner container on the Group block. add_action( 'widgets_init', 'gutenberg_remove_group_block_inner_continer_for_widget_patterns' );
function gutenberg_remove_group_block_inner_continer_for_widget_patterns() {
remove_filter( 'render_block', 'gutenberg_restore_group_inner_container', 10, 2 );
remove_filter( 'render_block', 'wp_restore_group_inner_container', 10, 2 );
}This gives us the following markup
However this will remove the inner container for all usages of Group including outside their use within Widgets. That won't cut the mustard so we'd need a way of conditionally removing it only when it's used as part of a Widget pattern. I'm not sure that's going to be robust or even possible. |
|
Just noting here that I tried to incorporate global $wp_registered_sidebars;
foreach ( $wp_registered_sidebars as $sidebar ) {
register_block_pattern(
"core/{$sidebar['id']}-group",
array(
// Translators: %s: Sidebar title.
'title' => sprintf( _x( '%s Widget Group', 'Block pattern title' ), $sidebar['title'] ),
'categories' => array( 'widgets' ),
'content' => '<!-- wp:group -->...<!-- /wp:group -->',
)
);
}This doesn't work though because So, a failed experiment 🙂 |
|
Ok @noisysocks - thank you so much for exploring this 👏
This seems like another reason this pattern-based approach will not work.
I agree. So for clarity the 2 reasons for abandoning this direction are:
Despite patterns being a good approach in an ideal world, it's clear that in this very specific, niche case they do not solve our problem. As a result I think it's now safe to close this PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Please read the description of #33874.
This PR implements the "Widget + Title" pattern proposed in that Issue.
Currently this is an experiment as if we pursue this approach we'll also need to consider:
Closes #33874.
How has this been tested?
Screenshots
Screen.Capture.on.2021-08-04.at.13-39-42.mp4
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).