-
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
Experimental: try new "Widget Group" block to act as container for Widgets #33881
Conversation
This comment has been minimized.
This comment has been minimized.
Excellent start. This block, if we want it to solve #32723 be saved in the expected markup where the title is the immediate child of the widget's wrapper and has the standard "title" class applied. I think this can be done through the block's save function, no need for server rendering. |
@draganescu I tested this already on the front end and it's doing what you describe. I used Twenty Fourteen. Update: ah thought I see there is still a nested Can anyone advise on exactly what are the standard classes that should be added and where? |
@draganescu I've updated this PR to lighten the DOM and better match the default markup provided by legacy Widgets. I tested with twentyfifteen and once saved the Legacy Widget version (created by using the Classic Widgets Plugin) and the Block Based version (using the new "Widget Box" block) look near identical. The only thing I cannot control is the markup of the |
Right now, I'm thinking the block seems like a better option than a pattern. At least a block is something that can continue to be improved whereas a pattern, once inserted, is kind of set in stone. Some thoughts:
|
@talldan Some good points - thanks! I've converted them into a Todo list:
The two items that are not completed are more complex.
I don't fully understand this. I assumed we would want a sematic heading. Indeed this seems to be what most Themes expect and what is generated by legacy Widgets by default. What value would this have and also would it continue to allow a 1-click-to-mirror experience for those wishing to retain the legacy block HTML markup pattern?
Yes this is a wider problem. I believe we added "click through" to reusable blocks and template parts. Should/could we explore that here? |
@ddryo pinging you here as you raised the original Issue. Would you be open to testing this out and letting me know whether is goes someway towards solving the problem you noted? |
} | ||
}, | ||
"supports": { | ||
"html": false, |
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.
Should we allow this? I guess we ought to. There's no harm right?
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.
Well I would not allow HTML of blocks to be changed in any block in the entire widgets editor.
}, | ||
{ | ||
template: TEMPLATE, | ||
renderAppender: InnerBlocks.ButtonBlockAppender, |
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.
If necessary we could create our own appender (probably in a follow up) which could be more specific about the fact that you should insert a Widget.
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.
You can insert any supported block not just a widget.
className: 'wp-widget-box__inner-blocks', | ||
}, | ||
{ | ||
template: TEMPLATE, |
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.
Instead of a template we could introduce a "placeholder" for the block. When first inserted it would have a title field into which you would type the title of your Widget (e.g. Latest Posts
). Then when you click submit it would auto create the Widget Box with a core/heading
block already inserted and populated.
Just a thought...
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.
Placeholders are for when we cannot represent the empty state with blocks in a useful and intuitive way. E.g. we don't want an "upload image" block, we want an image block placeholder that allows us to upload. But for this block the template will allow direct editing so we can represent the empty state just fine. We can add some placeholder text to the headline maybe?
It should be pretty much the same, the RichText would render a |
I made some changesI pushed up 6db4b04 which:
Thoughts on the pattern approachI also experimented with the pattern approach over here and discovered that it only lets us solve this problem:
And not this one:
So, I think the pattern approach is off the table. Leaky abstractionOne thing I dislike about this approach is that our block editor abstraction is leaky. We're trying to add widget titles to the block layer when widget titles have always existed in the widget layer. To illustrate, here's what a search widget in a footer looked like in WordPress versions prior to 5.8:
Here's what a search block in a footer can look like in 5.8:
So how do we add a title to the search block? Here's what we are proposing here with Widget Box:
But it would be more natural, I think, for the title to exist in the widget as it always has:
Perhaps Mixed paradigmsAnother thing that bugs me is that some widget-specific markup ( Similarly, some things (the blocks inside a Widget Box) appear exactly as it does on the frontend because of editor styles but other things (the title of the Widget box) do not. (This is I think what @talldan was getting at in #33881 (comment).) I think that the dichotomy here is that there are two ways to think about the widgets editor:
Perhaps we should allow the theme to pass |
True, though a widget group could also just be like any other block if the But I think the main thing is that it's an optional feature, and a lot of themes won't really need this block, it's really just for themes that have a distinct style for widgets (gives them an outline or something) or has widget areas that behave in a particular way (like the columns layout in Twenty Twenty One). Ideally the theme will be leaning towards a more freeform style. |
I suppose 😅 At any rate, right now the block is in
Do you think the default widgets created in a new WordPress installation should continue to use regular groups? |
I think the next step is to explore the idea raised in #33881 (comment). To re-iterate that conversation, if we make Widget Group look more akin to Legacy Widget then that should hopefully:
|
Hey all, I'm also on board with @talldan's suggestion to leverage the legacy widget UI. Just thinking out loud but I'm curious if there’s any way for the widget group block to be combined with the existing legacy widget block, or whether it needs to be a new separate block? I would have to think through the UX some more but it might be something like a one-time set up placeholder that would either result in the usual legacy widget format (switches between "preview" and “editing" mode) or the something like the block/widget hybrid tried in this PR, depending on what you insert. Let me know if this seems worth exploring — seems like it might be helpful to consolidate the "old way" into one legacy block and would help with some of the goals/considerations @noisysocks listed above. |
Hmm. I'm not really sure. It might be worth running it by others on the design team. I lean towards a seperate block, because:
|
Hi all, sharing a first pass at the editing flow for the Widget Group block. I tried making the container block act like a Legacy Widget as much as possible to leverage our two existing editing flows: widget-group.mov
There are some weird things about this but I think that's going to be the case with any of the directions we take here. Thoughts? |
Thanks @critterverse! I think that mostly makes sense, and you're right about anything we do here being a bit awkward 😅
What do you think about ditching the "Add a block" label? We're very consistent about ➕ representing "add a block" so I think this label might be unnecessary.
This isn't possible unfortunately. We can display the title in a manner of our choosing but we can't make it appear exactly as it will on the frontend because we cannot use the theme's |
This makes sense! So more like this:
Sorry, WYSIWYG was bad wording on my part — I just mean that in its resting state, the Widget Group can display like a Legacy Widget in "preview mode" (no worries about the title not displaying exactly as it will on the front end). |
It's great to have a clearer direction here ✨ My initial thoughts are that it might not be obvious that you have to click the parent One thing I was thinking was that the core team are working on the ability for a child to consume the parents toolbar controls so perhaps we could even make the title part of the Widget Group's block toolbar and then allow the child blocks to display that title control if they are children of the Widget Group. However, I may be overcomplicating things... Looking forward to seeing this evolve. |
This does seem a bit complicated, both from the perspective of treating the title like a toolbar action and tying the legacy UI to that (if I’m imagining this correctly). Interesting PR though, glad to have it on my radar!
Here's an alt version of the above flow that might help with this: widget-group-2.movIn this version the title input is removed from the placeholder set up and when a block is inserted, the editing interface for the Widget Group is displayed rather than first editing the inner block. This is slightly closer to the normal Legacy Widget set up flow and could help make the title more discoverable. Note that I changed the legacy editing UI to match the Legacy Widget block more closely, with the inner block type displayed at the top rather than "Widget Group." I think this makes the most sense? Similar to Legacy Widgets, we could use the block type as the default title to display if the title input field is left blank. This would work nicely for most widget blocks like "Archives" but would be kinda weird if you insert a Cover block or something 🤔 |
How would this work with inserting multiple blocks into the Widget Group, though? Or would you have to insert a Group block first? |
I had an idea! Please tell me if it's terrible. We already have a design metaphor for this problem of "how do you present visual content (that looks as it does on the frontend) within an abstract container (that does not necessarily look as it does on the frontend)": the widget area itself. Perhaps Widget Group could borrow this? Here's my attempt at butchering @critterverse's lovely Figma file. What I don't like:
What I do like:
|
Interesting exploration @noisysocks, but I agree about this implementation being a bit inception-y and I'm not sure I completely follow the design metaphor — for example in this scenario I would expect "Footer" (or whatever widget area title) to be displayed on the front end in the same way as "Archives." I prefer the legacy UI approach because:
I can keep looking into what might happen if users add or change inner blocks if we think we're on the right track with this direction. |
Yeah fair enough 🙂
I think I prefer the first prototype (#33881 (comment)) over the second (#33881 (comment)) because:
As a "thought experiment", here's how a Text widget behaves when it's in a Legacy Widget block. This scenario is slightly analogous to a Widget Group because Text widgets support adding multiple "blocks" (except we don't call them that) of content. Kapture.2021-08-30.at.11.53.59.mp4 |
Thanks for these videos @noisysocks, both are helpful for thinking about this :)
Using the above as a starting point, I'm still thinking that autofilling the title input based on the block that's initially inserted could work. The title can be manually updated if the block type isn't a particularly public-facing title or if you start changing/adding blocks (this doesn't seem too unexpected, imo). I like the idea of leaving the title input empty and not showing the title by default but one concern is that it might make the title harder to discover. For example, this note would no longer apply if the default behavior is to only show the inner block:
|
Yeah really good point. I'll start to implement one of the two flows (in a new PR) so that we can see what works in practice. |
I opened #34484 which takes this PR and implements the flow in #33881 (comment). Let's continue the discussion there. Thanks @getdave for working on this and getting the ball rolling 🙂 |
Description
Context #32723.
This approach tries adding a new block called "Widget Box". It's basically just a container to add:
This mimics the original widget markup format.
It also comes with some additional helpful touches:
Note that this could complement #33878 where we provide a block pattern. We can just use atransform
which is what I've done now.Closes #32723
To to
before_title
,after_title
for the rendered title markupHow has this been tested?
To compare against the original/legacy widget markup pattern:
Screenshots
Tested using TwentyFifteen. The aim is visual parity when trying to create an "Archives" Widget using the "Widget Box" block
Screen.Capture.on.2021-08-05.at.12-57-27.mp4
Types of changes
Checklist:
*.native.js
files for terms that need renaming or removal).