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

Update defaults for Home Link block #58307

Closed
tjcafferkey opened this issue Jan 26, 2024 · 5 comments · Fixed by #58387
Closed

Update defaults for Home Link block #58307

tjcafferkey opened this issue Jan 26, 2024 · 5 comments · Fixed by #58387
Assignees
Labels
[Block] Home Link Affects the Home Link Block [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.

Comments

@tjcafferkey
Copy link
Contributor

Description

It's now possible to use the Block Hooks API to insert inner blocks into the core Navigation block #57754. Using this API currently doesn't allow you to set attributes along with the hooked block.

We found that when you use the API to insert the core/home-link block it has insufficient default attributes. When we insert this block via the API we can't set any attributes and one of the conditions for this block to render server-side is a label attribute, see here.

When you insert the block via the editor, this gets added on the client-side. See here.

Step-by-step reproduction instructions

  1. Add the below code to your site.
  2. Load frontend and see that the block doesn't render at all.
  3. Change core/home-link to core/loginout in the snippet below to see a block rendering correctly as comparison.
function register_home_link_block_as_navigation_last_child( $hooked_blocks, $position, $anchor_block, $context ) {
	if ( $anchor_block === 'core/navigation' && $position === 'last_child' ) {
		$hooked_blocks[] = 'core/home-link';
	}

	return $hooked_blocks;
}

add_filter( 'hooked_block_types', 'register_home_link_block_as_navigation_last_child', 10, 4 );

Screenshots, screen recording, code snippet

No response

Environment info

  • WP 6.4
  • Gutenberg latest trunk or 17.6 and above.

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@tjcafferkey tjcafferkey added the [Type] Bug An existing feature does not function as intended label Jan 26, 2024
@jordesign jordesign added [Type] Enhancement A suggestion for improvement. [Block] Home Link Affects the Home Link Block and removed [Type] Bug An existing feature does not function as intended labels Jan 28, 2024
@ockham
Copy link
Contributor

ockham commented Jan 29, 2024

When you insert the block via the editor, this gets added on the client-side. See here.

I was wondering why this wasn't done via an attribute default when the block was created in the first place. Maybe it's the i18n? I.e. the __( 'Home' ) bit?

setAttributes( { label: __( 'Home' ) } );

(I'm not sure if we have a mechanism to support localized strings like that in block.json nowadays 🤔 )

@ockham
Copy link
Contributor

ockham commented Jan 29, 2024

Looks like this is the authoritative source of what block.json fields are translated (h/t @MaggieCabrera for the pointer!)

No attribute defaults in that one. So that might explain why this was done on the editor side in the first place 😬

@tjcafferkey
Copy link
Contributor Author

Thanks @ockham! I suspected it may have something to do with translations. Maybe instead of setting the attribute if there isn't one, we can render __( 'Home' ) in the absence of this attribute but allow users to override it by explicitly setting it still?

@ockham
Copy link
Contributor

ockham commented Jan 29, 2024

Thanks @ockham! I suspected it may have something to do with translations. Maybe instead of setting the attribute if there isn't one, we can render __( 'Home' ) in the absence of this attribute but allow users to override it by explicitly setting it still?

That sounds like a good idea! It would basically reflect the behavior in the editor, and allow inclusion of a Home Link block with no attributes set 👍

@ockham
Copy link
Contributor

ockham commented Jan 29, 2024

Looks like this is the authoritative source of what block.json fields are translated (h/t @MaggieCabrera for the pointer!)

No attribute defaults in that one. So that might explain why this was done on the editor side in the first place 😬

Post-scriptum, looks like it was actually discussed at the time: https://github.com/WordPress/gutenberg/pull/30293/files#r619407489

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Home Link Affects the Home Link Block [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants