Size Change: +266 B (0%)

Total Size: 1.25 MB

_{compressed-size-action}

There's an existing system for doing this that should make the PR a bit smaller.

You can see how the navigation link block achieves it here - https://github.com/WordPress/gutenberg/blob/trunk/packages/block-library/src/navigation-link/index.js#L25.

@talldan that reminds me, we should probably unexperimentalize that API :)

Thanks @talldan, using that function's a great idea! I've updated this PR — I went a little further and added a context param to the BlockTitle and useBlockDisplayTitle so that we can conditionally use the block content only in the list view. Let me know if you don't think this is the right way to handle it, but I thought that could be a neat way to ensure that the block name is still used in other places (such as the Remove button in the block settings dropdown in the editor canvas).

I went a little further and added a context param to the BlockTitle and useBlockDisplayTitle so that we can conditionally use the block content only in the list view.

I wonder if that'll be needed. I think the only other place it'll display is the breadcrumb at the bottom of the screen. I think other blocks that use __experimentalLabel show their label in the breadcrumb.

It might be good to get input from a designer about this part.

I wonder if that'll be needed

Good question, it appears to be used for both the breadcrumb at the bottom of the screen and the Remove button in the dropdown (the settings dropdown uses useBlockDisplayTitle here). With some longer titles it felt a bit odd to me using the content as the label, whereas in the List view it feels a bit clearer to me since it has the Heading icon. Here's how it looks if we don't have the check for list-view context:

I'll add a "Needs Design feedback" label.

