-
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
[Block Library - Query Loop]: Add block settings menu item to reset(remove the Innerblocks) #37667
Conversation
Size Change: +135 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
Thanks for the prototype and descriptive video! At a quick glance, this seems like it might work as a way to address 30925. The fact that it clears its inner content doesn't worry me personally, you can always undo, and you have to confirm your change when multi-entity saving. I wonder, though, if we should change the menu item to say something like "Start over" or "Reset query", and then open a modal |
Thanks for the feedback Joen!
I thought about the modal too, but this and the |
Right, so if you insert a pattern where the query block is one or multiple child blocks, you could easily get a loop within a loop situation. #36697 suggests treating the inserter differently when it's at the root level. I wonder if some types of pattern transformations should be considered differently when at the top level? I.e. I want to swap out this entire section of content being something you can only do at the root level. Tangentially, this animatic also has an exploded view which might be an interface we can use to surface section-level transformations. That's all a bit more forward looking, but that could potentially be a higher level way to swap out patterns. What do you think? |
I wonder if it would help to make the pattern element more explicit in this flow. IE instead of start over, it's view patterns. There were some low-level explorations for that in #28736. An exploded view like Joen mentioned could work too. Or we might even use the new pattern explorer modal... this would be nice as it offers an easy exit without having to undo, and is aligned with our desire to try using a modal for pattern insertion.
I think this one is a separate issue. While I understand the issues that can arise in these flows, enabling a reversion via the wrapper feels like it goes beyond the intended purpose for patterns. |
f7ad0ca
to
0c95202
Compare
In general, while this PR seems like we are transforming patterns, we are not. It's just happens that Could an approach like this for the specific block move forward with probably a |
I would be interested to see something like this: patterns.mp4Having access to patterns off the transform menu might scale better across other block types versus the "Reset Query" approach. FWIW I appreciate this isn't a true transform under the hood, but to the average user I think it does feel like one.
I'm conflicted on this one. I definitely see the argument for making this restriction. But there's a trade-off in the UX... if I become accustomed to accessing the pattern explorer via the block toolbar, it may not be clear why I can access it for some blocks, and not others. Especially since the exact same block will behave differently depending on whether it is nested or not. On balance, I think it might be best to proceed without the restriction. The nesting seems mostly to be an issue when working in something like a columns block, and the pattern may not appear as advertised due to space limitations. But I think that is kind of to be expected if you're working in a tight space anyway... |
0c95202
to
880b9b3
Compare
I think this issue is specific to So, I removed the top level block check and we just need to decide if we can move forward with this solution for now(besides copy adjustments, etc..). I personally believe that the issue has been raised as a way to |
This feels like a technical detail that most users will not appreciate. I do agree that in the narrow case of the Query block you can argue this action is a "reset", but that framing feels a bit awkward for other blocks, especially those that do not have a dedicated setup step. For example "resetting" something like a Blockquote is less natural than viewing Blockquote patterns. That is (imo) the broader issue here – as patterns permeate more flows in the Editor, we should make accessing them consistent and predictable regardless of what block is selected. There's an opportunity to lay the foundational solutions here. All that said, there may be a case to provide both:
We could concentrate on 2 for now, and work out the details of 1 later. |
If I got this correct then this PR by @jorgefilipecosta is associated to your PR, Nik. |
Related: #30925
When we insert a
Query Loop
block a user can select from a variety of available patterns or its block variations. The need for being able to go back to this selection state after having selected something, has been expressed quite some times.Currently the way to do this is to remove the existing block(s) and start fresh. This PR adds an extra menu item in block settings toolbar in
Query Loop
, so you can do it with an easier way.Here is the catch though.. Query Patterns are just block patterns which means they can contain any number of blocks and a common thing seems to be to wrap a
Query Loop
block with aGroup
block or even have multipleQuery Loop
blocks.When we insert any pattern in content they become blocks and there is no way currently to identify that they were part of a pattern. So this menu item here, might be confusing depending on the selected pattern...
In the below video I showcase the menu item for patterns without a wrapper and at the end with a
Group
wrapper to illustrate my above point.Screen.Recording.2021-12-30.at.5.50.00.PM.mov
I don't think this is an approach we will follow, but made this quick prototype to gather some feedback.
What do you think?