Navigation: Try adding navigation link variants via server #29095
Conversation
|
Size Change: -50 B (0%) Total Size: 1.39 MB
ℹ️ View Unchanged
|
gwwar
force-pushed
the
try/block-server-variants
branch
from
0420728
to
a58467b
Compare
| return settings; | ||
| } | ||
|
|
||
| addFilter( |
There was a problem hiding this comment.
The fact that we register this filter as a side effect next to its definition isn't ideal but I guess it's fine since it's a special case. We could add this filter inside:
gutenberg/packages/block-library/src/index.js
Line 207 in 2a55b96
Although it creates some indirection. The benefit would be that this code would be eliminated if the block isn't bundled. However, I assume that this block is going to be promoted to stable in WordPress 5.8 so 🤷🏻
There was a problem hiding this comment.
🤔 we might be able to avoid the icon handling if we say added string enum support for the @wordpress/icons package.
isActive is a bit trickier. Have we in practice seen different isActive functions for variant X and variant Y in the same variants list? We might be able to pull this up a level.
Although it creates some indirection. The benefit would be that this code would be eliminated if the block isn't bundled.
Oh interesting. I'd figure that the block already being behind the enableFSEBlocks flag would keep it out of the bundle. I'm open to experimenting with placement, though what it adds is probably pretty small.
There was a problem hiding this comment.
I think it's inevitable to use the filter on the client. I raised it mostly so you were aware of the implications it creates.
There was a problem hiding this comment.
@gziolo Do folks mind if we keep this as is for now? The filter does run if we exclude the navigation link from the block library loader, but the alternative of adding it in __experimentalRegisterExperimentalCoreBlocks gets relatively messy.
| return array( | ||
| 'name' => $name, | ||
| /* translators: %s: Entity type, eg: Post Link, Page Link, Category Link, Tag Link, Portfolio Link etc */ | ||
| 'title' => sprintf( __( '%s Link' ), $entity->labels->singular_name ), |
There was a problem hiding this comment.
@gziolo any recommendations on who to ask for i18n advice here? It's not ideal that we're using another translated label to build up these strings.
There was a problem hiding this comment.
I think it's fine and follows best practices as listed at https://codex.wordpress.org/I18n_for_WordPress_Developers.
@swissspidy, can you confirm if it is the right way to go?
There was a problem hiding this comment.
get_post_type_labels() shoulld be extended in core to have a new item_link entry set to Post Link / Page Link. Same for A link to a %s. below (maybe with key item_link_description?).
For example, in German the translation for A link to a %s. changes depending on the post type.
A link to a post -> Ein Link zu einem Beitrag
A link to a page -> Ein Link zu einer Seite
sprintf( __( 'A link to a %s.' ), $entity->labels->singular_name ) doesn't allow for that.
Related: https://core.trac.wordpress.org/ticket/51387 and #19144 (comment)
There was a problem hiding this comment.
Thank you @swissspidy. I can see how speaking German makes it easier to reason about those challenges with translations. In Polish it would be:
Link do wpisuLink do strony
When I think more about it, it gets even more fun in Polish because you couldn't share $entity->labels->singular_name:
To jest wpis(This is post)To jest strona(This is page)
So I guess your proposal should address those type of issues.
There was a problem hiding this comment.
Thanks for the guidance here @swissspidy @gziolo. I can open a trac issue and create a patch for a new entry in get_post_type_labels
There was a problem hiding this comment.
I prototyped the change in WordPress/wordpress-develop@298bbd1 and 5df7ec4. Happy to pull this out into a smaller PR.
I ended up adding some fallback logic, so we wouldn't end up with "Post Link" labels for other CPTs that haven't defined it.
| New Label | Fallback (notice the capitalization in description because this is built with singular_name) |
|---|---|
 |
 |
There was a problem hiding this comment.
Decided to pull out the fallback so behavior looks like this if labels are undefined:
gwwar
force-pushed
the
try/block-server-variants
branch
2 times, most recently
from
9cf8abf
to
159288d
Compare
gwwar
force-pushed
the
try/block-server-variants
branch
from
2b69ef0
to
5df7ec4
Compare
…category link in server from register_block_type_from_metadata instead of hardcoded file.
…register_block_type_from_metadata
gwwar
force-pushed
the
try/block-server-variants
branch
from
df4df14
to
6b07ad6
Compare
|
|
Thanks for the reviews @gziolo @vcanales @swissspidy ✨ The last item for consideration is if it's okay to keep the Theoretically I can merge this before the server PR, but we might want to see if WordPress/wordpress-develop#1015 has traction first. |
It's probably fine to leave it as is. If it becomes more popular then we can think about some sophisticated approach that optimizes the bundle size. We could for instance return the hooks as an array of arrays and include in: gutenberg/packages/block-library/src/index.js Lines 104 to 107 in f04f579 const { metadata, settings, name, hooks } = block;
for ( const hookArgs of hooks ) {
addFilter( ...hookArgs );
}It becomes a bit complex though 😄 |
|
Thanks to @gziolo we should expect the server side of this to land sometime this week after the 5.7 release WordPress/wordpress-develop#1015 (comment) Going to land the client patch first. Thanks again for the reviews @swissspidy @gziolo @vcanales ✨ |
Currently block variations are only defined on the client. In some cases, creating block variations on the server can be very useful, especially when needed data is not exposed in the REST APIs. Related to WordPress/gutenberg#29095. Props: gwwar, timothyblynjacobs. Fixes: #52688. git-svn-id: https://develop.svn.wordpress.org/trunk@50527 602fd350-edb4-49c9-b593-d223f7449a82
Currently block variations are only defined on the client. In some cases, creating block variations on the server can be very useful, especially when needed data is not exposed in the REST APIs. Related to WordPress/gutenberg#29095. Props: gwwar, timothyblynjacobs. Fixes: #52688. git-svn-id: https://develop.svn.wordpress.org/trunk@50527 602fd350-edb4-49c9-b593-d223f7449a82
Currently block variations are only defined on the client. In some cases, creating block variations on the server can be very useful, especially when needed data is not exposed in the REST APIs. Related to WordPress/gutenberg#29095. Props: gwwar, timothyblynjacobs. Fixes: #52688. Built from https://develop.svn.wordpress.org/trunk@50527 git-svn-id: http://core.svn.wordpress.org/trunk@50140 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Currently block variations are only defined on the client. In some cases, creating block variations on the server can be very useful, especially when needed data is not exposed in the REST APIs. Related to WordPress/gutenberg#29095. Props: gwwar, timothyblynjacobs. Fixes: #52688. Built from https://develop.svn.wordpress.org/trunk@50527 git-svn-id: https://core.svn.wordpress.org/trunk@50140 1a063a9b-81f0-0310-95a4-ce76da25c4cd
| ...( ! variation.icon && { | ||
| icon: getIcon( variation.name ), | ||
| } ), | ||
| ...( ! variation.isActive && { |
Currently block variations are only defined on the client. In some cases, creating block variations on the server can be very useful, especially when needed data is not exposed in the REST APIs. Related to WordPress/gutenberg#29095. Props: gwwar, timothyblynjacobs. Fixes: #52688. Built from https://develop.svn.wordpress.org/trunk@50527 git-svn-id: http://core.svn.wordpress.org/trunk@50140 1a063a9b-81f0-0310-95a4-ce76da25c4cd
|
In theory, it should work the same in WordPress core. However, maybe the issue reported in #49678 is the reason why it only works with the Gutenberg plugin. As far as I can tell, the same logic is in place in both places: |
Oh that's right, I just tested that issue 🤦♂️ I can confirm that changing the priority, makes it work. |
The navigation block currently allows insertion of Links, Post Links, Page Links, Tag Links and Category Links. This PR adds the ability to insert links for custom post types and taxonomies. If we install a Portfolio plugin for example, we should be able to add Portfolio Links.
In master, the list of link variations is hardcoded. This PR experiments with registering navigation link variants using
register_block_type_from_metadataand passing not only a render callback, but the variations array. One benefit of doing this is being able to fetch post types and taxonomies and filter byshow_in_nav_menuswhich is not exposed via the REST API. It also has some flexibility in pulling other data from the server without needing to bloat the API response (values here are unlikely to be reused elsewhere).Part of #24814, Alternative to #29011, Requires WordPress/wordpress-develop#1015 to test.
navigationlink.mp4
TODO
The server definition is being blown away by the local block.json.Drop placeholder change in favor of Blocks: Ensure that metadata registered on the server for core block is preserved on the client #29213Testing Instructions with WordPress/wordpress-develop#1015
try/block-server-variants. One way of doing this on a local install is to link this vialn -s your-gutenberg-checkout-path your-wordpress-local-path/wp-content/plugins/gutenberg. Runnpm run buildin your gutenberg checkoutIf Custom Post Type or Taxonomy does not define new
item_linkoritem_link_descriptionlabels it shows the fallback:With
item_linkoritem_link_descriptionlabelsThis will require manually modifying a taxonomy or post_type registration since no plugin has added this yet:
CPT Drafts should not render on Navigation Block
privateTesting Instructions for other WP versions