TL;DR for designers: should we use Heading content as the label just in the list view, or is it also okay in the breadcrumb at the bottom of the screen, and in the remove button in the settings menu? (I'm happy to implement it either way 🙂)

Good question, it appears to be used for both the breadcrumb at the bottom of the screen and the Remove button in the dropdown (the settings dropdown uses useBlockDisplayTitle here).

Hmm, yeah, it doesn't look great in the remove button. There may need to be some more nuance in how this is shown. I think the distinction is in whether the title shows content (like it will for Heading and does for Navigation Link), or whether it represents the semantic name of a block (like it does for variations and the names of template parts / reusable blocks).

I think the distinction is in whether the title shows content

Ah, so we could potentially use a context value of content instead of list-view?

I think the problem with really specific terms like 'list-view' is that this could grow into a really big list of contexts as different types of UI are introduced, or ideas around this are iterated on.

All in all, I'm not sure this context system is the right solution in the first place. Maybe it should be something like this pseudocode:

getLabels( blockType, attributes ) {
    return {
        // used where there's a short amount of space for the block (e.g. remove block button)
        title: `${ blockType.title } (${ attributes.level })`,
        // used where there's more space (list view, breadcrumbs)
        label: attributes.content,
        // used for aria labels
        accessibility: `${ attributes.level } - ${ attributes.content }`,
    };
}

These outstanding issues are one of the reasons why it hasn't been stabilised yet

These outstanding issues are one of the reasons why it hasn't been stabilised yet

Ah, gotcha. And then it's also potentially tricky if we want different behaviour for different blocks (like in the screenshot of how the Paragraph block might work: #41855 (comment)), which appears to be a strength of the existing API 🤔. If we wind up wanting to proceed with the label in the list view being different from the remove button or breadcrumbs, but it's too fiddly to land on a change to that API, I can always roll this PR back to the initial commit.

Alternately, if it's acceptable that the Remove button also uses the content, then there's a clearer path to a smaller changeset for this PR (just change the __experimentalLabel for the heading block for all visual contexts, and don't mess with the BlockTitle component 🙂).

Thanks again for thinking through the implications, and I'm happy to keep hacking away tomorrow!

My inclination would be to use only on list-view. Not quite sure about breadcrumbs, but I lean towards no.

Thanks for the feedback! I ran out of time to progress this one today, but I'll revisit next week. I suspect we'll need a little more logic to make sure that long titles truncate comfortably for headings with a custom anchor id in the button label, too, so as a first step, it might be worth us keeping the logic encapsulated to the List View button component for now (with a hard-coded list of blocks we're dealing with similar to my first commit), also so that we can side-step the discussion about the shape of the label API.

That said, do let me know if anyone feels strongly about how it should be implemented, though, and I'll incorporate the feedback when I revisit this next week. Thanks, all! 🙇

I've updated this PR to revert back to the approach of using some hard-coded logic in the block-select-button component, so that we can side-step the problem of the label API for the moment.

I took a look at how to get the truncation to play nicely with the id attribute, and it looks like if we add an HStack wrapper over the title and anchor tag, we can get the truncation to factor in the existence of that pill. As an example, here's a screenshot of the current state of the PR:

In the above screenshot, from the highlighted Heading:

• A long heading that gets truncated to still allow room for the HTML anchor.
• A long heading that gets truncated.
• A long heading that gets truncated and is locked (shows how it sits with a lock icon)
• A reusable block that gets truncated (no change here, just demonstrating)

In each of the above examples, space is effectively "reserved" for the lock icon via the max width settings. If the lock is removed on that particular heading, then the whole List View returns to the default width of 350px.

To achieve the above, I've added a max-width of 225px to the wrapper containing the title + anchor id, which appears to be the maximum width before the long titles start to extend over the minimum width of the List View container (350px) — there might be a better width to go with (either a little less or a little more), but at least in my testing today, it seemed to be a kind of sweet spot between too narrow, or letting long headings stretch out the list view to be a bit too wide. Very happy for feedback or ideas on an ideal max width.

The PR probably still needs a little bit of tidying, but happy to try implementing any other ideas. I think the additional HStack with the max-width should help us with the Paragraph block follow-up, too, where we'll try out having grey truncated content text next to the block title (as in this screenshot: #41855 (comment)).

(Note: I think the current change possibly breaks the e2e tests, so I'll look at fixing that up, too)

I've updated this PR to revert back to the approach of using some hard-coded logic in the block-select-button component, so that we can side-step the problem of the label API for the moment.

Sorry to potentially make you u-turn again, but I do think it'd be better to use the existing label API. The context === 'list-view' approach you outlined is a reasonable trade-off right now if there are no other quick wins. At least this means that all the blocks that have a label are implemented similarly, and there's then a basis from which to improve the whole API.

I don't think defining the block names implicitly in list view is the way to go - it wouldn't be something that could easily develop as an API because it limits the feature to core blocks.

Sorry to potentially make you u-turn again, but I do think it'd be better to use the existing label API. The context === 'list-view' approach you outlined is a reasonable trade-off right now if there are no other quick wins.

Not a bother at all @talldan, it's easy to revert! I'll rework this PR to go with the list-view context approach later on today, and it was still useful figuring out the max-width stuff yesterday, so no wasted effort 🙂

Just a note for myself, I think the e2e tests are failing because of the additional div so the title span isn't a direct child of the a tag in the list view. I'll probably need to update the logic in: https://github.com/WordPress/gutenberg/blob/trunk/packages/e2e-test-utils/src/get-list-view-blocks.js#L7, and maybe try to make it a little more flexible. I'll look at that tomorrow.

From testing just now - anchor text truncates a little early and doesn't show many characters:

Before:

After:

It feels like it should take up any available space. It's probably really tricky to account for all the different situations.

It feels like it should take up any available space. It's probably really tricky to account for all the different situations.

It sure is! There's kind of a delicate balancing act between which things should fill up which available space and when 😆
I can have another go at tweaking that one on Monday (it might be that anchor should be in the same wrapper as title, but not the same wrapper as lock 🤔). Thanks for testing!

Thanks for the reviews, folks! It turned out not to be too hard to fix up the anchor, too — I wound up using flex: 1 on the title wrapper so that it isn't quite as greedy at using up the available space, and then used a min() function on the max-width of the anchor so that it never uses up more than 40% of the available space (so that it isn't too greedy with taking up space in narrow titles 😄)

Seems to be working nicely now, so I'll merge this in once tests pass:

Happy to continue tweaking in follow-ups, of course — I'll have a go at the Paragraph block next. Thanks again! 🙇

We could consider not rendering the anchor label on headings when they match the same name. I also think it might be useful to add a "Copy anchor" to the ellipsis menu.