-
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
Experiment: Allow creating template parts from block library #42142
Conversation
Size Change: +1.41 kB (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
Thanks for the PR @talldan, it's good to see this in action to get a better feel for the overall flow(s). Based on some initial exploration I think it can work well. I imagine the new 'Template Parts' section in the Inserter is to aid testing, but just to clarify – these can probably live in the 'Theme' section for now. It tripped me up a bit to see existing template parts as options in the modal, that seems a bit redundant. It might be better to only surface patterns here. On a similar note, it could be confusing to see patterns that have already been used to create template parts, e.g. the 'Large header with dark background' in Twenty Twenty-Two. If it's possible to exclude such patterns that would be 👌 New Header and New Footer UX feels good, but I'm not sure how useful New Template Part is going to be. The scope of a general template part is very broad, so the likelihood of patterns we surface being relevant is quite low. Without some kind of filtering options on the modal, it feels too clunky. For now I would be inclined to limit this to the creation of headers and footers only.
I think it's probably ok to add this without polish for now, as the modal refinement could be quite a big project. There are some designs in #39308 (comment) that explore this in detail.
Could we use the pattern name as a 'default' title?
Agreed. Existing template parts should be listed by name instead (#31748, #41399). |
Thanks for the feedback @jameskoster - I'll get to work on making those changes. |
That'd be good. Also we should take into account what template you are creating it in — if I'm creating a new header on a category archive template, and I already have a header template part, the name should probably be something like "Archive {Name of Pattern}". |
Throughout development so far I've been focusing on launching a modal directly from the inserter menu item. An alternative simpler technical solution is to insert a template part block as normal when clicking the item in the inserter, but have the block open the pattern selection modal straight away when certain conditions are met (an initial attribute is set?). I think it would be possible to do this without any block API changes, but there's a drawback that it'll be more difficult to get the accessibility story right ( |
f755494
to
e2ecaae
Compare
@jameskoster I've pushed an iteration on this that includes most of the changes discussed. Template part naming doesn't quite do what @mtias mentioned yet, still looking into that one, but it will use the pattern name. Let me know what you think. I'm still trying a few things when it comes to the technical implementation, so this PR is not ready for code review yet. |
Thanks @talldan, I appreciate the work in this update. Here's what I'm seeing: header.mp4It feels pretty slick to use, but there are a couple of drawbacks:
These are relatively easy to fix, but we should think holistically about which approach will be best. On the one hand we have a single Header block – as in this PR – which handles both the insertion of existing template parts and the creation of new ones via patterns. On the other hand we have the option of listing existing template parts in the Inserter (#31748), then having a dedicated "New template part" block which opens the pattern modal. Considering that we also want to list existing template parts in the Patterns tab (#41399), I'm inclined to say that the latter could be the way to go. But, there's also a world where we implement both, so that we have:
I would love to hear thoughts from @WordPress/gutenberg-design on this one. |
If we add 'New header' and 'New Footer' blocks, here are some icons based on the recently added 'Add template' icon: New header
New footer
|
Thanks for the further feedback @jameskoster. I'll have a think about the best way to achieve that and let you know when I have something more to test. |
Just connecting a dot that @critterverse pointed out to me... the work here could go a long way towards closing #41397 as well. If it's not suitable to include in this PR, perhaps we can look into it as a follow up. |
I think it makes sense to break this up into two or three PRs along those lines. For the last one, it looks like there are a lot of similarities to the way we show each instance of a reusable block in the inserter rather than just one 'reusable block'. At the moment, reusable block doesn't use block variations to achieve this, there's a very ad-hoc piece of code that shouldn't really be there: gutenberg/packages/block-editor/src/store/selectors.js Lines 1937 to 1982 in f01f981
I think there's a good opportunity to remove this code in favor of using the block api. |
Yeah that seems like a good idea. I suppose this PR can facilitate the first point in the list. To that end, the missing pieces are:
There are a lot of similarities in general, not to mention patterns as well 😁 The more we can align the interactions and obfuscate the underlying complexity, the better the experience will be 💪 |
f8380b7
to
90101fc
Compare
This should be getting closer to what's described in the last comment.
We don't have an internationalized plural form of the area label, but I can look at adding this.
This is still on my to do list. Let me know if there's anything else I missed. |
No worries, let's do it in a follow up. I'll make an issue. Aside from the missing 'Start blank' option this is working really well, except for one quirk: If you choose a pattern in the modal, a template part is created with the name of that pattern. If you repeat the process another template part is created with the same name, which can lead to confusion: Two possible resolutions:
Each have some trade-offs. Option 1 could result in a proliferation of template parts if the user isn't diligent. The second option could lead to legitimate patterns being hidden. What do you think? Edit: I noticed it's possible to give a template part a duplicate name even when creating manually, so I opened #42473. Perhaps it's better to address this separately? |
I just noticed something else, unsure if it's only on this PR or an existing problem; if you drag 'Header' from the Inserter on to the canvas nothing happens. |
Let's see if I understand this correctly based on my limited understanding. Brainstorming coming up based on the initial post by Dan. Voicing my confusion. There will be icons called New Header, New Footer etc directly in the Block inserter screen. First off all I might start the Block Inserter with the Theme section instead of the Text section when in Full Site Editing, as it would show possible blocks/template parts one could add to a template before going into more detailed blocks. |
@paaljoachim please see the three bullet points at the end of this comment for an overview of what we're considering. This PR originally concentrated on the second point, but kind of morphed into concentrating on the first one :) |
@paaljoachim I think the video on the PR description was a little outdated when you read it. It's updated now. |
I've put together an alternative PR - #42581. The difference in that PR is that when selecting a Header or Footer from the inserter, the block is inserted first and then the inserted block immediately displays the selection modal. There are some differences in the user flow - if the user closes the modal, the block is still present in a placeholder state. I thought it'd be interesting to try because it's only a few lines of code to achieve almost the same thing and doesn't require any new APIs. It still has a few issues to iron out though, and it isn't quite as smooth as this PR. |
Very interesting problem! Just an idea, but what if, instead of I'm thinking that should let you set Regarding drag-and-drop, could we treat this flow as a progressive enhancement to the default flow and not bother having it for block insertion flows that don't involve selecting an item in the inserter? So that's drag-and-drop, |
Good to try! Having both behaviours (open modal and insert block) execute concurrently feels a little bit clunky to me. I fear it would get clunkier when we get around to implementing the animation @noisysocks linked to.
Yeah that could work :) |
90101fc
to
dda4fb5
Compare
I've refactored this so it works closer to how @noisysocks described This API is a bit too open right now as it allows plugins to render anything in the inserter for their block. It'd be good to start with something more restrictive, so I'm working on that next. |
Nice! I think we're just missing a solution to the duplicate name issue, and the start blank affordance. |
dda4fb5
to
5cfa57b
Compare
I've added the 'Start blank' option. I think the modal also requires a loading spinner, as most of the actions involve an API request and aren't immediate. Any thoughts on how it should look, @jameskoster? |
Oh good idea. We can probably just use the Spinner component. |
Looking good in my testing. Nice that the focus isn't lost when pressing ESC too.
Yeah, makes sense. We have to be especially careful here as some plugins could abuse this to promote their blocks. (Though admittedly, bad actors will always find a way...) |
Here's where we are: create.tp.mp4The spinner appearing above the content feels a bit awkward, could we place a white scrim over them, or just hide them? Otherwise we're looking good :) One thing to think about – potentially as a follow-up – what can we do to mitigate the proliferation of template parts? If you insert a header and choose a pattern, the template part is created immediately. If you undo, it persists in the system. If you delete the block before saving, it persists. If you repeat the process (create template part from pattern) you essentially end up with duplicate template parts. It's easy to see many unused / duplicate template parts building up over time. |
@jameskoster There is an overlay, but maybe it's too subtle. I've pushed a commit to make it more obvious. |
Some random thoughts—it may be interesting to treat patterns a bit like the theme provided template parts. When a user creates a template part from a pattern, the pattern name is recorded in the template part data, and after making customisations the user has the option of clearing those customisations back to the original template. If the user tries creating a second template part from a pattern, a dialog can be shown offering the option of using the existing template part (and the UI can show whether this existing template part has had customisations). There may be some challenges though - a pattern can be provided from a number of sources (plugins, themes, pattern directory), and can change over time or even be deleted, so I don't know whether 'clearing customisations' would always reliably give the user what they want. |
Much better! :) I think this PR is ready for a wider audience / review now. I suppose the proliferation issue will be resolved if/when we combine patterns with template parts (see discussion in #34352). Still, I think there are some behind-the-scenes adjustments we can implement to help such as:
|
Show workflows in inserter Add selector for getting inserter workflow items Stub out workflow items in inserter Avoid computing draggable blocks for non-draggable items Spread items to flatten them rather than creating a nested array Fix error thrown for non-draggable inserter block Show workflow component when selecting workflow Fix order of items returned from hook Pass root client id to workflows Remove unused property Make header and footer the active workflows Use proper area label Insert the template part block Use same classname as other modal Show existing template parts in modal Add non-area template part workflow Avoid showing empty sections Change inserter panel title Remove existing template parts from modal Iteration 2: Try an `insert` block API Add `insert` property to inserter items selector result Show `insert` component for inserter items that have one Refactor callbacks Use pattern name for created template part Iteration 3: Launch modal from inserted block Reorganise components and utils Revert "Iteration 3: Launch modal from inserted block" This reverts commit 3d04accc51fd04e169fd4567f76cb4a5b85bfa92. Refactor template part selection modal to be more generic More refactoring Fix close button Fix non-pluralized text Fix naming nitpick Refactor to use `inserterItem` API
c3a3326
to
604e010
Compare
What?
Exploration for #31746
Allows selection of a pattern or existing template part from the inserter prior to insertion of Header / Footer (template part) blocks.
How?
I'm currently testing a few ideas around how to implement this.
Currently this adds an
inserterItem
property for for blocks and block variations that can be defined as a React component. It replaces the normal item for the block in the inserter.The block editor package also exports a
InserterListItemWithModal
(name could be revised) component that makes it easier to implement an inserter item that launches a modal and has the right aria roles.For header and footer template part variations, the pattern selection modal is shown and the inserted template part will be pre-configured with that pattern.
Problems with this approach:
InserterListItemWithModal
.Testing Instructions
Screenshots or screencast
Kapture.2022-07-21.at.13.03.41.mp4