Skip to content
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

Add metadata: Name sections of blocks #403

Open
carolinan opened this issue Sep 25, 2024 · 12 comments
Open

Add metadata: Name sections of blocks #403

carolinan opened this issue Sep 25, 2024 · 12 comments
Labels
[Type] Enhancement A suggestion for improvement.

Comments

@carolinan
Copy link
Contributor

Is your feature request related to a problem? Please describe.
In #402, I am giving an example of why the pattern metadata should be removed.

In this issue, I would like to propose that metadata, specifically block names, can be added deliberately instead, to make it easy for users to manage sections in the ListView.

@carolinan carolinan added the [Type] Enhancement A suggestion for improvement. label Sep 25, 2024
@richtabor
Copy link
Member

In this issue, I would like to propose that metadata, specifically block names, can be added deliberately instead, to make it easy for users to manage sections in the ListView.

100%

A consistent convention could just be to use the category as the top-level block name. I.e. "Call to action" instead of "CTA: Grid layout with products and link"

@unscripted
Copy link
Member

@carolinan I'm pretty sure I follow, but for clarity, would you mind providing a few examples of what it is now vs what it would become?

@carolinan
Copy link
Contributor Author

@unscripted Blocks can be named by adding the name meta data to the comment with the JSON attributes:
"metadata":{"name":"Example"}

<!-- wp:group {"tagName":"main","metadata":{"name":"Main"},"style":{"spacing":{"margin":{"top":"var:preset|spacing|60"}}},"layout":{"type":"constrained"}} -->
<main class="wp-block-group" style="margin-top:var(--wp--preset--spacing--60)">

The name shows in the ListView and in the block settings sidebar:
image

@carolinan
Copy link
Contributor Author

I am blocking this temporally while researching if the name metadata is translatable.

If it is not already translatable by the parsing of the name, then

  • The name can not be added in the HTML files.
  • The name needs to be wrapped inside translation functions in the PHP files:

"metadata":{"name":"<?php echo esc_html_x( 'Main', 'HTML tag name', 'twentytwentyfive' ); ?>"},

@carolinan carolinan added the [Status] Blocked Used to indicate that a current effort isn't able to move forward label Oct 2, 2024
@carolinan
Copy link
Contributor Author

carolinan commented Oct 2, 2024

I am going to remove the block, but recommend that the theme does not add names to the sections.
If anyone wants to test the PHP approach above, you are welcome to, I would love to hear the results.

Link to Slack discussion:
https://wordpress.slack.com/archives/C02QB2JS7/p1727845190074579

The PHP approach could work, but needs testing to verify it doesn‘t break when changing locales etc.

@carolinan carolinan removed the [Status] Blocked Used to indicate that a current effort isn't able to move forward label Oct 2, 2024
@richtabor
Copy link
Member

I am blocking this temporally while researching if the name metadata is translatable.

If it is not already translatable by the parsing of the name, then

The name can not be added in the HTML files.
The name needs to be wrapped inside translation functions in the PHP files:

Why not add translations functions for the names? It's much more helpful seeing "Pricing Table" in the list view than "Group"—if all we have to do is add the function yea?

@getdave
Copy link

getdave commented Oct 9, 2024

I think we need to check with Block Bindings folks. I remember something about translation of this potentially being a problem as they were using the metadata name as a key. I might be out of date though.

Maybe @talldan would know?

@carolinan
Copy link
Contributor Author

carolinan commented Oct 10, 2024

Maybe @SantosGuillamot knows?

We now have three-four questions:

  • If we can safely place PHP translation functions inside the metadata name in the PHP files or if there is a better way.
    "metadata":{"name":"<?php echo esc_html_x( 'Main', 'HTML tag name', 'twentytwentyfive' ); ?>"},

  • If we are expected to include the pattern metadata in the JSON comment in the block markup inside the pattern file.
    And to follow up on that, what about when the pattern has multiple blocks, but none of them are container blocks? If the pattern has no "wrapper".

If it is intended to be included in the JSON, this must be documented so that there is no question for theme authors how to manage it, including in the Theme Developer Handbook.

@talldan
Copy link

talldan commented Oct 10, 2024

I think we need to check with Block Bindings folks. I remember something about translation of this potentially being a problem as they were using the metadata name as a key. I might be out of date though.

Yep, synced patterns do use the block name as a key for the Pattern Overrides feature. Synced patterns are stored as post types (user created content), and are not something that can be provided by a theme, so as of today this isn't an issue.

Right now all theme patterns are unsynced and can't have overrides, and I expect the names could be translated.

Just to note that it has been proposed that themes could be able to provide synced patterns in the future, if that happened the translated names (in synced patterns) might cause an issue when switching languages (the keys would suddenly change and cause a mismatch).

If we are expected to include the pattern metadata in the JSON comment in the block markup inside the pattern file.
And to follow up on that, what about when the pattern has multiple blocks, but none of them are container blocks? If the pattern has no "wrapper".

So I imagine this is regarding the categories and patternName keys? No, I don't think this metadata should be included. When an unsynced pattern is inserted by a user, this metadata is automatically added to the wrapper block from the pattern, so there should be no need to include it in the pattern source.

@draganescu
Copy link

I would love to not end up with PHP in JSON in HTML in comment in quotes. I would prefer* always passing that metadata through translation and cache it than this

  • prefer as I would go to that length, not as in that is a good alternative, I didn't consider it thorroughly

@talldan
Copy link

talldan commented Oct 10, 2024

A consistent convention could just be to use the category as the top-level block name. I.e. "Call to action" instead of "CTA: Grid layout with products and link"

When originally reading the issue, I missed that this was the intention. I thought it was more about general naming of blocks in theme templates/patterns.

At the moment AFAIK, the core/pattern (or wp:pattern) block is still being used to insert these theme patterns into a template, is that correct?

Perhaps it should be a feature of that block to add the name/category, rather than having to add the translated metadata into the pattern source code. That block would always add the translated name because it would do so at runtime.

It's also more consistent with how the inserter adds the pattern name to the top level block when inserting unsynced patterns.

Edit: it already does this, so now I'm confused! (https://github.com/WordPress/gutenberg/blob/b06549de7eebf6a1b5f67d8465648d42f9b0a27e/packages/block-library/src/pattern/edit.js#L112-L114)

@carolinan
Copy link
Contributor Author

At the moment AFAIK, the core/pattern (or wp:pattern) block is still being used to insert these theme patterns into a template, is that correct?

Yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

6 participants