-
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
Template Part Block: Add a block label #24557
Conversation
Size Change: +720 B (0%) Total Size: 1.16 MB
ℹ️ View Unchanged
|
501ed86
to
2c93ec4
Compare
2c93ec4
to
af2ef8c
Compare
@@ -0,0 +1,27 @@ | |||
/** |
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.
Note:
We can definitely talk about refactoring the labels component for reusability if we move forward with template part labels
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.
I am liking this, its working well! 🎉
Some thoughts on other additions (probably future PRs if design likes this enough as is for now):
- A hover state that triggers a pop-up/tooltip next to the label that can have text describing how it behaves differently from other blocks.
- Refactoring this out of template part so we can use it for other blocks with their own custom descriptions.
const [ title ] = useEntityProp( | ||
'postType', | ||
'wp_template_part', | ||
'title', | ||
postId | ||
); |
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.
I don't have a strong opinion on this, but figured I would point it out. It seems we are also using this useEntityProp
for title in the name-panel component. Maybe it would make sense to move it into index and pass it down to these two child components? But maybe it doesn't really matter and is more simple this way.
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.
I don't have a strong opinion either. If this becomes widely shared, I'd err on the side of removing implicit dependencies and moving useEntityProp
out of the label component. The consuming component can always pass the value through props, and the label component will only be responsible for rendering text.
This is looking nice! Awesome job. Let's tweak the label slightly to make it same as the mocks (https://d.pr/i/DX7mKm).
We're now left with an interesting issue (which is why I didn't wanna go to the label immediately): The label we added is another label similar to the selected block label. So we effectively have two labels for the same thing. |
@dubielzyk - Im curious about this, why are we going with 10px? He had it at 10px previously and it seemed incredibly small and inconsistent with the rest of the editor. I had recommended the change to
|
The block toolbar of the first child appears on top of the new label. -- Similarly, in Select mode the new label competes with the existing button: -- The new label seems to be picking up some theme styles. The screenshot above is with Twenty Nineteen active. Also, the icon here doesn't really correlate to the Template Part block icon (the LEGO): |
I wonder if there is an existing style we can use; 10px seems a little small, and I'm not sure we use letter-spacing elsewhere.
Where does this number come from? I struggle myself with the general "grid" in Gutenberg and 22px isn't a value I've come across. |
I just tested this out locally. Overall, the concept works for me. I don't think it's ready just yet as I think some iteration is needed around where the label appears in a variety of situations:
Is the icon supposed to be a reusable block icon for template parts? |
8562c6b
to
00ec050
Compare
I see. Which brings us back to the question of why we have two modes in the first place. @dubielzyk 😛
Great point! I'm removing it entirely in select mode (for the time being), but this is definitely an ongoing discussion. It seems like the selection block labels are genericized with "Group", "Link", "Site Title", so it doesn't make sense to me to display custom names at all.
The icon is a placeholder. @shaunandrews @MichaelArestad My mistake for not clarifying in the description of the PR, but these code changes aren't meant to be the finalized version of what we implement. We're in the process of deciding whether or not these labels are best for improving the template part editing user experience, and a lot of details are up for discussion. More context can be found in this issue #22064 |
Should we only show the label of the "current active" template part, not the grandparent template part? This makes sense to me, because when you edit the child template part, you are only saving changes to the child. You don't actually modify the grandparent template part from a technical POV.
I think we've made some changes to this in #23847, where the label in select mode uses the dynamic label of the block (which is something like
One of the ideas with the icon here is to communicate "reusability" and that the template part is a bit unique in that regard. IMO, it might be possible to find an icon which could to indicate reusability across a wide variety of blocks. Example: how does the user know that their site title block will actually have a very wide affect across the entirety of their site? Perhaps an icon could help indicate that the site title block is special in that way. Really happy to see some progress here! |
Wonderful comments @shaunandrews and @MichaelArestad . Thanks everyone. I'll work on improvements, but below are a few label alternatives I've looked at. Some definitely have the same issues so ignore those 😂 That being said, while the idea of a Label might feel like it belongs more in the Select mode I really enjoy playing with this PR. The label only shows when you're editing a template part and the label is still pretty subtle. I feel like it adds clarity to what you're actually editing. I'm working on some further communication and messaging ideas and one that comes to mind is putting some info in the Block inspector. I'll create a separate issue for this 😃 |
To clarify, I don't think the label is necessary in select mode. Just edit mode. |
I'm curious what @shaunandrews and @MichaelArestad think, but this makes a lot of sense to me @noahtallen. I'll try to explore this idea. |
@jeyip Showing the label of only the current active template part makes sense to me and further clarifies to the user what they are editing. |
The more I think about this, I think adding more "floating UI" into the canvas is likely the wrong approach. I think exposing the current template part in the top bar next to the document title ( |
@shaunandrews thanks for the input! I like the idea, particularly because the topbar is stickied and always visible, which isn't true for the current mocks for template part labels. I have some questions though.
@dubielzyk I'm curious what your thoughts are as well. |
Any UI we add into the canvas area is likely to affect the display of the document; In this case, the label would obscure other elements. Along the same lines, as the canvas starts to display a more realistic rendering of the front-end of a site, these sorts of floating UI will have to compete with a site's visual design. I always take pause when I see more UI being introduce into the canvas itself. |
To me adding it to the top bar seems like an unnatural place to show it, but it seems like adding a label to the canvas is going against a lot of principles and isn't getting overall agreement. I'm fine with moving forward with that solution. I'm working on the Document Settings (#20877) so I can have a deeper look at that solution. Thanks for the feedback and thoughts @shaunandrews and @jeyip :) |
+1 on this. I might even go one step further, and question why we're adding this at all. There are already two locations where the template part is exposed to users; the breadcrumb, and the block navigator. Do we need another one? If so, what problem is it solving? To be clear, I totally appreciate why users need to understand the significance of editing template parts. But it seems to me that the important consequence of that behaviour is the impact it will have on the rest of the site when changes are saved. I'm not sure that displaying a label on the currently focused template part really conveys that, assuming that the average user likely doesn't know what a "template part" is. |
Part of the problem is that users simply don't use the block navigator or block breadcrumbs. Most folks I've seen use template parts (including myself) have no clue what they are working with in the editor even though these tools exist! Maybe part of the solution is making those easier to access, but how? The breadcrumb is as obvious as it can be at the bottom, and the block navigator is not visible while you are editing anyways, so it isn't really a tool for "reorienting myself immediately." Beyond this, I worry that our focus on removing stuff from the canvas is leading to a UX which is harder to understand and reason about. I think layout building requires tools, organization, and an understanding of how stuff works on your site. Part of that would include outlines and tools in-line while you are working with different site-level documents. This is very different from post writing, which is all about removing distractions to focus on writing text. These are entirely different use cases, and other applications for those use cases reflect that. (e.g. MS word, google docs have little inline UI, whereas page builders and site builders often do have outlines and text tips to keep you oriented.) In my mind, the site editor is less about writing content, and much more about designing a great site!
Isn't this ok given that it would only show up while specifically editing the template part?
I think this is very true. But I don't think this PR specifically solves that problem. (Though I totally want to dig into solving that problem as well since it affects so many different blocks!) The problem we have currently is that even with tools like the block navigator, it takes a lot of effort to determine exactly which blocks on the canvas are children of the template part block. Currently, without a border/label while editing, you would have to click on every block until you find the location where it switches to a different block being the parent. That's a pretty rough UX. I would even say that if you don't understand what a template part is, you wouldn't even know to try that approach! (And it would certainly not be familiar to users who don't use the block navigator or breadcrumb very often.) The border solves part of that: while editing the template part, you know exactly where it begins and ends. I would suggest that the label solves the other part: exactly "what" does this border indicate? I think this is intuitive if you know what a template part is, and even if you don't, gives you enough information to work with it.
I think the big problem with this is that it relies on users to know to go looking at the header to find the information, similar to the breadcrumb or block navigator. I want to mention again that we are very open to more designs and explorations around this! The thing is: the experience isn't great right now, and I think something (like this PR along with the border) is better than nothing. I should also mention that I'm thinking about these problems from the standpoint of a beginner to WordPress (and even site building in general), so I'm very much in favor of being obvious about what's happening. |
Based on some previous discussion I'm not sure if the breadcrumb is going to stick around in the long run, but even if it does it doesn't help much from my experience. It's not easy to tell at a glance if you are editing something in the header, especially when there is a lot of nesting and that line gets too long. I think the problem with the block navigator, aside from discoverability that @noahtallen already mentioned, is that I'd have to toggle it back on whenever I want to make sure that I'm actually adding a block to the template part, and not to some other area. This will be cumbersome when adding multiple blocks. |
Do you think something like a spotlight-like UI would work better, @jameskoster @shaunandrews ? Or is it the notion of adding any UI to the canvas at all? |
You know what - you're right 😁 I think I'm getting too far ahead of myself, thinking about the holistic flow of editing template parts.
Just to note, this may not always be the case: #22113
It's an interesting idea, but forcing spotlight mode feels a little heavy-handed to me. To avoid disrupting a users edit flow I wonder if the consequences should be revealed when they're finished - on save - using the multi-entity-saving patterns that are being worked on? |
Not quite sure where we landed with this PR @jameskoster @shaunandrews. Because of the experimental nature of the site editor (and the ability to easily revert changes), @noahtallen @Addison-Stavlo and I discussed the idea of merging this PR. It would be a concrete change we could analyze and test in the context of other template part editing improvements, and if folks are still uncertain about it's utility in the future, we could roll back the updates made here. |
I'm not a fan, but I'm also not sure I have veto power :) -- I'm still seeing some bleed-through of my theme's styles in the font, which is pretty small. This seems to be using a new icon I haven't seen before, and its pretty hard to recognize at this small size. In Top Toolbar mode, it can be difficult to understand what block is selected; I see the "Header" label and a subtle border around the Template Part block — but I have have the Site Title block selected. |
Do you think there is a better way to solve this problem? |
This is a very good point. To the untrained eye it would be easy to assume that the header template part is selected there.
Perhaps we should concentrate efforts around the persistent block navigator seeing as that has the potential to solve this issue, and provide affordances for various other template editing activities as well? |
I am a firm believer that a spotlight-like UI like this improves the understanding of template parts, and what exactly you are editing in the Site Editor. Not only it provides better focus (imagine a homepage layout from our current Gutenboarding designs in the site editor, with tens of blocks competing for attention, in their full colorful glory), but it allows users to learn their site structure as they edit. |
This was deliberately introduced to provide context about which template part our blocks exist in.
This is an interesting idea. For those with less context, here's a link to the persistent block navigator issue. I'm curious about a few things:
|
From what I can tell this does aim to solve the same sort of issue, but from a different use case and perspective. That block navigator will add some great improvements, but I don't see it making the need for on-page visual indicators to go away entirely.
1000% agree with this. Casting shade and/or dimming opacity on elements outside of the template part when it is selected feels very fitting for conveying that it is a separate entity from its surroundings. |
@jameskoster How will this interact with proposed sidebar navigation in #23939? 🤔 |
I imagine this won't be an issue if we add the current template part name next to the template name in #20877 |
I mean, this is very similar to the block breadcrumb, and lots of people don't know about that even though it is visible all the time :) (But I do agree that it would be a nice improvement!) |
The template part name in the topbar also reminds me of breadcrumbs. I think that it's an improvement as well, but I think Addie made a great observation.
Maybe both would be warranted? The on-page visual indicators seem like they could complement the template part name in the topbar. |
With all the nesting involved, I don't think it would be unreasonable to consider initially auto-exposing the block navigator when template editing.
For just learning what template part you're editing at a glance I agree it is overkill. But that is what the breadcrumb (or template part name in the top bar :)) is for.
In particular; quickly and accurately selecting heavily nested blocks, dragging/dropping between those nested groups, and adding blocks in the correct location in one click. I'm sure there are others to be explored :) |
@noahtallen I wouldn't compare it to the success of breadcrumbs. The location of the breadcrumbs make them near invisible. Putting the name in the top bar would be a visible change that could reduce the need for this PR. I also strongly feel a persistent block navigator will be key to helping folks navigate the interface. It's something I'm particularly looking forward to. It also will alleviate this issue. |
It seems like we're at a bit of impasse. The original problem still stands:
But this particular solution introduces an additional problem, which in my mind blocks the merging of this PR. I think its worth pausing this particular PR and going back to the drawing board to explore more options for solving this problem. |
Agreed. Looks like this isn't the right way to go. In the original issue I've suggested (#22064 (comment)) we try out a spotlight like mode for template parts (no label or borders). Perhaps we can try that out and see what people think? |
I agree with this since we are now at an impasse :) I do still feel this is one of the best options if the only worry is helping folks understand template parts, but I'll certainly concede it causes some other problems along the way. I look forward to hearing more ideas about how to solve it! |
Thanks for the input everyone! 🙏 I appreciate the honest and open discussion. I'll go ahead and close this PR for now. If things change, we can always revisit this idea in the future. |
Description
When the child of a template part block is selected, it can be unclear which template part we're modifying.
This PR prototypes template part labels when a child block is selected (as discussed here)
Mocks
How has this been tested?
Screenshots
When selecting a child of a template part block
Types of changes
Checklist: