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

Navigation: Add post format variation to navigation link block #30403

Merged
merged 8 commits into from
Apr 1, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/block-library/src/navigation-link/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ function getSuggestionsQuery( type, kind ) {
return { type: 'term', subtype: 'category' };
case 'tag':
return { type: 'term', subtype: 'post_tag' };
case 'format':
return { type: 'post-format' };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For folks wondering about the kebab case:

if ( ! disablePostFormats && ( ! type || type === 'post-format' ) ) {

FYI on behavior, it also looks like we removed the labels in search results if all results are the same:
#24839

So this is expected behavior:

Custom Link Search No labels with all post formats
custom link search mixed types post format same types

default:
if ( kind === 'taxonomy' ) {
return { type: 'term', subtype: type };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
page as pageIcon,
postTitle as postIcon,
tag as tagIcon,
blockDefault as formatIcon,
} from '@wordpress/icons';

// FALLBACK: this is only used when the server does not understand the variations property in the
Expand Down Expand Up @@ -49,6 +50,13 @@ const fallbackVariations = [
description: __( 'A link to a tag.' ),
attributes: { type: 'tag' },
},
{
name: 'format',
icon: formatIcon,
title: __( 'Post Format Link' ),
description: __( 'A link to a post format.' ),
attributes: { type: 'format' },
},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't seem to make this work without the fallback 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see what's happening. First, I need to be on trunk for core. Now after that I have two Tag Link variations. 😕

Wait, so it looks like Post Formats have the same item_link and item_link_description as tags 🤯

Someone copied and pasted it years ago and nobody ever noticed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, ok, so I think there are some defaults in core for tags or categories:
https://github.com/WordPress/wordpress-develop/blob/0df28171ed9c3c31ebc93add82b64df8f690f87c/src/wp-includes/taxonomy.php#L597-L629

Seems unusual.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't seem to make this work without the fallback

Right any WP instances running 5.7 and below will be using the fallback.

We'll want to update hooks.js or similar to only add this value when post-formats are supported by the theme:

// Fallback handling may be deleted after supported WP ranges understand the `variations`
// property when passed to register_block_type_from_metadata in index.php
if ( ! settings.variations ) {
return {
...settings,
variations: fallbackVariations,
};
}

Shows up in TT1 / WP 5.7 No Search Results
Screen Shot 2021-03-31 at 8 54 01 AM Screen Shot 2021-03-31 at 8 53 41 AM

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Irrelevant, but I think TT1 does support formats, but the search only shows results when one is added to a published post

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

disablePostFormats is an editor setting from the wordpress/editor package, so this is tricky to use. The navigation editor doesn't use the editor package, and those settings aren't really relevant to it.

Tempted to either remove it, or keep it as is. The impact is quite low either way.

I think I'll remove it for now.

];

/**
Expand Down
14 changes: 10 additions & 4 deletions packages/block-library/src/navigation-link/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,14 @@ function render_block_core_navigation_link( $attributes, $content, $block ) {
* @return array
*/
function build_variation_for_navigation_link( $entity, $kind ) {
$name = 'post_tag' === $entity->name ? 'tag' : $entity->name;
$custom_variation_names = array(
'post_tag' => 'tag',
'post_format' => 'format',
);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This kind of thing makes me think the variations could be called the same thing as the the entity names.


$name = array_key_exists( $entity->name, $custom_variation_names )
? $custom_variation_names[ $entity->name ]
: $entity->name;

$title = '';
$description = '';
Expand Down Expand Up @@ -263,7 +270,6 @@ function build_variation_for_navigation_link( $entity, $kind ) {
* @throws WP_Error An WP_Error exception parsing the block definition.
*/
function register_block_core_navigation_link() {

$post_types = get_post_types( array( 'show_in_nav_menus' => true ), 'objects' );
$taxonomies = get_taxonomies( array( 'show_in_nav_menus' => true ), 'objects' );
$built_ins = array();
Expand All @@ -272,7 +278,7 @@ function register_block_core_navigation_link() {
if ( $post_types ) {
foreach ( $post_types as $post_type ) {
$variation = build_variation_for_navigation_link( $post_type, 'post-type' );
if ( 'post' === $variation['name'] || 'page' === $variation['name'] ) {
if ( 'post' === $variation['name'] || 'page' === $variation['name'] || 'format' === $variation['name'] ) {
talldan marked this conversation as resolved.
Show resolved Hide resolved
$built_ins[] = $variation;
} else {
$variations[] = $variation;
Expand All @@ -282,7 +288,7 @@ function register_block_core_navigation_link() {
if ( $taxonomies ) {
foreach ( $taxonomies as $taxonomy ) {
$variation = build_variation_for_navigation_link( $taxonomy, 'taxonomy' );
if ( 'category' === $variation['name'] || 'tag' === $variation['name'] ) {
if ( 'category' === $variation['name'] || 'tag' === $variation['name'] || 'post_' ) {
$built_ins[] = $variation;
} else {
$variations[] = $variation;
Expand Down